2005-02-24 Havoc Pennington <hp@redhat.com>

	* dbus/dbus-server.c, dbus/dbus-server-unix.c: change semantics so
	you must disconnect before unref, since locking and other things
	are screwed up otherwise. Fix assorted other locking stuff.

	* dbus/dbus-signature.c (dbus_signature_iter_get_element_type):
	fix compilation

	* dbus/dbus-threads-internal.h: move the mutex/condvar wrappers
	into a private header and don't export from the library

	* throughout - call _dbus_thread_stuff vs. dbus_thread_stuff

12 files changed, 167 insertions, 117 deletions

diff --git a/ChangeLog b/ChangeLog
index 5359bf51..9ff8aa44 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+2005-02-24  Havoc Pennington  <hp@redhat.com>
+
+	* dbus/dbus-server.c, dbus/dbus-server-unix.c: change semantics so
+	you must disconnect before unref, since locking and other things
+	are screwed up otherwise. Fix assorted other locking stuff.
+
+	* dbus/dbus-signature.c (dbus_signature_iter_get_element_type):
+	fix compilation
+
+	* dbus/dbus-threads-internal.h: move the mutex/condvar wrappers
+	into a private header and don't export from the library
+
+	* throughout - call _dbus_thread_stuff vs. dbus_thread_stuff
+
 2005-02-24  Colin Walters  <walters@verbum.org>
 	
 	* dbus/dbus-signature.c: New file; implements various functions
diff --git a/dbus/Makefile.am b/dbus/Makefile.am
index 24d99107..ed58cd75 100644
--- a/dbus/Makefile.am
+++ b/dbus/Makefile.am
@@ -82,6 +82,7 @@ DBUS_LIB_SOURCES=				\
 	dbus-signature.h			\
 	dbus-timeout.c				\
 	dbus-timeout.h				\
+	dbus-threads-internal.h			\
 	dbus-threads.c				\
 	dbus-transport.c			\
 	dbus-transport.h			\
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index ae83af69..9c57edc9 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -38,6 +38,7 @@
 #include "dbus-string.h"
 #include "dbus-pending-call.h"
 #include "dbus-object-tree.h"
+#include "dbus-threads-internal.h"
 
 #ifdef DBUS_DISABLE_CHECKS
 #define TOOK_LOCK_CHECK(connection)
@@ -60,14 +61,14 @@
 
 #define CONNECTION_LOCK(connection)   do {                                      \
     if (TRACE_LOCKS) { _dbus_verbose ("  LOCK: %s\n", _DBUS_FUNCTION_NAME); }   \
-    dbus_mutex_lock ((connection)->mutex);                                      \
+    _dbus_mutex_lock ((connection)->mutex);                                      \
     TOOK_LOCK_CHECK (connection);                                               \
   } while (0)
 
 #define CONNECTION_UNLOCK(connection) do {                                              \
     if (TRACE_LOCKS) { _dbus_verbose ("  UNLOCK: %s\n", _DBUS_FUNCTION_NAME);  }        \
     RELEASING_LOCK_CHECK (connection);                                                  \
-    dbus_mutex_unlock ((connection)->mutex);                                            \
+    _dbus_mutex_unlock ((connection)->mutex);                                            \
   } while (0)
 
 #define DISPATCH_STATUS_NAME(s)                                            \
@@ -922,7 +923,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
   CONNECTION_UNLOCK (connection);
   
   _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_lock (connection->io_path_mutex);
+  _dbus_mutex_lock (connection->io_path_mutex);
 
   _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n",
                  _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds);
@@ -935,16 +936,16 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
         {
           _dbus_verbose ("%s waiting %d for IO path to be acquirable\n",
                          _DBUS_FUNCTION_NAME, timeout_milliseconds);
-          dbus_condvar_wait_timeout (connection->io_path_cond,
-                                     connection->io_path_mutex,
-                                     timeout_milliseconds);
+          _dbus_condvar_wait_timeout (connection->io_path_cond,
+                                      connection->io_path_mutex,
+                                      timeout_milliseconds);
         }
       else
         {
           while (connection->io_path_acquired)
             {
               _dbus_verbose ("%s waiting for IO path to be acquirable\n", _DBUS_FUNCTION_NAME);
-              dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex);
+              _dbus_condvar_wait (connection->io_path_cond, connection->io_path_mutex);
             }
         }
     }
@@ -959,7 +960,7 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
                  _DBUS_FUNCTION_NAME, connection->io_path_acquired, we_acquired);
 
   _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_unlock (connection->io_path_mutex);
