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

	* dbus/dbus-object-tree.c (handle_default_introspect_and_unlock):
	fix a double-unlock

	* dbus/dbus-connection.c
	(_dbus_connection_detach_pending_call_unlocked): add this

        Initial semi-correct pass through to fix thread locking; there are
	still some issues with the condition variable paths I'm pretty
	sure

	* dbus/dbus-server.c: add a mutex on DBusServer and appropriate
	lock/unlock calls

	* dbus/dbus-connection.c (_dbus_connection_do_iteration_unlocked):
	rename to add _unlocked
	(struct DBusConnection): move "dispatch_acquired" and
	"io_path_acquired" to use only one bit each.
	(CONNECTION_LOCK, CONNECTION_UNLOCK): add checks with !DBUS_DISABLE_CHECKS
	(dbus_connection_set_watch_functions): hacky fix to reentrancy
	(_dbus_connection_add_watch, _dbus_connection_remove_watch)
	(_dbus_connection_toggle_watch, _dbus_connection_add_timeout)
	(_dbus_connection_remove_timeout)
	(_dbus_connection_toggle_timeout): drop lock when calling out to
	user functions; done in a hacky/bad way.
	(_dbus_connection_send_and_unlock): add a missing unlock
	(_dbus_connection_block_for_reply): add a missing unlock

	* dbus/dbus-transport.c (_dbus_transport_get_is_authenticated):
	drop lock in a hacky probably unsafe way to call out to user
	function

8 files changed, 721 insertions, 155 deletions

diff --git a/dbus/dbus-connection-internal.h b/dbus/dbus-connection-internal.h
index 21dc415f..57e6fb0d 100644
--- a/dbus/dbus-connection-internal.h
+++ b/dbus/dbus-connection-internal.h
@@ -74,7 +74,7 @@ void              _dbus_connection_toggle_timeout              (DBusConnection
                                                                 DBusTimeout        *timeout,
                                                                 dbus_bool_t         enabled);
 DBusConnection*   _dbus_connection_new_for_transport           (DBusTransport      *transport);
-void              _dbus_connection_do_iteration                (DBusConnection     *connection,
+void              _dbus_connection_do_iteration_unlocked       (DBusConnection     *connection,
                                                                 unsigned int        flags,
                                                                 int                 timeout_milliseconds);
 
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index dfc5d44b..cac4fc8c 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -1,7 +1,7 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-connection.c DBusConnection object
  *
- * Copyright (C) 2002, 2003, 2004  Red Hat Inc.
+ * Copyright (C) 2002, 2003, 2004, 2005  Red Hat Inc.
  *
  * Licensed under the Academic Free License version 2.1
  * 
@@ -39,20 +39,37 @@
 #include "dbus-pending-call.h"
 #include "dbus-object-tree.h"
 
-#if 0
-#define CONNECTION_LOCK(connection)   do {                      \
-    _dbus_verbose ("  LOCK: %s\n", _DBUS_FUNCTION_NAME);        \
-    dbus_mutex_lock ((connection)->mutex);                      \
+#ifdef DBUS_DISABLE_CHECKS
+#define TOOK_LOCK_CHECK(connection)
+#define RELEASING_LOCK_CHECK(connection)
+#define HAVE_LOCK_CHECK(connection)
+#else
+#define TOOK_LOCK_CHECK(connection) do {                \
+    _dbus_assert (!(connection)->have_connection_lock); \
+    (connection)->have_connection_lock = TRUE;          \
   } while (0)
-#define CONNECTION_UNLOCK(connection) do {                      \
-    _dbus_verbose ("  UNLOCK: %s\n", _DBUS_FUNCTION_NAME);      \
-    dbus_mutex_unlock ((connection)->mutex);                    \
+#define RELEASING_LOCK_CHECK(connection) do {            \
+    _dbus_assert ((connection)->have_connection_lock);   \
+    (connection)->have_connection_lock = FALSE;          \
   } while (0)
-#else
-#define CONNECTION_LOCK(connection)    dbus_mutex_lock ((connection)->mutex)
-#define CONNECTION_UNLOCK(connection)  dbus_mutex_unlock ((connection)->mutex)
+#define HAVE_LOCK_CHECK(connection)        _dbus_assert ((connection)->have_connection_lock)
+/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */
 #endif
 