+  _dbus_mutex_unlock (connection->io_path_mutex);
 
   CONNECTION_LOCK (connection);
   
@@ -983,7 +984,7 @@ _dbus_connection_release_io_path (DBusConnection *connection)
   HAVE_LOCK_CHECK (connection);
   
   _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_lock (connection->io_path_mutex);
+  _dbus_mutex_lock (connection->io_path_mutex);
   
   _dbus_assert (connection->io_path_acquired);
 
@@ -991,10 +992,10 @@ _dbus_connection_release_io_path (DBusConnection *connection)
                  _DBUS_FUNCTION_NAME, connection->io_path_acquired);
   
   connection->io_path_acquired = FALSE;
-  dbus_condvar_wake_one (connection->io_path_cond);
+  _dbus_condvar_wake_one (connection->io_path_cond);
 
   _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_unlock (connection->io_path_mutex);
+  _dbus_mutex_unlock (connection->io_path_mutex);
 }
 
 /**
@@ -1113,27 +1114,27 @@ _dbus_connection_new_for_transport (DBusTransport *transport)
   if (connection == NULL)
     goto error;
 
-  mutex = dbus_mutex_new ();
+  mutex = _dbus_mutex_new ();
   if (mutex == NULL)
     goto error;
 
-  io_path_mutex = dbus_mutex_new ();
+  io_path_mutex = _dbus_mutex_new ();
   if (io_path_mutex == NULL)
     goto error;
 
-  dispatch_mutex = dbus_mutex_new ();
+  dispatch_mutex = _dbus_mutex_new ();
   if (dispatch_mutex == NULL)
     goto error;
   
-  message_returned_cond = dbus_condvar_new ();
+  message_returned_cond = _dbus_condvar_new ();
   if (message_returned_cond == NULL)
     goto error;
   
-  dispatch_cond = dbus_condvar_new ();
+  dispatch_cond = _dbus_condvar_new ();
   if (dispatch_cond == NULL)
     goto error;
   
-  io_path_cond = dbus_condvar_new ();
+  io_path_cond = _dbus_condvar_new ();
   if (io_path_cond == NULL)
     goto error;
 
@@ -1203,22 +1204,22 @@ _dbus_connection_new_for_transport (DBusTransport *transport)
     _dbus_list_free_link (disconnect_link);
   
   if (io_path_cond != NULL)
-    dbus_condvar_free (io_path_cond);
+    _dbus_condvar_free (io_path_cond);
   
   if (dispatch_cond != NULL)
-    dbus_condvar_free (dispatch_cond);
+    _dbus_condvar_free (dispatch_cond);
   
   if (message_returned_cond != NULL)
-    dbus_condvar_free (message_returned_cond);
+    _dbus_condvar_free (message_returned_cond);
   
   if (mutex != NULL)
-    dbus_mutex_free (mutex);
+    _dbus_mutex_free (mutex);
 
   if (io_path_mutex != NULL)
-    dbus_mutex_free (io_path_mutex);
+    _dbus_mutex_free (io_path_mutex);
 
   if (dispatch_mutex != NULL)
-    dbus_mutex_free (dispatch_mutex);
+    _dbus_mutex_free (dispatch_mutex);
   
   if (connection != NULL)
     dbus_free (connection);
@@ -1540,13 +1541,13 @@ _dbus_connection_last_unref (DBusConnection *connection)
 
   _dbus_list_clear (&connection->link_cache);
   
-  dbus_condvar_free (connection->dispatch_cond);
-  dbus_condvar_free (connection->io_path_cond);
+  _dbus_condvar_free (connection->dispatch_cond);
+  _dbus_condvar_free (connection->io_path_cond);
 
-  dbus_mutex_free (connection->io_path_mutex);
-  dbus_mutex_free (connection->dispatch_mutex);
+  _dbus_mutex_free (connection->io_path_mutex);
+  _dbus_mutex_free (connection->dispatch_mutex);
 
-  dbus_mutex_free (connection->mutex);
+  _dbus_mutex_free (connection->mutex);
   
   dbus_free (connection);
 }
@@ -2758,12 +2759,12 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection)
   CONNECTION_UNLOCK (connection);
   
   _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_lock (connection->dispatch_mutex);
+  _dbus_mutex_lock (connection->dispatch_mutex);
 
   while (connection->dispatch_acquired)
     {
       _dbus_verbose ("%s waiting for dispatch to be acquirable\n", _DBUS_FUNCTION_NAME);
-      dbus_condvar_wait (connection->dispatch_cond, connection->dispatch_mutex);
+      _dbus_condvar_wait (connection->dispatch_cond, connection->dispatch_mutex);
     }
   
   _dbus_assert (!connection->dispatch_acquired);
@@ -2771,7 +2772,7 @@ _dbus_connection_acquire_dispatch (DBusConnection *connection)
   connection->dispatch_acquired = TRUE;
 
   _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_unlock (connection->dispatch_mutex);
+  _dbus_mutex_unlock (connection->dispatch_mutex);
   
   CONNECTION_LOCK (connection);
   _dbus_connection_unref_unlocked (connection);
@@ -2790,15 +2791,15 @@ _dbus_connection_release_dispatch (DBusConnection *connection)
   HAVE_LOCK_CHECK (connection);
   
   _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_lock (connection->dispatch_mutex);
+  _dbus_mutex_lock (connection->dispatch_mutex);
   
   _dbus_assert (connection->dispatch_acquired);
 
   connection->dispatch_acquired = FALSE;
-  dbus_condvar_wake_one (connection->dispatch_cond);
+  _dbus_condvar_wake_one (connection->dispatch_cond);
 
   _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
-  dbus_mutex_unlock (connection->dispatch_mutex);
+  _dbus_mutex_unlock (connection->dispatch_mutex);
 }
 
 static void
diff --git a/dbus/dbus-dataslot.c b/dbus/dbus-dataslot.c
index 675f5cf6..8026d048 100644
--- a/dbus/dbus-dataslot.c
+++ b/dbus/dbus-dataslot.c
@@ -70,7 +70,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator,
 {
   dbus_int32_t slot;
 
-  if (!dbus_mutex_lock (mutex))
+  if (!_dbus_mutex_lock (mutex))
     return FALSE;
 
   if (allocator->n_allocated_slots == 0)
@@ -142,7 +142,7 @@ _dbus_data_slot_allocator_alloc (DBusDataSlotAllocator *allocator,
                  slot, allocator, allocator->n_allocated_slots, allocator->n_used_slots);
   
  out:
-  dbus_mutex_unlock (allocator->lock);
+  _dbus_mutex_unlock (allocator->lock);
   return slot >= 0;
 }
 
@@ -161,7 +161,7 @@ void
 _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator,
                                 dbus_int32_t          *slot_id_p)
 {
-  dbus_mutex_lock (allocator->lock);
+  _dbus_mutex_lock (allocator->lock);
   
   _dbus_assert (*slot_id_p < allocator->n_allocated_slots);
   _dbus_assert (allocator->allocated_slots[*slot_id_p].slot_id == *slot_id_p);
@@ -171,7 +171,7 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator,
 
   if (allocator->allocated_slots[*slot_id_p].refcount > 0)
     {
-      dbus_mutex_unlock (allocator->lock);
+      _dbus_mutex_unlock (allocator->lock);
       return;
     }
 
@@ -193,11 +193,11 @@ _dbus_data_slot_allocator_free (DBusDataSlotAllocator *allocator,
       allocator->n_allocated_slots = 0;
       allocator->lock = NULL;
 
-      dbus_mutex_unlock (mutex);
+      _dbus_mutex_unlock (mutex);
     }
   else
     {
-      dbus_mutex_unlock (allocator->lock);
+      _dbus_mutex_unlock (allocator->lock);
     }
 }
 
@@ -243,11 +243,11 @@ _dbus_data_slot_list_set  (DBusDataSlotAllocator *allocator,
    * be e.g. realloc()ing allocated_slots. We avoid doing this if asserts
    * are disabled, since then the asserts are empty.
    */
-  if (!dbus_mutex_lock (allocator->lock))
+  if (!_dbus_mutex_lock (allocator->lock))
     return FALSE;
   _dbus_assert (slot < allocator->n_allocated_slots);
   _dbus_assert (allocator->allocated_slots[slot].slot_id == slot);
-  dbus_mutex_unlock (allocator->lock);
+  _dbus_mutex_unlock (allocator->lock);
 #endif
   
   if (slot >= list->n_slots)
@@ -301,12 +301,12 @@ _dbus_data_slot_list_get  (DBusDataSlotAllocator *allocator,
    * be e.g. realloc()ing allocated_slots. We avoid doing this if asserts
    * are disabled, since then the asserts are empty.
    */
-  if (!dbus_mutex_lock (allocator->lock))
+  if (!_dbus_mutex_lock (allocator->lock))
     return FALSE;
   _dbus_assert (slot >= 0);
   _dbus_assert (slot < allocator->n_allocated_slots);
   _dbus_assert (allocator->allocated_slots[slot].slot_id == slot);
-  dbus_mutex_unlock (allocator->lock);
+  _dbus_mutex_unlock (allocator->lock);
 #endif
 
   if (slot >= list->n_slots)
@@ -389,7 +389,7 @@ _dbus_data_slot_test (void)
 
   _dbus_data_slot_list_init (&list);
 
-  mutex = dbus_mutex_new ();
+  mutex = _dbus_mutex_new ();
   if (mutex == NULL)
     _dbus_assert_not_reached ("failed to alloc mutex");
   
@@ -469,7 +469,7 @@ _dbus_data_slot_test (void)
       ++i;
     }
 