+#define TRACE_LOCKS 1
+
+#define CONNECTION_LOCK(connection)   do {                                      \
+    if (TRACE_LOCKS) { _dbus_verbose ("  LOCK: %s\n", _DBUS_FUNCTION_NAME); }   \
+    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);                                            \
+  } while (0)
+
 #define DISPATCH_STATUS_NAME(s)                                            \
                      ((s) == DBUS_DISPATCH_COMPLETE ? "complete" :         \
                       (s) == DBUS_DISPATCH_DATA_REMAINS ? "data remains" : \
@@ -171,10 +188,7 @@ struct DBusConnection
 
   DBusMutex *mutex; /**< Lock on the entire DBusConnection */
 
-  dbus_bool_t dispatch_acquired; /**< Protects dispatch() */
   DBusCondVar *dispatch_cond;    /**< Protects dispatch() */
-
-  dbus_bool_t io_path_acquired;  /**< Protects transport io path */
   DBusCondVar *io_path_cond;     /**< Protects transport io path */
   
   DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */
@@ -215,9 +229,16 @@ struct DBusConnection
                          *   for the global linked list mempool lock
                          */
   DBusObjectTree *objects; /**< Object path handlers registered with this connection */
-
+  
+  unsigned int dispatch_acquired : 1; /**< Someone has dispatch path */
+  unsigned int io_path_acquired : 1;  /**< Someone has transport io path */
+  
   unsigned int exit_on_disconnect : 1; /**< If #TRUE, exit after handling disconnect signal */
-
+  
+#ifndef DBUS_DISABLE_CHECKS
+  unsigned int have_connection_lock : 1; /**< Used to check locking */
+#endif
+  
 #ifndef DBUS_DISABLE_CHECKS
   int generation; /**< _dbus_current_generation that should correspond to this connection */
 #endif 
@@ -387,6 +408,8 @@ static void
 _dbus_connection_queue_synthesized_message_link (DBusConnection *connection,
 						 DBusList *link)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   _dbus_list_append_link (&connection->incoming_messages, link);
 
   connection->n_incoming += 1;
@@ -408,6 +431,7 @@ _dbus_connection_queue_synthesized_message_link (DBusConnection *connection,
 dbus_bool_t
 _dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
   return connection->outgoing_messages != NULL;
 }
 
@@ -441,6 +465,8 @@ dbus_connection_has_messages_to_send (DBusConnection *connection)
 DBusMessage*
 _dbus_connection_get_message_to_send (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   return _dbus_list_get_last (&connection->outgoing_messages);
 }
 
@@ -458,6 +484,8 @@ _dbus_connection_message_sent (DBusConnection *connection,
 {
   DBusList *link;
 
+  HAVE_LOCK_CHECK (connection);
+  
   /* This can be called before we even complete authentication, since
    * it's called on disconnect to clean up the outgoing queue.
    * It's also called as we successfully send each message.
@@ -495,6 +523,62 @@ _dbus_connection_message_sent (DBusConnection *connection,
   dbus_message_unref (message);
 }
 
+typedef dbus_bool_t (* DBusWatchAddFunction)     (DBusWatchList *list,
+                                                  DBusWatch     *watch);
+typedef void        (* DBusWatchRemoveFunction)  (DBusWatchList *list,
+                                                  DBusWatch     *watch);
+typedef void        (* DBusWatchToggleFunction)  (DBusWatchList *list,
+                                                  DBusWatch     *watch,
+                                                  dbus_bool_t    enabled);
+
+static dbus_bool_t
+protected_change_watch (DBusConnection         *connection,
+                        DBusWatch              *watch,
+                        DBusWatchAddFunction    add_function,
+                        DBusWatchRemoveFunction remove_function,
+                        DBusWatchToggleFunction toggle_function,
+                        dbus_bool_t             enabled)
+{
+  DBusWatchList *watches;
+  dbus_bool_t retval;
+  
+  HAVE_LOCK_CHECK (connection);
+
+  /* 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 = connection->watches;
+  if (watches)
+    {
+      connection->watches = NULL;
+      _dbus_connection_ref_unlocked (connection);
+      CONNECTION_UNLOCK (connection);
+
+      if (add_function)
+        retval = (* add_function) (watches, watch);
+      else if (remove_function)
+        {
+          retval = TRUE;
+          (* remove_function) (watches, watch);
+        }
+      else
+        {
+          retval = TRUE;
+          (* toggle_function) (watches, watch, enabled);
+        }
+      
+      CONNECTION_LOCK (connection);
+      connection->watches = watches;
+      _dbus_connection_unref_unlocked (connection);
+
+      return retval;
+    }
+  else
+    return FALSE;
+}
+     
+
 /**
  * Adds a watch using the connection's DBusAddWatchFunction if
  * available. Otherwise records the watch to be added when said
@@ -509,11 +593,9 @@ dbus_bool_t
 _dbus_connection_add_watch (DBusConnection *connection,
                             DBusWatch      *watch)
 {
-  if (connection->watches) /* null during finalize */
-    return _dbus_watch_list_add_watch (connection->watches,
-                                       watch);
-  else
-    return FALSE;
+  return protected_change_watch (connection, watch,
+                                 _dbus_watch_list_add_watch,
+                                 NULL, NULL, FALSE);
 }
 
 /**
@@ -528,9 +610,10 @@ void
 _dbus_connection_remove_watch (DBusConnection *connection,
                                DBusWatch      *watch)
 {
-  if (connection->watches) /* null during finalize */
-    _dbus_watch_list_remove_watch (connection->watches,
-                                   watch);
+  protected_change_watch (connection, watch,
+                          NULL,
+                          _dbus_watch_list_remove_watch,
+                          NULL, FALSE);
 }
 
 /**
@@ -549,10 +632,66 @@ _dbus_connection_toggle_watch (DBusConnection *connection,
                                dbus_bool_t     enabled)
 {
   _dbus_assert (watch != NULL);
+
+  protected_change_watch (connection, watch,
+                          NULL, NULL,
+                          _dbus_watch_list_toggle_watch,
+                          enabled);
+}
+
+typedef dbus_bool_t (* DBusTimeoutAddFunction)    (DBusTimeoutList *list,
+                                                   DBusTimeout     *timeout);
+typedef void        (* DBusTimeoutRemoveFunction) (DBusTimeoutList *list,
+                                                   DBusTimeout     *timeout);
+typedef void        (* DBusTimeoutToggleFunction) (DBusTimeoutList *list,
+                                                   DBusTimeout     *timeout,
+                                                   dbus_bool_t      enabled);
+
+static dbus_bool_t
+protected_change_timeout (DBusConnection           *connection,
+                          DBusTimeout              *timeout,
+                          DBusTimeoutAddFunction    add_function,
+                          DBusTimeoutRemoveFunction remove_function,
+                          DBusTimeoutToggleFunction toggle_function,
+                          dbus_bool_t               enabled)
+{
+  DBusTimeoutList *timeouts;
+  dbus_bool_t retval;
+  
+  HAVE_LOCK_CHECK (connection);
+
+  /* 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
+   */
   
-  if (connection->watches) /* null during finalize */
-    _dbus_watch_list_toggle_watch (connection->watches,
-                                   watch, enabled);
+  timeouts = connection->timeouts;
+  if (timeouts)
+    {
+      connection->timeouts = NULL;
+      _dbus_connection_ref_unlocked (connection);
+      CONNECTION_UNLOCK (connection);
+
+      if (add_function)
+        retval = (* add_function) (timeouts, timeout);
+      else if (remove_function)
+        {
+          retval = TRUE;
+          (* remove_function) (timeouts, timeout);
+        }
+      else
+        {
+          retval = TRUE;
+          (* toggle_function) (timeouts, timeout, enabled);
+        }
+      
+      CONNECTION_LOCK (connection);
+      connection->timeouts = timeouts;
+      _dbus_connection_unref_unlocked (connection);
+
+      return retval;
+    }
+  else
+    return FALSE;
 }
 
 /**
@@ -570,11 +709,9 @@ dbus_bool_t
 _dbus_connection_add_timeout (DBusConnection *connection,
 			      DBusTimeout    *timeout)
 {
- if (connection->timeouts) /* null during finalize */
-    return _dbus_timeout_list_add_timeout (connection->timeouts,
-					   timeout);
-  else
-    return FALSE;  
+  return protected_change_timeout (connection, timeout,
+                                   _dbus_timeout_list_add_timeout,
+                                   NULL, NULL, FALSE);
 }
 
 /**
@@ -589,9 +726,10 @@ void
 _dbus_connection_remove_timeout (DBusConnection *connection,
 				 DBusTimeout    *timeout)
 {
-  if (connection->timeouts) /* null during finalize */
-    _dbus_timeout_list_remove_timeout (connection->timeouts,
-				       timeout);
+  protected_change_timeout (connection, timeout,
+                            NULL,
+                            _dbus_timeout_list_remove_timeout,
+                            NULL, FALSE);
 }
 
 /**
@@ -604,19 +742,22 @@ _dbus_connection_remove_timeout (DBusConnection *connection,
  * @param enabled whether to enable or disable
  */
 void
-_dbus_connection_toggle_timeout (DBusConnection *connection,
+_dbus_connection_toggle_timeout (DBusConnection   *connection,
                                  DBusTimeout      *timeout,
-                                 dbus_bool_t     enabled)
+                                 dbus_bool_t       enabled)
 {
-  if (connection->timeouts) /* null during finalize */
-    _dbus_timeout_list_toggle_timeout (connection->timeouts,
-                                       timeout, enabled);
+  protected_change_timeout (connection, timeout,
+                            NULL, NULL,
+                            _dbus_timeout_list_toggle_timeout,
+                            enabled);
 }
 
 static dbus_bool_t
 _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
                                                DBusPendingCall *pending)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   _dbus_assert (pending->reply_serial != 0);
 
   if (!_dbus_connection_add_timeout (connection, pending->timeout))
@@ -627,6 +768,8 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
                                     pending))
     {
       _dbus_connection_remove_timeout (connection, pending->timeout);
+
+      HAVE_LOCK_CHECK (connection);
       return FALSE;
     }
   
@@ -634,6 +777,8 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
   pending->connection = connection;
 
   dbus_pending_call_ref (pending);
+
+  HAVE_LOCK_CHECK (connection);
   
   return TRUE;
 }
@@ -664,6 +809,19 @@ free_pending_call_on_hash_removal (void *data)
 }
 
 static void