-  dbus_mutex_free (mutex);
+  _dbus_mutex_free (mutex);
   
   return TRUE;
 }
diff --git a/dbus/dbus-internals.h b/dbus/dbus-internals.h
index be19e1bc..dcbc6183 100644
--- a/dbus/dbus-internals.h
+++ b/dbus/dbus-internals.h
@@ -267,8 +267,8 @@ extern int _dbus_current_generation;
 #define _DBUS_LOCK_NAME(name)           _dbus_lock_##name
 #define _DBUS_DECLARE_GLOBAL_LOCK(name) extern DBusMutex  *_dbus_lock_##name
 #define _DBUS_DEFINE_GLOBAL_LOCK(name)  DBusMutex         *_dbus_lock_##name  
-#define _DBUS_LOCK(name)                dbus_mutex_lock   (_dbus_lock_##name)
-#define _DBUS_UNLOCK(name)              dbus_mutex_unlock (_dbus_lock_##name)
+#define _DBUS_LOCK(name)                _dbus_mutex_lock   (_dbus_lock_##name)
+#define _DBUS_UNLOCK(name)              _dbus_mutex_unlock (_dbus_lock_##name)
 
 _DBUS_DECLARE_GLOBAL_LOCK (list);
 _DBUS_DECLARE_GLOBAL_LOCK (connection_slots);
diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h
index 5862c6c2..284d02c5 100644
--- a/dbus/dbus-server-protected.h
+++ b/dbus/dbus-server-protected.h
@@ -25,6 +25,7 @@
 
 #include <config.h>
 #include <dbus/dbus-internals.h>
+#include <dbus/dbus-threads-internal.h>
 #include <dbus/dbus-server.h>
 #include <dbus/dbus-timeout.h>
 #include <dbus/dbus-watch.h>
@@ -125,14 +126,14 @@ void        _dbus_server_unref_unlocked (DBusServer             *server);
 
 #define SERVER_LOCK(server)   do {                                              \
     if (TRACE_LOCKS) { _dbus_verbose ("  LOCK: %s\n", _DBUS_FUNCTION_NAME); }   \
-    dbus_mutex_lock ((server)->mutex);                                          \
+    _dbus_mutex_lock ((server)->mutex);                                          \
     TOOK_LOCK_CHECK (server);                                                   \
   } while (0)
 
 #define SERVER_UNLOCK(server) do {                                                      \
     if (TRACE_LOCKS) { _dbus_verbose ("  UNLOCK: %s\n", _DBUS_FUNCTION_NAME);  }        \
     RELEASING_LOCK_CHECK (server);                                                      \
-    dbus_mutex_unlock ((server)->mutex);                                                \
+    _dbus_mutex_unlock ((server)->mutex);                                                \
   } while (0)
 
 DBUS_END_DECLS
diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c
index 4a07f149..0aef536e 100644
--- a/dbus/dbus-server-unix.c
+++ b/dbus/dbus-server-unix.c
@@ -61,6 +61,12 @@ unix_finalize (DBusServer *server)
   
   _dbus_server_finalize_base (server);
 
+  if (unix_server->watch)
+    {
+      _dbus_watch_unref (unix_server->watch);
+      unix_server->watch = NULL;
+    }
+  
   dbus_free (unix_server->socket_name);
   dbus_free (server);
 }
@@ -69,6 +75,9 @@ unix_finalize (DBusServer *server)
  * @todo unreffing the connection at the end may cause
  * us to drop the last ref to the connection before
  * disconnecting it. That is invalid.
+ *
+ * @todo doesn't this leak a server refcount if
+ * new_connection_function is NULL?
  */
 /* Return value is just for memory, not other failures. */
 static dbus_bool_t
@@ -200,6 +209,8 @@ unix_disconnect (DBusServer *server)
 {
   DBusServerUnix *unix_server = (DBusServerUnix*) server;
 
+  HAVE_LOCK_CHECK (server);
+  
   if (unix_server->watch)
     {
       _dbus_server_remove_watch (server,
@@ -217,6 +228,8 @@ unix_disconnect (DBusServer *server)
       _dbus_string_init_const (&tmp, unix_server->socket_name);
       _dbus_delete_file (&tmp, NULL);
     }
+
+  HAVE_LOCK_CHECK (server);
 }
 
 static DBusServerVTable unix_vtable = {
@@ -242,12 +255,13 @@ _dbus_server_new_for_fd (int               fd,
                          const DBusString *address)
 {
   DBusServerUnix *unix_server;
+  DBusServer *server;
   DBusWatch *watch;
   
   unix_server = dbus_new0 (DBusServerUnix, 1);
   if (unix_server == NULL)
     return NULL;
-
+  
   watch = _dbus_watch_new (fd,
                            DBUS_WATCH_READABLE,
                            TRUE,
@@ -267,26 +281,25 @@ _dbus_server_new_for_fd (int               fd,
       return NULL;
     }
 
-#ifndef DBUS_DISABLE_CHECKS
-  unix_server->base.have_server_lock = TRUE;
-#endif
+  server = (DBusServer*) unix_server;
+
+  SERVER_LOCK (server);
   
   if (!_dbus_server_add_watch (&unix_server->base,
                                watch))
     {
+      SERVER_UNLOCK (server);
       _dbus_server_finalize_base (&unix_server->base);
       _dbus_watch_unref (watch);
       dbus_free (unix_server);
       return NULL;
     }
-
-#ifndef DBUS_DISABLE_CHECKS
-  unix_server->base.have_server_lock = FALSE;
-#endif
   
   unix_server->fd = fd;
   unix_server->watch = watch;
 
+  SERVER_UNLOCK (server);
+  
   return (DBusServer*) unix_server;
 }
 
diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c
index c11fbd48..156d5ccd 100644
--- a/dbus/dbus-server.c
+++ b/dbus/dbus-server.c
@@ -76,7 +76,7 @@ _dbus_server_init_base (DBusServer             *server,
   if (!_dbus_string_copy_data (address, &server->address))
     goto failed;
 
-  server->mutex = dbus_mutex_new ();
+  server->mutex = _dbus_mutex_new ();
   if (server->mutex == NULL)
     goto failed;
   
@@ -97,7 +97,7 @@ _dbus_server_init_base (DBusServer             *server,
  failed:
   if (server->mutex)
     {
-      dbus_mutex_free (server->mutex);
+      _dbus_mutex_free (server->mutex);
       server->mutex = NULL;
     }
   if (server->watches)
@@ -127,19 +127,24 @@ _dbus_server_init_base (DBusServer             *server,
  */
 void
 _dbus_server_finalize_base (DBusServer *server)
-{  
+{
+  /* We don't have the lock, but nobody should be accessing
+   * concurrently since they don't have a ref
+   */
+#ifndef DBUS_DISABLE_CHECKS
+  _dbus_assert (!server->have_server_lock);
+#endif
+  _dbus_assert (server->disconnected);
+  
   /* calls out to application code... */
   _dbus_data_slot_list_free (&server->slot_list);
 
   dbus_server_set_new_connection_function (server, NULL, NULL, NULL);
 
-  if (!server->disconnected)
-    dbus_server_disconnect (server);
-
   _dbus_watch_list_free (server->watches);
   _dbus_timeout_list_free (server->timeouts);
 
-  dbus_mutex_free (server->mutex);
+  _dbus_mutex_free (server->mutex);
   
   dbus_free (server->address);
 
@@ -168,8 +173,9 @@ protected_change_watch (DBusServer             *server,
   
   HAVE_LOCK_CHECK (server);
 
-  /* This isn't really safe or reasonable; a better pattern is the "do everything, then
-   * drop lock and call out" one; but it has to be propagated up through all callers
+  /* This isn't really safe or reasonable; a better pattern is the "do
+   * everything, then drop lock and call out" one; but it has to be
+   * propagated up through all callers
    */
   
   watches = server->watches;
@@ -213,6 +219,7 @@ dbus_bool_t
 _dbus_server_add_watch (DBusServer *server,
                         DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   return protected_change_watch (server, watch,
                                  _dbus_watch_list_add_watch,
                                  NULL, NULL, FALSE);
@@ -228,6 +235,7 @@ void
 _dbus_server_remove_watch  (DBusServer *server,
                             DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   protected_change_watch (server, watch,
                           NULL,
                           _dbus_watch_list_remove_watch,
@@ -250,6 +258,7 @@ _dbus_server_toggle_watch (DBusServer  *server,
 {
   _dbus_assert (watch != NULL);
 
+  HAVE_LOCK_CHECK (server);
   protected_change_watch (server, watch,
                           NULL, NULL,
                           _dbus_watch_list_toggle_watch,
@@ -595,6 +604,7 @@ DBusServer *
 dbus_server_ref (DBusServer *server)
 {
   _dbus_return_val_if_fail (server != NULL, NULL);
+  _dbus_return_val_if_fail (server->refcount.value > 0, NULL);
 
 #ifdef DBUS_HAVE_ATOMIC_INT
   _dbus_atomic_inc (&server->refcount);
@@ -611,9 +621,9 @@ dbus_server_ref (DBusServer *server)
 
 /**
  * Decrements the reference count of a DBusServer.  Finalizes the
- * server if the reference count reaches zero. The server connection
- * will be closed as with dbus_server_disconnect() when the server is
- * finalized.
+ * server if the reference count reaches zero.
+ *
+ * The server must be disconnected before the refcount reaches zero.
  *
  * @param server the server.
  */
@@ -623,6 +633,7 @@ dbus_server_unref (DBusServer *server)
   dbus_bool_t last_unref;
   
   _dbus_return_if_fail (server != NULL);
+  _dbus_return_if_fail (server->refcount.value > 0);
 
 #ifdef DBUS_HAVE_ATOMIC_INT
   last_unref = (_dbus_atomic_dec (&server->refcount) == 1);
@@ -639,6 +650,9 @@ dbus_server_unref (DBusServer *server)
   
   if (last_unref)
     {
+      /* lock not held! */
+      _dbus_assert (server->disconnected);
+      
       _dbus_assert (server->vtable->finalize != NULL);
       
       (* server->vtable->finalize) (server);
@@ -653,6 +667,9 @@ dbus_server_unref (DBusServer *server)
 void
 _dbus_server_ref_unlocked (DBusServer *server)
 {
+  _dbus_assert (server != NULL);
+  _dbus_assert (server->refcount.value > 0);
+  
   HAVE_LOCK_CHECK (server);
 
 #ifdef DBUS_HAVE_ATOMIC_INT
@@ -675,6 +692,7 @@ _dbus_server_unref_unlocked (DBusServer *server)
   dbus_bool_t last_unref;
   
   _dbus_assert (server != NULL);
+  _dbus_assert (server->refcount.value > 0);
 
   HAVE_LOCK_CHECK (server);
   
@@ -689,6 +707,10 @@ _dbus_server_unref_unlocked (DBusServer *server)
   
   if (last_unref)
     {
+      _dbus_assert (server->disconnected);
+      
+      SERVER_UNLOCK (server);
+      
       _dbus_assert (server->vtable->finalize != NULL);
       
       (* server->vtable->finalize) (server);
@@ -707,21 +729,23 @@ void
 dbus_server_disconnect (DBusServer *server)
 {
   _dbus_return_if_fail (server != NULL);
+  _dbus_return_if_fail (server->refcount.value > 0);
 
   SERVER_LOCK (server);
+  _dbus_server_ref_unlocked (server);
   
   _dbus_assert (server->vtable->disconnect != NULL);
 
-  if (server->disconnected)
+  if (!server->disconnected)
     {
-      SERVER_UNLOCK (server);
-      return;
+      /* this has to be first so recursive calls to disconnect don't happen */
+      server->disconnected = TRUE;
+      
+      (* server->vtable->disconnect) (server);
     }
-  
-  (* server->vtable->disconnect) (server);
-  server->disconnected = TRUE;
 
   SERVER_UNLOCK (server);
+  dbus_server_unref (server);
 }
 
 /**
@@ -1086,6 +1110,7 @@ _dbus_server_test (void)
       if (server == NULL)
 	_dbus_assert_not_reached ("Failed to listen for valid address.");
 
+      dbus_server_disconnect (server);
       dbus_server_unref (server);
 
       /* Try disconnecting before unreffing */
@@ -1094,7 +1119,6 @@ _dbus_server_test (void)
 	_dbus_assert_not_reached ("Failed to listen for valid address.");
 
       dbus_server_disconnect (server);
-
       dbus_server_unref (server);
     }
 
diff --git a/dbus/dbus-signature.c b/dbus/dbus-signature.c
index c984bcfd..698c21ed 100644
--- a/dbus/dbus-signature.c
+++ b/dbus/dbus-signature.c
@@ -97,7 +97,7 @@ dbus_signature_iter_get_element_type (const DBusSignatureIter *iter)
 {
   DBusSignatureRealIter *real_iter = (DBusSignatureRealIter *) iter;
 
-  _dbus_retur(dbus_signature_iter_get_current_type (iter) == DBUS_TYPE_ARRAY);
+  _dbus_return_val_if_fail (dbus_signature_iter_get_current_type (iter) == DBUS_TYPE_ARRAY, DBUS_TYPE_INVALID);
 
   return _dbus_first_type_in_signature_c_str (real_iter->pos, 1);
 }
diff --git a/dbus/dbus-threads.c b/dbus/dbus-threads.c
index 3d1de5ef..0d004d62 100644
--- a/dbus/dbus-threads.c
+++ b/dbus/dbus-threads.c
@@ -22,6 +22,7 @@
  */
 #include "dbus-threads.h"
 #include "dbus-internals.h"
+#include "dbus-threads-internal.h"
 
 static DBusThreadFunctions thread_functions =
 {
@@ -41,9 +42,9 @@ static int thread_init_generation = 0;
 #define _DBUS_DUMMY_CONDVAR ((DBusCondVar*)0xABCDEF2)
 
 /**
- * @defgroup DBusThreads Thread functions
- * @ingroup  DBus
- * @brief dbus_threads_init(), dbus_mutex_lock(), etc.
+ * @defgroup DBusThreadsInternals Thread functions
+ * @ingroup  DBusInternals
+ * @brief _dbus_mutex_lock(), etc.
  *
  * Functions and macros related to threads and thread locks.
  *
@@ -59,7 +60,7 @@ static int thread_init_generation = 0;
  * @returns new mutex or #NULL
  */
 DBusMutex*
-dbus_mutex_new (void)
+_dbus_mutex_new (void)
 {
   if (thread_functions.mutex_new)
     return (* thread_functions.mutex_new) ();
@@ -72,7 +73,7 @@ dbus_mutex_new (void)
  * nothing if passed a #NULL pointer.
  */
 void
-dbus_mutex_free (DBusMutex *mutex)
+_dbus_mutex_free (DBusMutex *mutex)
 {
   if (mutex && thread_functions.mutex_free)
     (* thread_functions.mutex_free) (mutex);
@@ -85,7 +86,7 @@ dbus_mutex_free (DBusMutex *mutex)
  * @returns #TRUE on success
  */
 dbus_bool_t
-dbus_mutex_lock (DBusMutex *mutex)
+_dbus_mutex_lock (DBusMutex *mutex)
 {
   if (mutex && thread_functions.mutex_lock)
     return (* thread_functions.mutex_lock) (mutex);
@@ -99,7 +100,7 @@ dbus_mutex_lock (DBusMutex *mutex)
  * @returns #TRUE on success
  */
 dbus_bool_t
-dbus_mutex_unlock (DBusMutex *mutex)
+_dbus_mutex_unlock (DBusMutex *mutex)
 {
   if (mutex && thread_functions.mutex_unlock)
     return (* thread_functions.mutex_unlock) (mutex);
@@ -116,7 +117,7 @@ dbus_mutex_unlock (DBusMutex *mutex)
  * @returns new mutex or #NULL
  */
 DBusCondVar *
-dbus_condvar_new (void)
+_dbus_condvar_new (void)
 {
   if (thread_functions.condvar_new)
     return (* thread_functions.condvar_new) ();
@@ -129,7 +130,7 @@ dbus_condvar_new (void)
  * nothing if passed a #NULL pointer.
  */
 void
-dbus_condvar_free (DBusCondVar *cond)
+_dbus_condvar_free (DBusCondVar *cond)
 {
   if (cond && thread_functions.condvar_free)
     (* thread_functions.condvar_free) (cond);
@@ -142,8 +143,8 @@ dbus_condvar_free (DBusCondVar *cond)
  * Does nothing if passed a #NULL pointer.
  */
 void
-dbus_condvar_wait (DBusCondVar *cond,
-		   DBusMutex   *mutex)
+_dbus_condvar_wait (DBusCondVar *cond,
+                    DBusMutex   *mutex)
 {
   if (cond && mutex && thread_functions.condvar_wait)
     (* thread_functions.condvar_wait) (cond, mutex);
@@ -162,9 +163,9 @@ dbus_condvar_wait (DBusCondVar *cond,
  * timeout was reached.
  */
 dbus_bool_t
-dbus_condvar_wait_timeout (DBusCondVar               *cond,
-			   DBusMutex                 *mutex,
-			   int                        timeout_milliseconds)
+_dbus_condvar_wait_timeout (DBusCondVar               *cond,
+                            DBusMutex                 *mutex,
+                            int                        timeout_milliseconds)
 {
   if (cond && mutex && thread_functions.condvar_wait)
     return (* thread_functions.condvar_wait_timeout) (cond, mutex, timeout_milliseconds);
@@ -178,7 +179,7 @@ dbus_condvar_wait_timeout (DBusCondVar               *cond,
  * Does nothing if passed a #NULL pointer.
  */
 void
-dbus_condvar_wake_one (DBusCondVar *cond)
+_dbus_condvar_wake_one (DBusCondVar *cond)
 {
   if (cond && thread_functions.condvar_wake_one)
     (* thread_functions.condvar_wake_one) (cond);
@@ -190,7 +191,7 @@ dbus_condvar_wake_one (DBusCondVar *cond)
  * Does nothing if passed a #NULL pointer.
  */
 void
-dbus_condvar_wake_all (DBusCondVar *cond)
+_dbus_condvar_wake_all (DBusCondVar *cond)
 {
   if (cond && thread_functions.condvar_wake_all)
     (* thread_functions.condvar_wake_all) (cond);
@@ -205,7 +206,7 @@ shutdown_global_locks (void *data)
   i = 0;
   while (i < _DBUS_N_GLOBAL_LOCKS)
     {
-      dbus_mutex_free (*(locks[i]));
+      _dbus_mutex_free (*(locks[i]));
       *(locks[i]) = NULL;
       ++i;
     }
@@ -245,7 +246,7 @@ init_global_locks (void)
   
   while (i < _DBUS_N_ELEMENTS (global_locks))
     {
-      *global_locks[i] = dbus_mutex_new ();
+      *global_locks[i] = _dbus_mutex_new ();
       
       if (*global_locks[i] == NULL)
         goto failed;
@@ -266,14 +267,26 @@ init_global_locks (void)
                                      
   for (i = i - 1; i >= 0; i--)
     {
-      dbus_mutex_free (*global_locks[i]);
+      _dbus_mutex_free (*global_locks[i]);
       *global_locks[i] = NULL;
     }
   return FALSE;
 }
 
+/** @} */ /* end of internals */
+
+/**
+ * @defgroup DBusThreads Thread functions
+ * @ingroup  DBus
+ * @brief dbus_threads_init()
+ *
+ * Functions and macros related to threads and thread locks.
+ *
+ * @{
+ */
 
 /**
+ * 
  * Initializes threads. If this function is not called,
  * the D-BUS library will not lock any data structures.
  * If it is called, D-BUS will do locking, at some cost
diff --git a/dbus/dbus-threads.h b/dbus/dbus-threads.h
index 4391ff1d..cf1c01d2 100644
--- a/dbus/dbus-threads.h
+++ b/dbus/dbus-threads.h
@@ -97,26 +97,8 @@ typedef struct
   
 } DBusThreadFunctions;
 
-
-DBusMutex*   dbus_mutex_new            (void);
-void         dbus_mutex_free           (DBusMutex                 *mutex);
-dbus_bool_t  dbus_mutex_lock           (DBusMutex                 *mutex);
-dbus_bool_t  dbus_mutex_unlock         (DBusMutex                 *mutex);
-
-DBusCondVar* dbus_condvar_new          (void);
-void         dbus_condvar_free         (DBusCondVar               *cond);
-void         dbus_condvar_wait         (DBusCondVar               *cond,
-					DBusMutex                 *mutex);
-dbus_bool_t  dbus_condvar_wait_timeout (DBusCondVar               *cond,
-					DBusMutex                 *mutex,
-					int                        timeout_milliseconds);
-void         dbus_condvar_wake_one     (DBusCondVar               *cond);
-void         dbus_condvar_wake_all     (DBusCondVar               *cond);
-
 dbus_bool_t  dbus_threads_init         (const DBusThreadFunctions *functions);
 
-
-
 DBUS_END_DECLS
 
 #endif /* DBUS_THREADS_H */
diff --git a/test/glib/test-profile.c b/test/glib/test-profile.c
index ec0e4fd0..ca4f33c0 100644
--- a/test/glib/test-profile.c
+++ b/test/glib/test-profile.c
@@ -48,7 +48,7 @@
  */
 #define N_CLIENT_THREADS 1
 /* It seems like at least 750000 or so iterations reduces the variability to sane levels */
-#define N_ITERATIONS 750
+#define N_ITERATIONS 750000
 #define N_PROGRESS_UPDATES 20
 /* Don't make PAYLOAD_SIZE too huge because it gets used as a static buffer size */
 #define PAYLOAD_SIZE 0
@@ -303,8 +303,9 @@ no_bus_init_server (ServerData       *sd)
 
 static void
 no_bus_stop_server (ServerData *sd,
-                      void       *server)
+                    void       *server)
 {
+  dbus_server_disconnect (server);
   dbus_server_unref (server);
 }