+_dbus_connection_detach_pending_call_unlocked (DBusConnection  *connection,
+                                               DBusPendingCall *pending)
+{
+  /* Can't have a destroy notifier on the pending call if we're going to do this */
+
+  dbus_pending_call_ref (pending);
+  _dbus_hash_table_remove_int (connection->pending_replies,
+                               pending->reply_serial);
+  _dbus_assert (pending->connection == NULL);
+  dbus_pending_call_unref (pending);
+}
+
+static void
 _dbus_connection_detach_pending_call_and_unlock (DBusConnection  *connection,
                                                  DBusPendingCall *pending)
 {
@@ -674,6 +832,7 @@ _dbus_connection_detach_pending_call_and_unlock (DBusConnection  *connection,
   dbus_pending_call_ref (pending);
   _dbus_hash_table_remove_int (connection->pending_replies,
                                pending->reply_serial);
+  _dbus_assert (pending->connection == NULL);
   CONNECTION_UNLOCK (connection);
   dbus_pending_call_unref (pending);
 }
@@ -749,14 +908,25 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
 {
   dbus_bool_t res = TRUE;
 
+  _dbus_verbose ("%s start connection->io_path_acquired = %d timeout = %d\n",
+                 _DBUS_FUNCTION_NAME, connection->io_path_acquired, timeout_milliseconds);
+  
   if (connection->io_path_acquired)
     {
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = FALSE;
+#endif
+      
       if (timeout_milliseconds != -1) 
 	res = dbus_condvar_wait_timeout (connection->io_path_cond,
 					 connection->mutex,
 					 timeout_milliseconds);
       else
 	dbus_condvar_wait (connection->io_path_cond, connection->mutex);
+
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = TRUE;
+#endif
     }
   
   if (res)
@@ -765,6 +935,9 @@ _dbus_connection_acquire_io_path (DBusConnection *connection,
 
       connection->io_path_acquired = TRUE;
     }
+
+  _dbus_verbose ("%s end connection->io_path_acquired = %d res = %d\n",
+                 _DBUS_FUNCTION_NAME, connection->io_path_acquired, res);
   
   return res;
 }
@@ -781,6 +954,9 @@ _dbus_connection_release_io_path (DBusConnection *connection)
 {
   _dbus_assert (connection->io_path_acquired);
 
+  _dbus_verbose ("%s start connection->io_path_acquired = %d\n",
+                 _DBUS_FUNCTION_NAME, connection->io_path_acquired);
+  
   connection->io_path_acquired = FALSE;
   dbus_condvar_wake_one (connection->io_path_cond);
 }
@@ -789,7 +965,7 @@ _dbus_connection_release_io_path (DBusConnection *connection)
 /**
  * Queues incoming messages and sends outgoing messages for this
  * connection, optionally blocking in the process. Each call to
- * _dbus_connection_do_iteration() will call select() or poll() one
+ * _dbus_connection_do_iteration_unlocked() will call select() or poll() one
  * time and then read or write data if possible.
  *
  * The purpose of this function is to be able to flush outgoing
@@ -815,10 +991,14 @@ _dbus_connection_release_io_path (DBusConnection *connection)
  * @param timeout_milliseconds maximum blocking time, or -1 for no limit.
  */
 void
-_dbus_connection_do_iteration (DBusConnection *connection,
-                               unsigned int    flags,
-                               int             timeout_milliseconds)
+_dbus_connection_do_iteration_unlocked (DBusConnection *connection,
+                                        unsigned int    flags,
+                                        int             timeout_milliseconds)
 {
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+  
+  HAVE_LOCK_CHECK (connection);
+  
   if (connection->n_outgoing == 0)
     flags &= ~DBUS_ITERATION_DO_WRITING;
 
@@ -829,6 +1009,8 @@ _dbus_connection_do_iteration (DBusConnection *connection,
 				    flags, timeout_milliseconds);
       _dbus_connection_release_io_path (connection);
     }
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 /**
@@ -949,11 +1131,15 @@ _dbus_connection_new_for_transport (DBusTransport *transport)
   connection->client_serial = 1;
 
   connection->disconnect_message_link = disconnect_link;
+
+  CONNECTION_LOCK (connection);
   
   if (!_dbus_transport_set_connection (transport, connection))
     goto error;
 
-  _dbus_transport_ref (transport);  
+  _dbus_transport_ref (transport);
+
+  CONNECTION_UNLOCK (connection);
   
   return connection;
   
@@ -1006,9 +1192,11 @@ _dbus_connection_new_for_transport (DBusTransport *transport)
  */
 DBusConnection *
 _dbus_connection_ref_unlocked (DBusConnection *connection)
-{
+{  
   _dbus_assert (connection != NULL);
   _dbus_assert (connection->generation == _dbus_current_generation);
+
+  HAVE_LOCK_CHECK (connection);
   
 #ifdef DBUS_HAVE_ATOMIC_INT
   _dbus_atomic_inc (&connection->refcount);
@@ -1031,7 +1219,9 @@ _dbus_connection_unref_unlocked (DBusConnection *connection)
 {
   dbus_bool_t last_unref;
 
-  _dbus_return_if_fail (connection != NULL);
+  HAVE_LOCK_CHECK (connection);
+  
+  _dbus_assert (connection != NULL);
 
   /* The connection lock is better than the global
    * lock in the atomic increment fallback
@@ -1089,6 +1279,8 @@ _dbus_connection_handle_watch (DBusWatch                   *watch,
   DBusDispatchStatus status;
 
   connection = data;
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   CONNECTION_LOCK (connection);
   _dbus_connection_acquire_io_path (connection, -1);
@@ -1096,10 +1288,16 @@ _dbus_connection_handle_watch (DBusWatch                   *watch,
                                          watch, condition);
   _dbus_connection_release_io_path (connection);
 
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
+  
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* this calls out to user code */
   _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
   
   return retval;
 }
@@ -1154,7 +1352,10 @@ dbus_connection_open (const char     *address,
       dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
       return NULL;
     }
-  
+
+#ifndef DBUS_DISABLE_CHECKS
+  _dbus_assert (!connection->have_connection_lock);
+#endif
   return connection;
 }
 
@@ -1357,7 +1558,8 @@ dbus_connection_disconnect (DBusConnection *connection)
   
   CONNECTION_LOCK (connection);
   _dbus_transport_disconnect (connection->transport);
-  
+
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* this calls out to user code */
@@ -1367,6 +1569,7 @@ dbus_connection_disconnect (DBusConnection *connection)
 static dbus_bool_t
 _dbus_connection_get_is_connected_unlocked (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
   return _dbus_transport_get_is_connected (connection->transport);
 }
 
@@ -1445,6 +1648,8 @@ _dbus_connection_preallocate_send_unlocked (DBusConnection *connection)
 {
   DBusPreallocatedSend *preallocated;
 
+  HAVE_LOCK_CHECK (connection);
+  
   _dbus_assert (connection != NULL);
   
   preallocated = dbus_new (DBusPreallocatedSend, 1);
@@ -1604,9 +1809,9 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection       *con
   /* Now we need to run an iteration to hopefully just write the messages
    * out immediately, and otherwise get them queued up
    */
-  _dbus_connection_do_iteration (connection,
-                                 DBUS_ITERATION_DO_WRITING,
-                                 -1);
+  _dbus_connection_do_iteration_unlocked (connection,
+                                          DBUS_ITERATION_DO_WRITING,
+                                          -1);
 
   /* If stuff is still queued up, be sure we wake up the main loop */
   if (connection->n_outgoing > 0)
@@ -1621,10 +1826,13 @@ _dbus_connection_send_preallocated_and_unlock (DBusConnection       *connection,
 {
   DBusDispatchStatus status;
 
+  HAVE_LOCK_CHECK (connection);
+  
   _dbus_connection_send_preallocated_unlocked_no_update (connection,
                                                          preallocated,
                                                          message, client_serial);
 
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* this calls out to user code */
@@ -1699,7 +1907,10 @@ _dbus_connection_send_and_unlock (DBusConnection *connection,
   
   preallocated = _dbus_connection_preallocate_send_unlocked (connection);
   if (preallocated == NULL)
-    return FALSE;
+    {
+      CONNECTION_UNLOCK (connection);
+      return FALSE;
+    }
 
   _dbus_connection_send_preallocated_and_unlock (connection,
 						 preallocated,
@@ -1762,6 +1973,7 @@ reply_handler_timeout (void *data)
 				   pending->timeout);
   pending->timeout_added = FALSE;
 
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* Unlocks, and calls out to user code */
@@ -1877,8 +2089,12 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
   if (pending_return)
     *pending_return = pending;
   else
-    dbus_pending_call_unref (pending);
+    {
+      _dbus_connection_detach_pending_call_unlocked (connection, pending);
+      dbus_pending_call_unref (pending);
+    }
 
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* this calls out to user code */
@@ -1898,6 +2114,8 @@ check_for_reply_unlocked (DBusConnection *connection,
                           dbus_uint32_t   client_serial)
 {
   DBusList *link;
+
+  HAVE_LOCK_CHECK (connection);
   
   link = _dbus_list_get_first_link (&connection->incoming_messages);
 
@@ -1980,14 +2198,19 @@ _dbus_connection_block_for_reply (DBusConnection     *connection,
    * gets the message before we do?
    */
   /* always block at least once as we know we don't have the reply yet */
-  _dbus_connection_do_iteration (connection,
-                                 DBUS_ITERATION_DO_READING |
-                                 DBUS_ITERATION_BLOCK,
-                                 timeout_milliseconds);
+  _dbus_connection_do_iteration_unlocked (connection,
+                                          DBUS_ITERATION_DO_READING |
+                                          DBUS_ITERATION_BLOCK,
+                                          timeout_milliseconds);
 
  recheck_status:
 
+  _dbus_verbose ("%s top of recheck\n", _DBUS_FUNCTION_NAME);
+  
+  HAVE_LOCK_CHECK (connection);
+  
   /* queue messages and get status */
+
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   if (status == DBUS_DISPATCH_DATA_REMAINS)
@@ -1996,7 +2219,8 @@ _dbus_connection_block_for_reply (DBusConnection     *connection,
       
       reply = check_for_reply_unlocked (connection, client_serial);
       if (reply != NULL)
-        {          
+        {
+          _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
           status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
           _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
@@ -2011,7 +2235,10 @@ _dbus_connection_block_for_reply (DBusConnection     *connection,
   _dbus_get_current_time (&tv_sec, &tv_usec);
   
   if (!_dbus_connection_get_is_connected_unlocked (connection))
-    return NULL;
+    {
+      CONNECTION_UNLOCK (connection);
+      return NULL;
+    }
   else if (tv_sec < start_tv_sec)
     _dbus_verbose ("dbus_connection_send_with_reply_and_block(): clock set backward\n");
   else if (connection->disconnect_message_link == NULL)
@@ -2042,10 +2269,10 @@ _dbus_connection_block_for_reply (DBusConnection     *connection,
       else
         {          
           /* block again, we don't have the reply buffered yet. */
-          _dbus_connection_do_iteration (connection,
-                                         DBUS_ITERATION_DO_READING |
-                                         DBUS_ITERATION_BLOCK,
-                                         timeout_milliseconds);
+          _dbus_connection_do_iteration_unlocked (connection,
+                                                  DBUS_ITERATION_DO_READING |
+                                                  DBUS_ITERATION_BLOCK,
+                                                  timeout_milliseconds);
         }
 
       goto recheck_status;
@@ -2144,16 +2371,25 @@ dbus_connection_flush (DBusConnection *connection)
   CONNECTION_LOCK (connection);
   while (connection->n_outgoing > 0 &&
          _dbus_connection_get_is_connected_unlocked (connection))
-    _dbus_connection_do_iteration (connection,
-                                   DBUS_ITERATION_DO_READING |
-                                   DBUS_ITERATION_DO_WRITING |
-                                   DBUS_ITERATION_BLOCK,
-                                   -1);
+    {
+      _dbus_verbose ("doing iteration in %s\n", _DBUS_FUNCTION_NAME);
+      HAVE_LOCK_CHECK (connection);
+      _dbus_connection_do_iteration_unlocked (connection,
+                                              DBUS_ITERATION_DO_READING |
+                                              DBUS_ITERATION_DO_WRITING |
+                                              DBUS_ITERATION_BLOCK,
+                                              -1);
+    }
 
+  HAVE_LOCK_CHECK (connection);
+  _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
+  HAVE_LOCK_CHECK (connection);
   /* Unlocks and calls out to user code */
   _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 /* Call with mutex held. Will drop it while waiting and re-acquire
@@ -2165,7 +2401,15 @@ _dbus_connection_wait_for_borrowed (DBusConnection *connection)
   _dbus_assert (connection->message_borrowed != NULL);
 
   while (connection->message_borrowed != NULL)
-    dbus_condvar_wait (connection->message_returned_cond, connection->mutex);
+    {
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = FALSE;
+#endif
+      dbus_condvar_wait (connection->message_returned_cond, connection->mutex);
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = TRUE;
+#endif
+    }
 }
 
 /**
@@ -2191,6 +2435,8 @@ dbus_connection_borrow_message  (DBusConnection *connection)
   _dbus_return_val_if_fail (connection != NULL, NULL);
   /* can't borrow during dispatch */
   _dbus_return_val_if_fail (!connection->dispatch_acquired, NULL);
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   /* this is called for the side effect that it queues
    * up any messages from the transport
@@ -2283,6 +2529,8 @@ dbus_connection_steal_borrowed_message (DBusConnection *connection,
 static DBusList*
 _dbus_connection_pop_message_link_unlocked (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   if (connection->message_borrowed != NULL)
     _dbus_connection_wait_for_borrowed (connection);
   
@@ -2319,6 +2567,8 @@ static DBusMessage*
 _dbus_connection_pop_message_unlocked (DBusConnection *connection)
 {
   DBusList *link;
+
+  HAVE_LOCK_CHECK (connection);
   
   link = _dbus_connection_pop_message_link_unlocked (connection);
 
@@ -2340,6 +2590,8 @@ static void
 _dbus_connection_putback_message_link_unlocked (DBusConnection *connection,
                                                 DBusList       *message_link)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   _dbus_assert (message_link != NULL);
   /* You can't borrow a message while a link is outstanding */
   _dbus_assert (connection->message_borrowed == NULL);
@@ -2381,6 +2633,8 @@ dbus_connection_pop_message (DBusConnection *connection)
   DBusMessage *message;
   DBusDispatchStatus status;
 
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+  
   /* this is called for the side effect that it queues
    * up any messages from the transport
    */
@@ -2411,7 +2665,15 @@ static void
 _dbus_connection_acquire_dispatch (DBusConnection *connection)
 {
   if (connection->dispatch_acquired)
-    dbus_condvar_wait (connection->dispatch_cond, connection->mutex);
+    {
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = FALSE;
+#endif
+      dbus_condvar_wait (connection->dispatch_cond, connection->mutex);
+#ifndef DBUS_DISABLE_CHECKS
+      connection->have_connection_lock = TRUE;
+#endif
+    }
   _dbus_assert (!connection->dispatch_acquired);
 
   connection->dispatch_acquired = TRUE;
@@ -2445,6 +2707,8 @@ _dbus_connection_failed_pop (DBusConnection *connection,
 static DBusDispatchStatus
 _dbus_connection_get_dispatch_status_unlocked (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   if (connection->n_incoming > 0)
     return DBUS_DISPATCH_DATA_REMAINS;
   else if (!_dbus_transport_queue_messages (connection->transport))
@@ -2511,7 +2775,7 @@ _dbus_connection_update_dispatch_status_and_unlock (DBusConnection    *connectio
   DBusDispatchStatusFunction function;
   void *data;
 
-  /* We have the lock */
+  HAVE_LOCK_CHECK (connection);
 
   _dbus_connection_ref_unlocked (connection);
 
@@ -2550,6 +2814,8 @@ dbus_connection_get_dispatch_status (DBusConnection *connection)
   DBusDispatchStatus status;
 
   _dbus_return_val_if_fail (connection != NULL, DBUS_DISPATCH_COMPLETE);
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   CONNECTION_LOCK (connection);
 
@@ -2759,7 +3025,8 @@ dbus_connection_dispatch (DBusConnection *connection)
                  dbus_message_get_member (message) :
                  "no member",
                  dbus_message_get_signature (message));
-  
+
+  HAVE_LOCK_CHECK (connection);
   result = _dbus_object_tree_dispatch_and_unlock (connection->objects,
                                                   message);
   
@@ -2874,7 +3141,8 @@ dbus_connection_dispatch (DBusConnection *connection)
     }
   
   _dbus_connection_release_dispatch (connection);
-  
+
+  _dbus_verbose ("%s before final status update\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   /* unlocks and calls user code */
@@ -2952,25 +3220,43 @@ dbus_connection_set_watch_functions (DBusConnection              *connection,
                                      DBusFreeFunction             free_data_function)
 {
   dbus_bool_t retval;
+  DBusWatchList *watches;
 
   _dbus_return_val_if_fail (connection != NULL, FALSE);
   
   CONNECTION_LOCK (connection);
+
+#ifndef DBUS_DISABLE_CHECKS
+  if (connection->watches == NULL)
+    {
+      _dbus_warn ("Re-entrant call to %s is not allowed\n",
+                  _DBUS_FUNCTION_NAME);
+      return FALSE;
+    }
+#endif
+  
   /* ref connection for slightly better reentrancy */
   _dbus_connection_ref_unlocked (connection);
 
-  /* FIXME this can call back into user code, and we need to drop the
-   * connection lock when it does.
+  /* This can call back into user code, and we need to drop the
+   * connection lock when it does. This is kind of a lame
+   * way to do it.
    */
-  retval = _dbus_watch_list_set_functions (connection->watches,
+  watches = connection->watches;
+  connection->watches = NULL;
+  CONNECTION_UNLOCK (connection);
+
+  retval = _dbus_watch_list_set_functions (watches,
                                            add_function, remove_function,
                                            toggled_function,
                                            data, free_data_function);
+  CONNECTION_LOCK (connection);
+  connection->watches = watches;
   
   CONNECTION_UNLOCK (connection);
   /* drop our paranoid refcount */
   dbus_connection_unref (connection);
-
+  
   return retval;
 }
 
@@ -3016,17 +3302,34 @@ dbus_connection_set_timeout_functions   (DBusConnection            *connection,
 					 DBusFreeFunction           free_data_function)
 {
   dbus_bool_t retval;
+  DBusTimeoutList *timeouts;
 
   _dbus_return_val_if_fail (connection != NULL, FALSE);
   
   CONNECTION_LOCK (connection);
+
+#ifndef DBUS_DISABLE_CHECKS
+  if (connection->timeouts == NULL)
+    {
+      _dbus_warn ("Re-entrant call to %s is not allowed\n",
+                  _DBUS_FUNCTION_NAME);
+      return FALSE;
+    }
+#endif
+  
   /* ref connection for slightly better reentrancy */
   _dbus_connection_ref_unlocked (connection);
+
+  timeouts = connection->timeouts;
+  connection->timeouts = NULL;
+  CONNECTION_UNLOCK (connection);
   
-  retval = _dbus_timeout_list_set_functions (connection->timeouts,
+  retval = _dbus_timeout_list_set_functions (timeouts,
                                              add_function, remove_function,
                                              toggled_function,
                                              data, free_data_function);
+  CONNECTION_LOCK (connection);
+  connection->timeouts = timeouts;
   
   CONNECTION_UNLOCK (connection);
   /* drop our paranoid refcount */
diff --git a/dbus/dbus-object-tree.c b/dbus/dbus-object-tree.c
index ae28cc1c..b693b919 100644
--- a/dbus/dbus-object-tree.c
+++ b/dbus/dbus-object-tree.c
@@ -1,7 +1,7 @@
 /* -*- mode: C; c-file-style: "gnu" -*- */
 /* dbus-object-tree.c  DBusObjectTree (internals of DBusConnection)
  *
- * Copyright (C) 2003  Red Hat Inc.
+ * Copyright (C) 2003, 2005  Red Hat Inc.
  *
  * Licensed under the Academic Free License version 2.1
  *
@@ -503,6 +503,7 @@ _dbus_object_tree_unregister_and_unlock (DBusObjectTree          *tree,
 #endif
     {
       _dbus_connection_ref_unlocked (connection);
+      _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
       _dbus_connection_unlock (connection);
     }
 
@@ -635,7 +636,10 @@ handle_default_introspect_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
       if (tree->connection)
 #endif
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+          _dbus_connection_unlock (tree->connection);
+        }
       
       return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
     }
@@ -647,7 +651,10 @@ handle_default_introspect_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
       if (tree->connection)
 #endif
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+          _dbus_connection_unlock (tree->connection);
+        }
 
       return DBUS_HANDLER_RESULT_NEED_MEMORY;
     }
@@ -690,6 +697,8 @@ handle_default_introspect_and_unlock (DBusObjectTree          *tree,
   if (tree->connection)
 #endif
     {
+      already_unlocked = TRUE;
+      
       if (!_dbus_connection_send_and_unlock (tree->connection, reply, NULL))
         goto out;
     }
@@ -702,7 +711,10 @@ handle_default_introspect_and_unlock (DBusObjectTree          *tree,
 #endif
     {
       if (!already_unlocked)
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s %d\n", _DBUS_FUNCTION_NAME, __LINE__);
+          _dbus_connection_unlock (tree->connection);
+        }
     }
   
   _dbus_string_free (&xml);
@@ -747,7 +759,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
       if (tree->connection)
 #endif
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+          _dbus_connection_unlock (tree->connection);
+        }
       
       _dbus_verbose ("No memory to get decomposed path\n");
 
@@ -759,7 +774,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
       if (tree->connection)
 #endif
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+          _dbus_connection_unlock (tree->connection);
+        }
       
       _dbus_verbose ("No path field in message\n");
       return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -822,7 +840,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
           if (tree->connection)
 #endif
-            _dbus_connection_unlock (tree->connection);
+            {
+              _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+              _dbus_connection_unlock (tree->connection);
+            }
 
           /* FIXME you could unregister the subtree in another thread
            * before we invoke the callback, and I can't figure out a
@@ -859,7 +880,10 @@ _dbus_object_tree_dispatch_and_unlock (DBusObjectTree          *tree,
 #ifdef DBUS_BUILD_TESTS
       if (tree->connection)
 #endif
-        _dbus_connection_unlock (tree->connection);
+        {
+          _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+          _dbus_connection_unlock (tree->connection);
+        }
     }
   
   while (list != NULL)
@@ -993,7 +1017,10 @@ _dbus_object_tree_list_registered_and_unlock (DBusObjectTree *tree,
 #ifdef DBUS_BUILD_TESTS
   if (tree->connection)
 #endif
-    _dbus_connection_unlock (tree->connection);
+    {
+      _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+      _dbus_connection_unlock (tree->connection);
+    }
 
   return result;
 }
diff --git a/dbus/dbus-server-protected.h b/dbus/dbus-server-protected.h
index bce29ca8..c8aa8601 100644
--- a/dbus/dbus-server-protected.h
+++ b/dbus/dbus-server-protected.h
@@ -23,6 +23,7 @@
 #ifndef DBUS_SERVER_PROTECTED_H
 #define DBUS_SERVER_PROTECTED_H
 
+#include <config.h>
 #include <dbus/dbus-internals.h>
 #include <dbus/dbus-server.h>
 #include <dbus/dbus-timeout.h>
@@ -51,8 +52,9 @@ struct DBusServerVTable
  */
 struct DBusServer
 {
-  int refcount;                               /**< Reference count. */
+  DBusAtomic refcount;                        /**< Reference count. */
   const DBusServerVTable *vtable;             /**< Virtual methods for this instance. */
+  DBusMutex *mutex;                           /**< Lock on the server object */
   DBusWatchList *watches;                     /**< Our watches */
   DBusTimeoutList *timeouts;                  /**< Our timeouts */  
 
@@ -74,6 +76,10 @@ struct DBusServer
   char **auth_mechanisms; /**< Array of allowed authentication mechanisms */
   
   unsigned int disconnected : 1;              /**< TRUE if we are disconnected. */
+
+#ifndef DBUS_DISABLE_CHECKS
+  unsigned int have_server_lock : 1; /**< Does someone have the server mutex locked */
+#endif
 };
 
 dbus_bool_t _dbus_server_init_base      (DBusServer             *server,
@@ -95,7 +101,38 @@ void        _dbus_server_toggle_timeout (DBusServer             *server,
                                          DBusTimeout            *timeout,
                                          dbus_bool_t             enabled);
 
+void        _dbus_server_ref_unlocked   (DBusServer             *server);
+
+#ifdef DBUS_DISABLE_CHECKS
+#define TOOK_LOCK_CHECK(server)
+#define RELEASING_LOCK_CHECK(server)
+#define HAVE_LOCK_CHECK(server)
+#else
+#define TOOK_LOCK_CHECK(server) do {                \
+    _dbus_assert (!(server)->have_server_lock); \
+    (server)->have_server_lock = TRUE;          \
+  } while (0)
+#define RELEASING_LOCK_CHECK(server) do {            \
+    _dbus_assert ((server)->have_server_lock);   \
+    (server)->have_server_lock = FALSE;          \
+  } while (0)
+#define HAVE_LOCK_CHECK(server)        _dbus_assert ((server)->have_server_lock)
+/* A "DO_NOT_HAVE_LOCK_CHECK" is impossible since we need the lock to check the flag */
+#endif
+
+#define TRACE_LOCKS 0
+
+#define SERVER_LOCK(server)   do {                                              \
+    if (TRACE_LOCKS) { _dbus_verbose ("  LOCK: %s\n", _DBUS_FUNCTION_NAME); }   \
+    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);                                                \
+  } while (0)
 
 DBUS_END_DECLS
 
diff --git a/dbus/dbus-server-unix.c b/dbus/dbus-server-unix.c
index f576b427..4a07f149 100644
--- a/dbus/dbus-server-unix.c
+++ b/dbus/dbus-server-unix.c
@@ -72,21 +72,29 @@ unix_finalize (DBusServer *server)
  */
 /* Return value is just for memory, not other failures. */
 static dbus_bool_t
-handle_new_client_fd (DBusServer *server,
-                      int         client_fd)
+handle_new_client_fd_and_unlock (DBusServer *server,
+                                 int         client_fd)
 {
   DBusConnection *connection;
   DBusTransport *transport;
+  DBusNewConnectionFunction new_connection_function;
+  void *new_connection_data;
   
   _dbus_verbose ("Creating new client connection with fd %d\n", client_fd);
-          
+
+  HAVE_LOCK_CHECK (server);
+  
   if (!_dbus_set_fd_nonblocking (client_fd, NULL))
-    return TRUE;
+    {
+      SERVER_UNLOCK (server);
+      return TRUE;
+    }
   
   transport = _dbus_transport_new_for_fd (client_fd, TRUE, NULL);
   if (transport == NULL)
     {
       close (client_fd);
+      SERVER_UNLOCK (server);
       return FALSE;
     }
 
@@ -94,6 +102,7 @@ handle_new_client_fd (DBusServer *server,
                                             (const char **) server->auth_mechanisms))
     {
       _dbus_transport_unref (transport);
+      SERVER_UNLOCK (server);
       return FALSE;
     }
   
@@ -103,19 +112,27 @@ handle_new_client_fd (DBusServer *server,
   
   connection = _dbus_connection_new_for_transport (transport);
   _dbus_transport_unref (transport);
+  transport = NULL; /* now under the connection lock */
   
   if (connection == NULL)
-    return FALSE;
+    {
+      SERVER_UNLOCK (server);
+      return FALSE;
+    }
   
-  /* See if someone wants to handle this new connection,
-   * self-referencing for paranoia
+  /* See if someone wants to handle this new connection, self-referencing
+   * for paranoia.
    */
-  if (server->new_connection_function)
+  new_connection_function = server->new_connection_function;
+  new_connection_data = server->new_connection_data;
+
+  _dbus_server_ref_unlocked (server);
+  SERVER_UNLOCK (server);
+  
+  if (new_connection_function)
     {
-      dbus_server_ref (server);
-      
-      (* server->new_connection_function) (server, connection,
-                                           server->new_connection_data);
+      (* new_connection_function) (server, connection,
+                                   new_connection_data);
       dbus_server_unref (server);
     }
   
@@ -133,6 +150,8 @@ unix_handle_watch (DBusWatch    *watch,
   DBusServer *server = data;
   DBusServerUnix *unix_server = data;
 
+  SERVER_LOCK (server);
+  
   _dbus_assert (watch == unix_server->watch);
 
   _dbus_verbose ("Handling client connection, flags 0x%x\n", flags);
@@ -155,12 +174,14 @@ unix_handle_watch (DBusWatch    *watch,
           else
             _dbus_verbose ("Failed to accept a client connection: %s\n",
                            _dbus_strerror (errno));
+
+          SERVER_UNLOCK (server);
         }
       else
         {
 	  _dbus_fd_set_close_on_exec (client_fd);	  
 
-          if (!handle_new_client_fd (server, client_fd))
+          if (!handle_new_client_fd_and_unlock (server, client_fd))
             _dbus_verbose ("Rejected client connection due to lack of memory\n");
         }
     }
@@ -246,6 +267,10 @@ _dbus_server_new_for_fd (int               fd,
       return NULL;
     }
 
+#ifndef DBUS_DISABLE_CHECKS
+  unix_server->base.have_server_lock = TRUE;
+#endif
+  
   if (!_dbus_server_add_watch (&unix_server->base,
                                watch))
     {
@@ -254,6 +279,10 @@ _dbus_server_new_for_fd (int               fd,
       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;
diff --git a/dbus/dbus-server.c b/dbus/dbus-server.c
index 4c706191..788aaad7 100644
--- a/dbus/dbus-server.c
+++ b/dbus/dbus-server.c
@@ -67,7 +67,7 @@ _dbus_server_init_base (DBusServer             *server,
                         const DBusString       *address)
 {
   server->vtable = vtable;
-  server->refcount = 1;
+  server->refcount.value = 1;
 
   server->address = NULL;
   server->watches = NULL;
@@ -75,6 +75,10 @@ _dbus_server_init_base (DBusServer             *server,
   
   if (!_dbus_string_copy_data (address, &server->address))
     goto failed;
+
+  server->mutex = dbus_mutex_new ();
+  if (server->mutex == NULL)
+    goto failed;
   
   server->watches = _dbus_watch_list_new ();
   if (server->watches == NULL)
@@ -91,6 +95,11 @@ _dbus_server_init_base (DBusServer             *server,
   return TRUE;
 
  failed:
+  if (server->mutex)
+    {
+      dbus_mutex_free (server->mutex);
+      server->mutex = NULL;
+    }
   if (server->watches)
     {
       _dbus_watch_list_free (server->watches);
@@ -118,7 +127,7 @@ _dbus_server_init_base (DBusServer             *server,
  */
 void
 _dbus_server_finalize_base (DBusServer *server)
-{
+{  
   /* calls out to application code... */
   _dbus_data_slot_list_free (&server->slot_list);
 
@@ -130,6 +139,8 @@ _dbus_server_finalize_base (DBusServer *server)
   _dbus_watch_list_free (server->watches);
   _dbus_timeout_list_free (server->timeouts);
 
+  dbus_mutex_free (server->mutex);
+  
   dbus_free (server->address);
 
   dbus_free_string_array (server->auth_mechanisms);
@@ -146,6 +157,7 @@ dbus_bool_t
 _dbus_server_add_watch (DBusServer *server,
                         DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   return _dbus_watch_list_add_watch (server->watches, watch);
 }
 
@@ -159,6 +171,7 @@ void
 _dbus_server_remove_watch  (DBusServer *server,
                             DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   _dbus_watch_list_remove_watch (server->watches, watch);
 }
 
@@ -176,6 +189,8 @@ _dbus_server_toggle_watch (DBusServer  *server,
                            DBusWatch   *watch,
                            dbus_bool_t  enabled)
 {
+  HAVE_LOCK_CHECK (server);
+  
   if (server->watches) /* null during finalize */
     _dbus_watch_list_toggle_watch (server->watches,
                                    watch, enabled);
@@ -194,6 +209,8 @@ dbus_bool_t
 _dbus_server_add_timeout (DBusServer  *server,
 			  DBusTimeout *timeout)
 {
+  HAVE_LOCK_CHECK (server);
+  
   return _dbus_timeout_list_add_timeout (server->timeouts, timeout);
 }
 
@@ -207,6 +224,8 @@ void
 _dbus_server_remove_timeout (DBusServer  *server,
 			     DBusTimeout *timeout)
 {
+  HAVE_LOCK_CHECK (server);
+  
   _dbus_timeout_list_remove_timeout (server->timeouts, timeout);  
 }
 
@@ -224,6 +243,8 @@ _dbus_server_toggle_timeout (DBusServer  *server,
                              DBusTimeout *timeout,
                              dbus_bool_t  enabled)
 {
+  HAVE_LOCK_CHECK (server);
+  
   if (server->timeouts) /* null during finalize */
     _dbus_timeout_list_toggle_timeout (server->timeouts,
                                        timeout, enabled);
@@ -457,8 +478,16 @@ DBusServer *
 dbus_server_ref (DBusServer *server)
 {
   _dbus_return_val_if_fail (server != NULL, NULL);
-  
-  server->refcount += 1;
+
+#ifdef DBUS_HAVE_ATOMIC_INT
+  _dbus_atomic_inc (&server->refcount);
+#else
+  SERVER_LOCK (server);
+  _dbus_assert (server->refcount.value > 0);
+
+  server->refcount.value += 1;
+  SERVER_UNLOCK (server);
+#endif
 
   return server;
 }
@@ -474,12 +503,24 @@ dbus_server_ref (DBusServer *server)
 void
 dbus_server_unref (DBusServer *server)
 {
+  dbus_bool_t last_unref;
+  
   _dbus_return_if_fail (server != NULL);
 
-  _dbus_assert (server->refcount > 0);
+#ifdef DBUS_HAVE_ATOMIC_INT
+  last_unref = (_dbus_atomic_dec (&server->refcount) == 1);
+#else
+  SERVER_LOCK (server);
+  
+  _dbus_assert (server->refcount.value > 0);
 
-  server->refcount -= 1;
-  if (server->refcount == 0)
+  server->refcount.value -= 1;
+  last_unref = (server->refcount.value == 0);
+  
+  SERVER_UNLOCK (server);
+#endif
+  
+  if (last_unref)
     {
       _dbus_assert (server->vtable->finalize != NULL);
       
@@ -488,6 +529,25 @@ dbus_server_unref (DBusServer *server)
 }
 
 /**
+ * Like dbus_server_ref() but does not acquire the lock (must already be held)
+ *
+ * @param server the server.
+ */
+void
+_dbus_server_ref_unlocked (DBusServer *server)
+{
+  HAVE_LOCK_CHECK (server);
+
+#ifdef DBUS_HAVE_ATOMIC_INT
+  _dbus_atomic_inc (&server->refcount);
+#else
+  _dbus_assert (server->refcount.value > 0);
+
+  server->refcount.value += 1;
+#endif
+}
+
+/**
  * Releases the server's address and stops listening for
  * new clients. If called more than once, only the first
  * call has an effect. Does not modify the server's
@@ -499,6 +559,8 @@ void
 dbus_server_disconnect (DBusServer *server)
 {
   _dbus_return_if_fail (server != NULL);
+
+  SERVER_LOCK (server);
   
   _dbus_assert (server->vtable->disconnect != NULL);
 
@@ -507,6 +569,8 @@ dbus_server_disconnect (DBusServer *server)
   
   (* server->vtable->disconnect) (server);
   server->disconnected = TRUE;
+
+  SERVER_UNLOCK (server);
 }
 
 /**
@@ -517,9 +581,15 @@ dbus_server_disconnect (DBusServer *server)
 dbus_bool_t
 dbus_server_get_is_connected (DBusServer *server)
 {
-  _dbus_return_val_if_fail (server != NULL, FALSE);
+  dbus_bool_t retval;
   
-  return !server->disconnected;
+  _dbus_return_val_if_fail (server != NULL, FALSE);
+
+  SERVER_LOCK (server);
+  retval = !server->disconnected;
+  SERVER_UNLOCK (server);
+
+  return retval;
 }
 
 /**
@@ -532,9 +602,15 @@ dbus_server_get_is_connected (DBusServer *server)
 char*
 dbus_server_get_address (DBusServer *server)
 {
-  _dbus_return_val_if_fail (server != NULL, NULL);
+  char *retval;
   
-  return _dbus_strdup (server->address);
+  _dbus_return_val_if_fail (server != NULL, NULL);
+
+  SERVER_LOCK (server);
+  retval = _dbus_strdup (server->address);
+  SERVER_UNLOCK (server);
+
+  return retval;
 }
 
 /**
@@ -555,14 +631,22 @@ dbus_server_set_new_connection_function (DBusServer                *server,
                                          void                      *data,
                                          DBusFreeFunction           free_data_function)
 {
-  _dbus_return_if_fail (server != NULL);
+  DBusFreeFunction old_free_function;
+  void *old_data;
   
-  if (server->new_connection_free_data_function != NULL)
-    (* server->new_connection_free_data_function) (server->new_connection_data);
+  _dbus_return_if_fail (server != NULL);
+
+  SERVER_LOCK (server);
+  old_free_function = server->new_connection_free_data_function;
+  old_data = server->new_connection_data;
   
   server->new_connection_function = function;
   server->new_connection_data = data;
   server->new_connection_free_data_function = free_data_function;
+  SERVER_UNLOCK (server);
+    
+  if (old_free_function != NULL)
+    (* old_free_function) (old_data);
 }
 
 /**
@@ -589,14 +673,34 @@ dbus_server_set_watch_functions (DBusServer              *server,
                                  void                    *data,
                                  DBusFreeFunction         free_data_function)
 {
+  dbus_bool_t result;
+  DBusWatchList *watches;
+  
   _dbus_return_val_if_fail (server != NULL, FALSE);
+
+  SERVER_LOCK (server);
+  watches = server->watches;
+  server->watches = NULL;
+  if (watches)
+    {
+      SERVER_UNLOCK (server);
+      result = _dbus_watch_list_set_functions (watches,
+                                               add_function,
+                                               remove_function,
+                                               toggled_function,
+                                               data,
+                                               free_data_function);
+      SERVER_LOCK (server);
+    }
+  else
+    {
+      _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME);
+      result = FALSE;
+    }
+  server->watches = watches;
+  SERVER_UNLOCK (server);
   
-  return _dbus_watch_list_set_functions (server->watches,
-                                         add_function,
-                                         remove_function,
-                                         toggled_function,
-                                         data,
-                                         free_data_function);
+  return result;
 }
 
 /**
@@ -622,12 +726,34 @@ dbus_server_set_timeout_functions (DBusServer                *server,
 				   void                      *data,
 				   DBusFreeFunction           free_data_function)
 {
+  dbus_bool_t result;
+  DBusTimeoutList *timeouts;
+  
   _dbus_return_val_if_fail (server != NULL, FALSE);
+
+  SERVER_LOCK (server);
+  timeouts = server->timeouts;
+  server->timeouts = NULL;
+  if (timeouts)
+    {
+      SERVER_UNLOCK (server);
+      result = _dbus_timeout_list_set_functions (timeouts,
+                                                 add_function,
+                                                 remove_function,
+                                                 toggled_function,
+                                                 data,
+                                                 free_data_function);
+      SERVER_LOCK (server);
+    }
+  else
+    {
+      _dbus_warn ("Re-entrant call to %s\n", _DBUS_FUNCTION_NAME);
+      result = FALSE;
+    }
+  server->timeouts = timeouts;
+  SERVER_UNLOCK (server);
   
-  return _dbus_timeout_list_set_functions (server->timeouts,
-                                           add_function, remove_function,
-                                           toggled_function,
-                                           data, free_data_function); 
+  return result;
 }
 
 /**
@@ -647,6 +773,8 @@ dbus_server_set_auth_mechanisms (DBusServer  *server,
   char **copy;
 
   _dbus_return_val_if_fail (server != NULL, FALSE);
+
+  SERVER_LOCK (server);
   
   if (mechanisms != NULL)
     {
@@ -660,6 +788,8 @@ dbus_server_set_auth_mechanisms (DBusServer  *server,
   dbus_free_string_array (server->auth_mechanisms);
   server->auth_mechanisms = copy;
 
+  SERVER_UNLOCK (server);
+  
   return TRUE;
 }
 
@@ -732,19 +862,16 @@ dbus_server_set_data (DBusServer       *server,
   dbus_bool_t retval;
 
   _dbus_return_val_if_fail (server != NULL, FALSE);
-  
-#if 0
-  dbus_mutex_lock (server->mutex);
-#endif
+
+  SERVER_LOCK (server);
   
   retval = _dbus_data_slot_list_set (&slot_allocator,
                                      &server->slot_list,
                                      slot, data, free_data_func,
                                      &old_free_func, &old_data);
 
-#if 0
-  dbus_mutex_unlock (server->mutex);
-#endif
+
+  SERVER_UNLOCK (server);
   
   if (retval)
     {
@@ -772,17 +899,13 @@ dbus_server_get_data (DBusServer   *server,
 
   _dbus_return_val_if_fail (server != NULL, NULL);
   
-#if 0
-  dbus_mutex_lock (server->mutex);
-#endif
+  SERVER_LOCK (server);
   
   res = _dbus_data_slot_list_get (&slot_allocator,
                                   &server->slot_list,
                                   slot);
 
-#if 0
-  dbus_mutex_unlock (server->mutex);
-#endif
+  SERVER_UNLOCK (server);
   
   return res;
 }
diff --git a/dbus/dbus-transport-unix.c b/dbus/dbus-transport-unix.c
index 4190da46..2959886a 100644
--- a/dbus/dbus-transport-unix.c
+++ b/dbus/dbus-transport-unix.c
@@ -70,6 +70,8 @@ static void
 free_watches (DBusTransport *transport)
 {
   DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   if (unix_transport->read_watch)
     {
@@ -90,12 +92,16 @@ free_watches (DBusTransport *transport)
       _dbus_watch_unref (unix_transport->write_watch);
       unix_transport->write_watch = NULL;
     }
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 static void
 unix_finalize (DBusTransport *transport)
 {
   DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+  _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME);
   
   free_watches (transport);
 
@@ -871,6 +877,8 @@ static void
 unix_disconnect (DBusTransport *transport)
 {
   DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+  _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME);
   
   free_watches (transport);
   
@@ -1004,7 +1012,10 @@ unix_do_iteration (DBusTransport *transport,
        * by the io_path_cond condvar, so we won't reenter this.
        */
       if (flags & DBUS_ITERATION_BLOCK)
-	_dbus_connection_unlock (transport->connection);
+        {
+          _dbus_verbose ("unlock %s pre poll\n", _DBUS_FUNCTION_NAME);
+          _dbus_connection_unlock (transport->connection);
+        }
       
     again:
       poll_res = _dbus_poll (&poll_fd, 1, poll_timeout);
@@ -1013,7 +1024,10 @@ unix_do_iteration (DBusTransport *transport,
 	goto again;
 
       if (flags & DBUS_ITERATION_BLOCK)
-	_dbus_connection_lock (transport->connection);
+        {
+          _dbus_verbose ("lock %s post poll\n", _DBUS_FUNCTION_NAME);
+          _dbus_connection_lock (transport->connection);
+        }
       
       if (poll_res >= 0)
         {
diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c
index 94d4a8a8..c08573d8 100644
--- a/dbus/dbus-transport.c
+++ b/dbus/dbus-transport.c
@@ -388,10 +388,12 @@ _dbus_transport_unref (DBusTransport *transport)
 {
   _dbus_assert (transport != NULL);
   _dbus_assert (transport->refcount > 0);
-
+  
   transport->refcount -= 1;
   if (transport->refcount == 0)
     {
+      _dbus_verbose ("%s: finalizing\n", _DBUS_FUNCTION_NAME);
+      
       _dbus_assert (transport->vtable->finalize != NULL);
       
       (* transport->vtable->finalize) (transport);
@@ -409,14 +411,18 @@ _dbus_transport_unref (DBusTransport *transport)
 void
 _dbus_transport_disconnect (DBusTransport *transport)
 {
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
+  
   _dbus_assert (transport->vtable->disconnect != NULL);
-
+  
   if (transport->disconnected)
     return;
 
   (* transport->vtable->disconnect) (transport);
   
   transport->disconnected = TRUE;
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 /**
@@ -437,7 +443,8 @@ _dbus_transport_get_is_connected (DBusTransport *transport)
  * Returns #TRUE if we have been authenticated.  Will return #TRUE
  * even if the transport is disconnected.
  *
- * @todo needs to drop connection->mutex when calling the unix_user_function
+ * @todo we drop connection->mutex when calling the unix_user_function,
+ * which may not be safe really.
  *
  * @param transport the transport
  * @returns whether we're authenticated
@@ -453,6 +460,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport)
       
       if (transport->disconnected)
         return FALSE;
+
+      /* paranoia ref since we call user callbacks sometimes */
+      _dbus_connection_ref_unlocked (transport->connection);
       
       maybe_authenticated =
         (!(transport->send_credentials_pending ||
@@ -486,21 +496,40 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport)
 
           if (transport->unix_user_function != NULL)
             {
-              /* FIXME we hold the connection lock here and should drop it */
-              if (!(* transport->unix_user_function) (transport->connection,
-                                                      auth_identity.uid,
-                                                      transport->unix_user_data))
+              dbus_bool_t allow;
+              DBusConnection *connection;
+              DBusAllowUnixUserFunction unix_user_function;
+              void *unix_user_data;
+              
+              /* Dropping the lock here probably isn't that safe. */
+
+              connection = transport->connection;
+              unix_user_function = transport->unix_user_function;
+              unix_user_data = transport->unix_user_data;
+
+              _dbus_verbose ("unlock %s\n", _DBUS_FUNCTION_NAME);
+              _dbus_connection_unlock (connection);
+              
+              allow = (* unix_user_function) (connection,
+                                              auth_identity.uid,
+                                              unix_user_data);
+
+              _dbus_verbose ("lock %s post unix user function\n", _DBUS_FUNCTION_NAME);
+              _dbus_connection_lock (connection);
+
+              if (allow)
+                {
+                  _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid);
+                }
+              else
                 {
                   _dbus_verbose ("Client UID "DBUS_UID_FORMAT
                                  " was rejected, disconnecting\n",
                                  auth_identity.uid);
                   _dbus_transport_disconnect (transport);
+                  _dbus_connection_unref_unlocked (connection);
                   return FALSE;
                 }
-              else
-                {
-                  _dbus_verbose ("Client UID "DBUS_UID_FORMAT" authorized\n", auth_identity.uid);
-                }
             }
           else
             {
@@ -515,6 +544,7 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport)
                                  " but our UID is "DBUS_UID_FORMAT", disconnecting\n",
                                  auth_identity.uid, our_identity.uid);
                   _dbus_transport_disconnect (transport);
+                  _dbus_connection_unref_unlocked (transport->connection);
                   return FALSE;
                 }
               else
@@ -527,8 +557,9 @@ _dbus_transport_get_is_authenticated (DBusTransport *transport)
         }
 
       transport->authenticated = maybe_authenticated;
-      
-      return transport->authenticated;
+
+      _dbus_connection_unref_unlocked (transport->connection);
+      return maybe_authenticated;
     }
 }
 
@@ -670,6 +701,8 @@ _dbus_transport_do_iteration (DBusTransport  *transport,
   (* transport->vtable->do_iteration) (transport, flags,
                                        timeout_milliseconds);
   _dbus_transport_unref (transport);
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 static dbus_bool_t