dbus/dbus dbus-connection-internal.h, 1.19, 1.19.2.1 dbus-connection.c, 1.87.2.1, 1.87.2.2 dbus-pending-call.c, 1.8, 1.8.2.1 dbus-pending-call.h, 1.6, 1.6.2.1 dbus-server-protected.h, 1.14, 1.14.2.1 dbus-server-unix.c, 1.24, 1.24.2.1 dbus-server.c, 1.36, 1.36.2.1 dbus-transport-unix.c, 1.44, 1.44.2.1 dbus-transport.c, 1.42, 1.42.2.1

Joe Shaw joe at freedesktop.org
Wed Feb 16 14:45:42 PST 2005


Update of /cvs/dbus/dbus/dbus
In directory gabe:/tmp/cvs-serv28286/dbus

Modified Files:
      Tag: dbus-0-23
	dbus-connection-internal.h dbus-connection.c 
	dbus-pending-call.c dbus-pending-call.h 
	dbus-server-protected.h dbus-server-unix.c dbus-server.c 
	dbus-transport-unix.c dbus-transport.c 
Log Message:
2005-02-16  Joe Shaw  <joeshaw at novell.com>

	* dbus/dbus-connection-internal.h, dbus/dbus-connection.[ch],
	dbus/dbus-pending-call.[ch], dbus/dbus-server-protected.h,
	dbus/dbus-server-unix.c, dbus/dbus-server.c,
	dbus/dbus-transport-unix.c, dbus/dbus-transport.c: Backport
	a ton of thread-related fixes from HEAD to this branch.

	* glib/dbus-gproxy.c: Update dbus_pending_call_get_reply() to
	dbus_pending_call_steal_reply().

Index: dbus-connection-internal.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection-internal.h,v
retrieving revision 1.19
retrieving revision 1.19.2.1
diff -u -d -r1.19 -r1.19.2.1
--- dbus-connection-internal.h	26 Nov 2004 01:53:13 -0000	1.19
+++ dbus-connection-internal.h	16 Feb 2005 22:45:40 -0000	1.19.2.1
@@ -41,8 +41,8 @@
   DBUS_ITERATION_BLOCK      = 1 << 2  /**< Block if nothing to do. */
 } DBusIterationFlags;
 
-/** default timeout value when waiting for a message reply */
-#define _DBUS_DEFAULT_TIMEOUT_VALUE (15 * 1000)
+/** default timeout value when waiting for a message reply, 25 seconds */
+#define _DBUS_DEFAULT_TIMEOUT_VALUE (25 * 1000)
 
 void              _dbus_connection_lock                        (DBusConnection     *connection);
 void              _dbus_connection_unlock                      (DBusConnection     *connection);
@@ -74,7 +74,7 @@
                                                                 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);
 
@@ -84,12 +84,12 @@
 void              _dbus_pending_call_notify                    (DBusPendingCall    *pending);
 void              _dbus_connection_remove_pending_call         (DBusConnection     *connection,
                                                                 DBusPendingCall    *pending);
-DBusMessage*      _dbus_connection_block_for_reply             (DBusConnection     *connection,
-                                                                dbus_uint32_t       client_serial,
-                                                                int                 timeout_milliseconds);
+void              _dbus_connection_block_pending_call          (DBusPendingCall    *pending);
 void              _dbus_pending_call_complete_and_unlock       (DBusPendingCall    *pending,
                                                                 DBusMessage        *message);
-
+dbus_bool_t       _dbus_connection_send_and_unlock             (DBusConnection     *connection,
+                                                                DBusMessage        *message,
+                                                                dbus_uint32_t      *client_serial);
 
 /**
  * @addtogroup DBusPendingCallInternals DBusPendingCall implementation details

Index: dbus-connection.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-connection.c,v
retrieving revision 1.87.2.1
retrieving revision 1.87.2.2
diff -u -d -r1.87.2.1 -r1.87.2.2
--- dbus-connection.c	10 Feb 2005 23:22:11 -0000	1.87.2.1
+++ dbus-connection.c	16 Feb 2005 22:45:40 -0000	1.87.2.2
@@ -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
  * 
@@ -40,20 +40,37 @@
 #include "dbus-object-tree.h"
 #include "dbus-marshal.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" : \
@@ -172,17 +189,17 @@
 
   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 */
+  DBusMutex *dispatch_mutex;     /**< Protects dispatch_acquired */
+  DBusCondVar *dispatch_cond;    /**< Notify when dispatch_acquired is available */
+  DBusMutex *io_path_mutex;      /**< Protects io_path_acquired */
+  DBusCondVar *io_path_cond;     /**< Notify when io_path_acquired is available */
   
   DBusList *outgoing_messages; /**< Queue of messages we need to send, send the end of the list first. */
   DBusList *incoming_messages; /**< Queue of messages we have received, end of the list received most recently. */
 
-  DBusMessage *message_borrowed; /**< True if the first incoming message has been borrowed */
-  DBusCondVar *message_returned_cond; /**< Used with dbus_connection_borrow_message() */
+  DBusMessage *message_borrowed; /**< Filled in if the first incoming message has been borrowed;
+                                  *   dispatch_acquired will be set by the borrower
+                                  */
   
   int n_outgoing;              /**< Length of outgoing queue. */
   int n_incoming;              /**< Length of incoming queue. */
@@ -216,9 +233,16 @@
                          *   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 (can drain incoming queue) */
+  unsigned int io_path_acquired : 1;  /**< Someone has transport io path (can use the transport to read/write messages) */
+  
   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 
@@ -228,6 +252,8 @@
 static void               _dbus_connection_update_dispatch_status_and_unlock (DBusConnection     *connection,
                                                                               DBusDispatchStatus  new_status);
 static void               _dbus_connection_last_unref                        (DBusConnection     *connection);
+static void               _dbus_connection_acquire_dispatch                  (DBusConnection     *connection);
+static void               _dbus_connection_release_dispatch                  (DBusConnection     *connection);
 
 static DBusMessageFilter *
 _dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -358,7 +384,7 @@
 
   _dbus_connection_wakeup_mainloop (connection);
   
-  _dbus_verbose ("Message %p (%d %s %s %s '%s') added to incoming queue %p, %d incoming\n",
+  _dbus_verbose ("Message %p (%d %s %s %s '%s' reply to %u) added to incoming queue %p, %d incoming\n",
                  message,
                  dbus_message_get_type (message),
 		 dbus_message_get_path (message),
@@ -369,6 +395,7 @@
                  dbus_message_get_member (message) :
                  "no member",
                  dbus_message_get_signature (message),
+                 dbus_message_get_reply_serial (message),
                  connection,
                  connection->n_incoming);
 }
@@ -387,6 +414,8 @@
 _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 +437,7 @@
 dbus_bool_t
 _dbus_connection_has_messages_to_send_unlocked (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
   return connection->outgoing_messages != NULL;
 }
 
@@ -441,6 +471,8 @@
 DBusMessage*
 _dbus_connection_get_message_to_send (DBusConnection *connection)
 {
+  HAVE_LOCK_CHECK (connection);
+  
   return _dbus_list_get_last (&connection->outgoing_messages);
 }
 
@@ -458,6 +490,8 @@
 {
   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 +529,62 @@
   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 +599,9 @@
 _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 +616,10 @@
 _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 +638,66 @@
                                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;
   
-  if (connection->watches) /* null during finalize */
-    _dbus_watch_list_toggle_watch (connection->watches,
-                                   watch, enabled);
+  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
+   */
+  
+  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 +715,9 @@
 _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 +732,10 @@
 _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 +748,22 @@
  * @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 +774,8 @@
                                     pending))
     {
       _dbus_connection_remove_timeout (connection, pending->timeout);
+
+      HAVE_LOCK_CHECK (connection);
       return FALSE;
     }
   
@@ -634,6 +783,8 @@
   pending->connection = connection;
 
   dbus_pending_call_ref (pending);
+
+  HAVE_LOCK_CHECK (connection);
   
   return TRUE;
 }
@@ -664,6 +815,19 @@
 }
 
 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 +838,7 @@
   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);
 }
@@ -737,7 +902,7 @@
 /**
  * Acquire the transporter I/O path. This must be done before
  * doing any I/O in the transporter. May sleep and drop the
- * connection mutex while waiting for the I/O path.
+ * IO path mutex while waiting for the I/O path.
  *
  * @param connection the connection.
  * @param timeout_milliseconds maximum blocking time, or -1 for no limit.
@@ -747,26 +912,63 @@
 _dbus_connection_acquire_io_path (DBusConnection *connection,
 				  int timeout_milliseconds)
 {
-  dbus_bool_t res = TRUE;
+  dbus_bool_t we_acquired;
+  
+  HAVE_LOCK_CHECK (connection);
+
+  /* We don't want the connection to vanish */
+  _dbus_connection_ref_unlocked (connection);
 
+  /* We will only touch io_path_acquired which is protected by our mutex */
+  CONNECTION_UNLOCK (connection);
+  
+  _dbus_verbose ("%s locking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  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);
+
+  we_acquired = FALSE;
+  
   if (connection->io_path_acquired)
     {
-      if (timeout_milliseconds != -1) 
-	res = dbus_condvar_wait_timeout (connection->io_path_cond,
-					 connection->mutex,
-					 timeout_milliseconds);
+      if (timeout_milliseconds != -1)
+        {
+          _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);
+        }
       else
-	dbus_condvar_wait (connection->io_path_cond, connection->mutex);
+        {
+          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);
+            }
+        }
     }
   
-  if (res)
+  if (!connection->io_path_acquired)
     {
-      _dbus_assert (!connection->io_path_acquired);
-
+      we_acquired = TRUE;
       connection->io_path_acquired = TRUE;
     }
   
-  return res;
+  _dbus_verbose ("%s end connection->io_path_acquired = %d we_acquired = %d\n",
+                 _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);
+
+  CONNECTION_LOCK (connection);
+  
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_connection_unref_unlocked (connection);
+  
+  return we_acquired;
 }
 
 /**
@@ -779,17 +981,27 @@
 static void
 _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_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);
-}
 
+  _dbus_verbose ("%s unlocking io_path_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->io_path_mutex);
+}
 
 /**
  * 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,20 +1027,30 @@
  * @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;
 
   if (_dbus_connection_acquire_io_path (connection,
 					(flags & DBUS_ITERATION_BLOCK) ? timeout_milliseconds : 0))
     {
+      HAVE_LOCK_CHECK (connection);
+      
       _dbus_transport_do_iteration (connection->transport,
 				    flags, timeout_milliseconds);
       _dbus_connection_release_io_path (connection);
     }
+
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 /**
@@ -848,6 +1070,8 @@
   DBusTimeoutList *timeout_list;
   DBusHashTable *pending_replies;
   DBusMutex *mutex;
+  DBusMutex *io_path_mutex;
+  DBusMutex *dispatch_mutex;
   DBusCondVar *message_returned_cond;
   DBusCondVar *dispatch_cond;
   DBusCondVar *io_path_cond;
@@ -861,6 +1085,8 @@
   pending_replies = NULL;
   timeout_list = NULL;
   mutex = NULL;
+  io_path_mutex = NULL;
+  dispatch_mutex = NULL;
   message_returned_cond = NULL;
   dispatch_cond = NULL;
   io_path_cond = NULL;
@@ -891,6 +1117,14 @@
   mutex = dbus_mutex_new ();
   if (mutex == NULL)
     goto error;
+
+  io_path_mutex = dbus_mutex_new ();
+  if (io_path_mutex == NULL)
+    goto error;
+
+  dispatch_mutex = dbus_mutex_new ();
+  if (dispatch_mutex == NULL)
+    goto error;
   
   message_returned_cond = dbus_condvar_new ();
   if (message_returned_cond == NULL)
@@ -929,8 +1163,9 @@
   connection->refcount.value = 1;
   connection->mutex = mutex;
   connection->dispatch_cond = dispatch_cond;
+  connection->dispatch_mutex = dispatch_mutex;
   connection->io_path_cond = io_path_cond;
-  connection->message_returned_cond = message_returned_cond;
+  connection->io_path_mutex = io_path_mutex;
   connection->transport = transport;
   connection->watches = watch_list;
   connection->timeouts = timeout_list;
@@ -949,11 +1184,15 @@
   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;
   
@@ -976,6 +1215,12 @@
   if (mutex != NULL)
     dbus_mutex_free (mutex);
 
+  if (io_path_mutex != NULL)
+    dbus_mutex_free (io_path_mutex);
+
+  if (dispatch_mutex != NULL)
+    dbus_mutex_free (dispatch_mutex);
+  
   if (connection != NULL)
     dbus_free (connection);
 
@@ -1006,9 +1251,11 @@
  */
 DBusConnection *
 _dbus_connection_ref_unlocked (DBusConnection *connection)
-{
-  _dbus_return_val_if_fail (connection != NULL, NULL);
-  _dbus_return_val_if_fail (connection->generation == _dbus_current_generation, NULL);
+{  
+  _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 +1278,9 @@
 {
   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,17 +1338,27 @@
   DBusDispatchStatus status;
 
   connection = data;
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   CONNECTION_LOCK (connection);
   _dbus_connection_acquire_io_path (connection, -1);
+  HAVE_LOCK_CHECK (connection);
   retval = _dbus_transport_handle_watch (connection->transport,
                                          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 +1413,10 @@
       dbus_set_error (error, DBUS_ERROR_NO_MEMORY, NULL);
       return NULL;
     }
-  
+
+#ifndef DBUS_DISABLE_CHECKS
+  _dbus_assert (!connection->have_connection_lock);
+#endif
   return connection;
 }
 
@@ -1281,8 +1543,10 @@
   
   dbus_condvar_free (connection->dispatch_cond);
   dbus_condvar_free (connection->io_path_cond);
-  dbus_condvar_free (connection->message_returned_cond);  
-  
+
+  dbus_mutex_free (connection->io_path_mutex);
+  dbus_mutex_free (connection->dispatch_mutex);
+
   dbus_mutex_free (connection->mutex);
   
   dbus_free (connection);
@@ -1357,7 +1621,8 @@
   
   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 +1632,7 @@
 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,7 +1711,9 @@
 {
   DBusPreallocatedSend *preallocated;
 
-  _dbus_return_val_if_fail (connection != NULL, NULL);
+  HAVE_LOCK_CHECK (connection);
+  
+  _dbus_assert (connection != NULL);
   
   preallocated = dbus_new (DBusPreallocatedSend, 1);
   if (preallocated == NULL)
@@ -1540,11 +1808,12 @@
   dbus_free (preallocated);
 }
 
+/* Called with lock held, does not update dispatch status */
 static void
-_dbus_connection_send_preallocated_unlocked (DBusConnection       *connection,
-                                             DBusPreallocatedSend *preallocated,
-                                             DBusMessage          *message,
-                                             dbus_uint32_t        *client_serial)
+_dbus_connection_send_preallocated_unlocked_no_update (DBusConnection       *connection,
+                                                       DBusPreallocatedSend *preallocated,
+                                                       DBusMessage          *message,
+                                                       dbus_uint32_t        *client_serial)
 {
   dbus_uint32_t serial;
   const char *sig;
@@ -1564,17 +1833,8 @@
   connection->n_outgoing += 1;
 
   sig = dbus_message_get_signature (message);
-#ifndef DBUS_DISABLE_ASSERT
-  {
-    DBusString foo;
-    _dbus_verbose (" validating signature '%s'\n", sig);
-    _dbus_string_init_const (&foo, sig);
-    _dbus_assert (_dbus_string_validate_signature (&foo, 0,
-                                                   _dbus_string_get_length (&foo)));
-  }
-#endif
   
-  _dbus_verbose ("Message %p (%d %s %s %s '%s') added to outgoing queue %p, %d pending to send\n",
+  _dbus_verbose ("Message %p (%d %s %s %s '%s') for %s added to outgoing queue %p, %d pending to send\n",
                  message,
                  dbus_message_get_type (message),
 		 dbus_message_get_path (message),
@@ -1585,6 +1845,9 @@
                  dbus_message_get_member (message) :
                  "no member",
                  sig,
+                 dbus_message_get_destination (message) ?
+                 dbus_message_get_destination (message) :
+                 "null",
                  connection,
                  connection->n_outgoing);
 
@@ -1600,15 +1863,18 @@
       if (client_serial)
         *client_serial = dbus_message_get_serial (message);
     }
+
+  _dbus_verbose ("Message %p serial is %u\n",
+                 message, dbus_message_get_serial (message));
   
   _dbus_message_lock (message);
 
   /* 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)
@@ -1623,10 +1889,13 @@
 {
   DBusDispatchStatus status;
 
-  _dbus_connection_send_preallocated_unlocked (connection,
-                                               preallocated,
-                                               message, client_serial);
+  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 */
@@ -1669,9 +1938,9 @@
 }
 
 static dbus_bool_t
-_dbus_connection_send_unlocked (DBusConnection *connection,
-                                DBusMessage    *message,
-                                dbus_uint32_t  *client_serial)
+_dbus_connection_send_unlocked_no_update (DBusConnection *connection,
+                                          DBusMessage    *message,
+                                          dbus_uint32_t  *client_serial)
 {
   DBusPreallocatedSend *preallocated;
 
@@ -1682,15 +1951,14 @@
   if (preallocated == NULL)
     return FALSE;
 
-
-  _dbus_connection_send_preallocated_unlocked (connection,
-                                               preallocated,
-                                               message,
-                                               client_serial);
+  _dbus_connection_send_preallocated_unlocked_no_update (connection,
+                                                         preallocated,
+                                                         message,
+                                                         client_serial);
   return TRUE;
 }
 
-static dbus_bool_t
+dbus_bool_t
 _dbus_connection_send_and_unlock (DBusConnection *connection,
 				  DBusMessage    *message,
 				  dbus_uint32_t  *client_serial)
@@ -1702,7 +1970,10 @@
   
   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,
@@ -1765,6 +2036,7 @@
 				   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 */
@@ -1870,7 +2142,7 @@
 						      pending))
     goto error;
   
-  if (!_dbus_connection_send_unlocked (connection, message, NULL))
+  if (!_dbus_connection_send_unlocked_no_update (connection, message, NULL))
     {
       _dbus_connection_detach_pending_call_and_unlock (connection,
 						       pending);
@@ -1880,8 +2152,12 @@
   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 */
@@ -1896,11 +2172,16 @@
   return FALSE;
 }
 
+/* This is slightly strange since we can pop a message here without
+ * the dispatch lock.
+ */
 static DBusMessage*
 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);
 
@@ -1921,46 +2202,49 @@
 }
 
 /**
- * Blocks a certain time period while waiting for a reply.
- * If no reply arrives, returns #NULL.
- *
- * @todo could use performance improvements (it keeps scanning
- * the whole message queue for example) and has thread issues,
- * see comments in source
+ * Blocks until a pending call times out or gets a reply.
  *
  * Does not re-enter the main loop or run filter/path-registered
  * callbacks. The reply to the message will not be seen by
  * filter callbacks.
  *
- * @param connection the connection
- * @param client_serial the reply serial to wait for
- * @param timeout_milliseconds timeout in milliseconds or -1 for default
- * @returns the message that is the reply or #NULL if no reply
+ * Returns immediately if pending call already got a reply.
+ * 
+ * @todo could use performance improvements (it keeps scanning
+ * the whole message queue for example)
+ *
+ * @param pending the pending call we block for a reply on
  */
-DBusMessage*
-_dbus_connection_block_for_reply (DBusConnection     *connection,
-                                  dbus_uint32_t       client_serial,
-                                  int                 timeout_milliseconds)
+void
+_dbus_connection_block_pending_call (DBusPendingCall *pending)
 {
   long start_tv_sec, start_tv_usec;
   long end_tv_sec, end_tv_usec;
   long tv_sec, tv_usec;
   DBusDispatchStatus status;
+  DBusConnection *connection;
+  dbus_uint32_t client_serial;
+  int timeout_milliseconds;
 
-  _dbus_return_val_if_fail (connection != NULL, NULL);
-  _dbus_return_val_if_fail (client_serial != 0, NULL);
-  _dbus_return_val_if_fail (timeout_milliseconds >= 0 || timeout_milliseconds == -1, FALSE);
+  _dbus_assert (pending != NULL);
+
+  if (dbus_pending_call_get_completed (pending))
+    return;
+
+  if (pending->connection == NULL)
+    return; /* call already detached */
+
+  dbus_pending_call_ref (pending); /* necessary because the call could be canceled */
   
-  if (timeout_milliseconds == -1)
-    timeout_milliseconds = _DBUS_DEFAULT_TIMEOUT_VALUE;
+  connection = pending->connection;
+  client_serial = pending->reply_serial;
 
-  /* it would probably seem logical to pass in _DBUS_INT_MAX
-   * for infinite timeout, but then math below would get
-   * all overflow-prone, so smack that down.
+  /* note that timeout_milliseconds is limited to a smallish value
+   * in _dbus_pending_call_new() so overflows aren't possible
+   * below
    */
-  if (timeout_milliseconds > _DBUS_ONE_HOUR_IN_MILLISECONDS * 6)
-    timeout_milliseconds = _DBUS_ONE_HOUR_IN_MILLISECONDS * 6;
-  
+  timeout_milliseconds = dbus_timeout_get_interval (pending->timeout);
+
   /* Flush message queue */
   dbus_connection_flush (connection);
 
@@ -1977,44 +2261,68 @@
                  client_serial,
                  start_tv_sec, start_tv_usec,
                  end_tv_sec, end_tv_usec);
-  
+
   /* Now we wait... */
-  /* THREAD TODO: This is busted. What if a dispatch() or pop_message
-   * 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);
 
+  /* the get_completed() is in case a dispatch() while we were blocking
+   * got the reply instead of us.
+   */
+  if (dbus_pending_call_get_completed (pending))
+    {
+      _dbus_verbose ("Pending call completed by dispatch in %s\n", _DBUS_FUNCTION_NAME);
+      _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+      return;
+    }
+  
   if (status == DBUS_DISPATCH_DATA_REMAINS)
     {
       DBusMessage *reply;
       
       reply = check_for_reply_unlocked (connection, client_serial);
       if (reply != NULL)
-        {          
-          status = _dbus_connection_get_dispatch_status_unlocked (connection);
+        {
+          _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
 
           _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
+          
+          _dbus_pending_call_complete_and_unlock (pending, reply);
+          dbus_message_unref (reply);
 
-          /* Unlocks, and calls out to user code */
+          CONNECTION_LOCK (connection);
+          status = _dbus_connection_get_dispatch_status_unlocked (connection);
           _dbus_connection_update_dispatch_status_and_unlock (connection, status);
           
-          return reply;
+          return;
         }
     }
   
   _dbus_get_current_time (&tv_sec, &tv_usec);
   
   if (!_dbus_connection_get_is_connected_unlocked (connection))
-    return NULL;
+    {
+      /* FIXME send a "DBUS_ERROR_DISCONNECTED" instead, just to help
+       * programmers understand what went wrong since the timeout is
+       * confusing
+       */
+      
+      _dbus_pending_call_complete_and_unlock (pending, NULL);
+      return;
+    }
   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)
@@ -2045,10 +2353,10 @@
       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;
@@ -2057,10 +2365,15 @@
   _dbus_verbose ("dbus_connection_send_with_reply_and_block(): Waited %ld milliseconds and got no reply\n",
                  (tv_sec - start_tv_sec) * 1000 + (tv_usec - start_tv_usec) / 1000);
 
-  /* unlocks and calls out to user code */
-  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
+  _dbus_assert (!dbus_pending_call_get_completed (pending));
+  
+  /* unlock and call user code */
+  _dbus_pending_call_complete_and_unlock (pending, NULL);
 
-  return NULL;
+  /* update user code on dispatch status */
+  CONNECTION_LOCK (connection);
+  status = _dbus_connection_get_dispatch_status_unlocked (connection);
+  _dbus_connection_update_dispatch_status_and_unlock (connection, status);
 }
 
 /**
@@ -2085,40 +2398,40 @@
  * @returns the message that is the reply or #NULL with an error code if the
  * function fails.
  */
-DBusMessage *
+DBusMessage*
 dbus_connection_send_with_reply_and_block (DBusConnection     *connection,
                                            DBusMessage        *message,
                                            int                 timeout_milliseconds,
                                            DBusError          *error)
 {
-  dbus_uint32_t client_serial;
   DBusMessage *reply;
+  DBusPendingCall *pending;
   
   _dbus_return_val_if_fail (connection != NULL, NULL);
   _dbus_return_val_if_fail (message != NULL, NULL);
   _dbus_return_val_if_fail (timeout_milliseconds >= 0 || timeout_milliseconds == -1, FALSE);  
   _dbus_return_val_if_error_is_set (error, NULL);
   
-  if (!dbus_connection_send (connection, message, &client_serial))
+  if (!dbus_connection_send_with_reply (connection, message,
+                                        &pending, timeout_milliseconds))
     {
       _DBUS_SET_OOM (error);
       return NULL;
     }
 
-  reply = _dbus_connection_block_for_reply (connection,
-                                            client_serial,
-                                            timeout_milliseconds);
+  _dbus_assert (pending != NULL);
   
-  if (reply == NULL)
-    {
-      if (dbus_connection_get_is_connected (connection))
-        dbus_set_error (error, DBUS_ERROR_NO_REPLY, "Message did not receive a reply");
-      else
-        dbus_set_error (error, DBUS_ERROR_DISCONNECTED, "Disconnected prior to receiving a reply");
+  dbus_pending_call_block (pending);
 
-      return NULL;
-    }
-  else if (dbus_set_error_from_message (error, reply))
+  reply = dbus_pending_call_steal_reply (pending);
+  dbus_pending_call_unref (pending);
+
+  /* call_complete_and_unlock() called from pending_call_block() should
+   * always fill this in.
+   */
+  _dbus_assert (reply != NULL);
+  
+   if (dbus_set_error_from_message (error, reply))
     {
       dbus_message_unref (reply);
       return NULL;
@@ -2147,28 +2460,25 @@
   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);
-}
 
-/* Call with mutex held. Will drop it while waiting and re-acquire
- * before returning
- */
-static void
-_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);
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 /**
@@ -2182,18 +2492,23 @@
  * quickly as possible and don't keep a reference to it after
  * returning it. If you need to keep the message, make a copy of it.
  *
+ * dbus_connection_dispatch() will block if called while a borrowed
+ * message is outstanding; only one piece of code can be playing with
+ * the incoming queue at a time. This function will block if called
+ * during a dbus_connection_dispatch().
+ *
  * @param connection the connection.
  * @returns next message in the incoming queue.
  */
 DBusMessage*
-dbus_connection_borrow_message  (DBusConnection *connection)
+dbus_connection_borrow_message (DBusConnection *connection)
 {
-  DBusMessage *message;
   DBusDispatchStatus status;
+  DBusMessage *message;
 
   _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
@@ -2204,21 +2519,28 @@
   
   CONNECTION_LOCK (connection);
 
-  if (connection->message_borrowed != NULL)
-    _dbus_connection_wait_for_borrowed (connection);
-  
-  message = _dbus_list_get_first (&connection->incoming_messages);
+  _dbus_connection_acquire_dispatch (connection);
 
-  if (message) 
-    connection->message_borrowed = message;
+  /* While a message is outstanding, the dispatch lock is held */
+  _dbus_assert (connection->message_borrowed == NULL);
+
+  connection->message_borrowed = _dbus_list_get_first (&connection->incoming_messages);
   
+  message = connection->message_borrowed;
+
+  /* Note that we KEEP the dispatch lock until the message is returned */
+  if (message == NULL)
+    _dbus_connection_release_dispatch (connection);
+
   CONNECTION_UNLOCK (connection);
+  
   return message;
 }
 
 /**
  * Used to return a message after peeking at it using
- * dbus_connection_borrow_message().
+ * dbus_connection_borrow_message(). Only called if
+ * message from dbus_connection_borrow_message() was non-#NULL.
  *
  * @param connection the connection
  * @param message the message from dbus_connection_borrow_message()
@@ -2229,15 +2551,16 @@
 {
   _dbus_return_if_fail (connection != NULL);
   _dbus_return_if_fail (message != NULL);
-  /* can't borrow during dispatch */
-  _dbus_return_if_fail (!connection->dispatch_acquired);
+  _dbus_return_if_fail (message == connection->message_borrowed);
+  _dbus_return_if_fail (connection->dispatch_acquired);
   
   CONNECTION_LOCK (connection);
   
   _dbus_assert (message == connection->message_borrowed);
   
   connection->message_borrowed = NULL;
-  dbus_condvar_wake_all (connection->message_returned_cond);
+
+  _dbus_connection_release_dispatch (connection);
   
   CONNECTION_UNLOCK (connection);
 }
@@ -2259,8 +2582,8 @@
 
   _dbus_return_if_fail (connection != NULL);
   _dbus_return_if_fail (message != NULL);
-  /* can't borrow during dispatch */
-  _dbus_return_if_fail (!connection->dispatch_acquired);
+  _dbus_return_if_fail (message == connection->message_borrowed);
+  _dbus_return_if_fail (connection->dispatch_acquired);
   
   CONNECTION_LOCK (connection);
  
@@ -2275,7 +2598,8 @@
 		 message, connection->n_incoming);
  
   connection->message_borrowed = NULL;
-  dbus_condvar_wake_all (connection->message_returned_cond);
+
+  _dbus_connection_release_dispatch (connection);
   
   CONNECTION_UNLOCK (connection);
 }
@@ -2286,8 +2610,9 @@
 static DBusList*
 _dbus_connection_pop_message_link_unlocked (DBusConnection *connection)
 {
-  if (connection->message_borrowed != NULL)
-    _dbus_connection_wait_for_borrowed (connection);
+  HAVE_LOCK_CHECK (connection);
+  
+  _dbus_assert (connection->message_borrowed == NULL);
   
   if (connection->n_incoming > 0)
     {
@@ -2322,6 +2647,8 @@
 _dbus_connection_pop_message_unlocked (DBusConnection *connection)
 {
   DBusList *link;
+
+  HAVE_LOCK_CHECK (connection);
   
   link = _dbus_connection_pop_message_link_unlocked (connection);
 
@@ -2343,9 +2670,13 @@
 _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);
+  /* We had to have the dispatch lock across the pop/putback */
+  _dbus_assert (connection->dispatch_acquired);
 
   _dbus_list_prepend_link (&connection->incoming_messages,
                            message_link);
@@ -2375,6 +2706,11 @@
  * useful in very simple programs that don't share a #DBusConnection
  * with any libraries or other modules.
  *
+ * There is a lock that covers all ways of accessing the incoming message
+ * queue, so dbus_connection_dispatch(), dbus_connection_pop_message(),
+ * dbus_connection_borrow_message(), etc. will all block while one of the others
+ * in the group is running.
+ * 
  * @param connection the connection.
  * @returns next message in the incoming queue.
  */
@@ -2384,6 +2720,8 @@
   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
    */
@@ -2392,32 +2730,52 @@
     return NULL;
   
   CONNECTION_LOCK (connection);
-
+  _dbus_connection_acquire_dispatch (connection);
+  HAVE_LOCK_CHECK (connection);
+  
   message = _dbus_connection_pop_message_unlocked (connection);
 
   _dbus_verbose ("Returning popped message %p\n", message);    
-  
+
+  _dbus_connection_release_dispatch (connection);
   CONNECTION_UNLOCK (connection);
   
   return message;
 }
 
 /**
- * Acquire the dispatcher. This must be done before dispatching
- * messages in order to guarantee the right order of
- * message delivery. May sleep and drop the connection mutex
- * while waiting for the dispatcher.
+ * Acquire the dispatcher. This is a separate lock so the main
+ * connection lock can be dropped to call out to application dispatch
+ * handlers.
  *
  * @param connection the connection.
  */
 static void
 _dbus_connection_acquire_dispatch (DBusConnection *connection)
 {
-  if (connection->dispatch_acquired)
-    dbus_condvar_wait (connection->dispatch_cond, connection->mutex);
+  HAVE_LOCK_CHECK (connection);
+
+  _dbus_connection_ref_unlocked (connection);
+  CONNECTION_UNLOCK (connection);
+  
+  _dbus_verbose ("%s locking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  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_assert (!connection->dispatch_acquired);
 
   connection->dispatch_acquired = TRUE;
+
+  _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->dispatch_mutex);
+  
+  CONNECTION_LOCK (connection);
+  _dbus_connection_unref_unlocked (connection);
 }
 
 /**
@@ -2430,10 +2788,18 @@
 static void
 _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_assert (connection->dispatch_acquired);
 
   connection->dispatch_acquired = FALSE;
   dbus_condvar_wake_one (connection->dispatch_cond);
+
+  _dbus_verbose ("%s unlocking dispatch_mutex\n", _DBUS_FUNCTION_NAME);
+  dbus_mutex_unlock (connection->dispatch_mutex);
 }
 
 static void
@@ -2448,6 +2814,8 @@
 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))
@@ -2514,7 +2882,7 @@
   DBusDispatchStatusFunction function;
   void *data;
 
-  /* We have the lock */
+  HAVE_LOCK_CHECK (connection);
 
   _dbus_connection_ref_unlocked (connection);
 
@@ -2553,6 +2921,8 @@
   DBusDispatchStatus status;
 
   _dbus_return_val_if_fail (connection != NULL, DBUS_DISPATCH_COMPLETE);
+
+  _dbus_verbose ("%s start\n", _DBUS_FUNCTION_NAME);
   
   CONNECTION_LOCK (connection);
 
@@ -2576,16 +2946,12 @@
  *
  * @todo some FIXME in here about handling DBUS_HANDLER_RESULT_NEED_MEMORY
  *
- * @todo right now a message filter gets run on replies to a pending
- * call in here, but not in the case where we block without entering
- * the main loop. Simple solution might be to just have the pending
- * call stuff run before the filters.
- *
  * @todo FIXME what if we call out to application code to handle a
  * message, holding the dispatch lock, and the application code runs
  * the main loop and dispatches again? Probably deadlocks at the
  * moment. Maybe we want a dispatch status of DBUS_DISPATCH_IN_PROGRESS,
- * and then the GSource etc. could handle the situation?
+ * and then the GSource etc. could handle the situation? Right now
+ * our GSource is NO_RECURSE
  * 
  * @param connection the connection
  * @returns dispatch status
@@ -2619,19 +2985,14 @@
   _dbus_connection_ref_unlocked (connection);
 
   _dbus_connection_acquire_dispatch (connection);
-  
-  /* This call may drop the lock during the execution (if waiting for
-   * borrowed messages to be returned) but the order of message
-   * dispatch if several threads call dispatch() is still
-   * protected by the lock, since only one will get the lock, and that
-   * one will finish the message dispatching
-   */
+  HAVE_LOCK_CHECK (connection);
+
   message_link = _dbus_connection_pop_message_link_unlocked (connection);
   if (message_link == NULL)
     {
       /* another thread dispatched our stuff */
 
-      _dbus_verbose ("another thread dispatched message\n");
+      _dbus_verbose ("another thread dispatched message (during acquire_dispatch above)\n");
       
       _dbus_connection_release_dispatch (connection);
 
@@ -2656,23 +3017,44 @@
                  dbus_message_get_member (message) :
                  "no member",
                  dbus_message_get_signature (message));
-  
-  result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
 
+  result = DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
+  
+  /* Pending call handling must be first, because if you do
+   * dbus_connection_send_with_reply_and_block() or
+   * dbus_pending_call_block() then no handlers/filters will be run on
+   * the reply. We want consistent semantics in the case where we
+   * dbus_connection_dispatch() the reply.
+   */
+  
   reply_serial = dbus_message_get_reply_serial (message);
   pending = _dbus_hash_table_lookup_int (connection->pending_replies,
                                          reply_serial);
+  if (pending)
+    {
+      _dbus_verbose ("Dispatching a pending reply\n");
+      _dbus_pending_call_complete_and_unlock (pending, message);
+      pending = NULL; /* it's probably unref'd */
+      
+      CONNECTION_LOCK (connection);
+      _dbus_verbose ("pending call completed in dispatch\n");
+      result = DBUS_HANDLER_RESULT_HANDLED;
+      goto out;
+    }
   
   if (!_dbus_list_copy (&connection->filter_list, &filter_list_copy))
     {
       _dbus_connection_release_dispatch (connection);
-
+      HAVE_LOCK_CHECK (connection);
+      
       _dbus_connection_failed_pop (connection, message_link);
 
       /* unlocks and calls user code */
       _dbus_connection_update_dispatch_status_and_unlock (connection,
                                                           DBUS_DISPATCH_NEED_MEMORY);
 
+      if (pending)
+        dbus_pending_call_unref (pending);
       dbus_connection_unref (connection);
       
       return DBUS_DISPATCH_NEED_MEMORY;
@@ -2714,40 +3096,11 @@
       _dbus_verbose ("No memory in %s\n", _DBUS_FUNCTION_NAME);
       goto out;
     }
-  
-  /* Did a reply we were waiting on get filtered? */
-  if (pending && result == DBUS_HANDLER_RESULT_HANDLED)
-    {
-      /* Queue the timeout immediately! */
-      if (pending->timeout_link)
-	{
-	  _dbus_connection_queue_synthesized_message_link (connection,
-							   pending->timeout_link);
-	  pending->timeout_link = NULL;
-	}
-      else
-	{
-	  /* We already queued the timeout? Then it was filtered! */
-	  _dbus_warn ("The timeout error with reply serial %d was filtered, so the DBusPendingCall will never stop pending.\n", reply_serial);
-	}
-    }
-  
-  if (result == DBUS_HANDLER_RESULT_HANDLED)
+  else if (result == DBUS_HANDLER_RESULT_HANDLED)
     {
       _dbus_verbose ("filter handled message in dispatch\n");
       goto out;
     }
-  
-  if (pending)
-    {
-      _dbus_pending_call_complete_and_unlock (pending, message);
-
-      pending = NULL;
-      
-      CONNECTION_LOCK (connection);
-      _dbus_verbose ("pending call completed in dispatch\n");
-      goto out;
-    }
 
   /* We're still protected from dispatch() reentrancy here
    * since we acquired the dispatcher
@@ -2762,7 +3115,8 @@
                  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);
   
@@ -2791,8 +3145,9 @@
         }
               
       if (!_dbus_string_append_printf (&str,
-                                       "Method \"%s\" on interface \"%s\" doesn't exist\n",
+                                       "Method \"%s\" with signature \"%s\" on interface \"%s\" doesn't exist\n",
                                        dbus_message_get_member (message),
+                                       dbus_message_get_signature (message),
                                        dbus_message_get_interface (message)))
         {
           _dbus_string_free (&str);
@@ -2823,8 +3178,8 @@
           goto out;
         }
 
-      _dbus_connection_send_preallocated_unlocked (connection, preallocated,
-                                                   reply, NULL);
+      _dbus_connection_send_preallocated_unlocked_no_update (connection, preallocated,
+                                                             reply, NULL);
 
       dbus_message_unref (reply);
       
@@ -2876,7 +3231,9 @@
     }
   
   _dbus_connection_release_dispatch (connection);
-  
+  HAVE_LOCK_CHECK (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 */
@@ -2954,25 +3311,43 @@
                                      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;
 }
 
@@ -3018,17 +3393,34 @@
 					 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 */

Index: dbus-pending-call.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-pending-call.c,v
retrieving revision 1.8
retrieving revision 1.8.2.1
diff -u -d -r1.8 -r1.8.2.1
--- dbus-pending-call.c	10 Aug 2004 03:07:00 -0000	1.8
+++ dbus-pending-call.c	16 Feb 2005 22:45:40 -0000	1.8.2.1
@@ -56,11 +56,19 @@
   DBusPendingCall *pending;
   DBusTimeout *timeout;
 
-  _dbus_return_val_if_fail (timeout_milliseconds >= 0 || timeout_milliseconds == -1, FALSE);
+  _dbus_assert (timeout_milliseconds >= 0 || timeout_milliseconds == -1);
   
   if (timeout_milliseconds == -1)
     timeout_milliseconds = _DBUS_DEFAULT_TIMEOUT_VALUE;
 
+  /* it would probably seem logical to pass in _DBUS_INT_MAX for
+   * infinite timeout, but then math in
+   * _dbus_connection_block_for_reply would get all overflow-prone, so
+   * smack that down.
+   */
+  if (timeout_milliseconds > _DBUS_ONE_HOUR_IN_MILLISECONDS * 6)
+    timeout_milliseconds = _DBUS_ONE_HOUR_IN_MILLISECONDS * 6;
+  
   if (!dbus_pending_call_allocate_data_slot (&notify_user_data_slot))
     return NULL;
   
@@ -102,6 +110,8 @@
 void
 _dbus_pending_call_notify (DBusPendingCall *pending)
 {
+  _dbus_assert (!pending->completed);
+  
   pending->completed = TRUE;
 
   if (pending->function)
@@ -227,11 +237,11 @@
 }
 
 /**
- * Cancels the pending call, such that any reply
- * or error received will just be ignored.
- * Drops at least one reference to the #DBusPendingCall
- * so will free the call if nobody else is holding
- * a reference.
+ * Cancels the pending call, such that any reply or error received
+ * will just be ignored.  Drops the dbus library's internal reference
+ * to the #DBusPendingCall so will free the call if nobody else is
+ * holding a reference. However you usually get a reference
+ * from dbus_connection_send() so probably your app owns a ref also.
  * 
  * @param pending the pending call
  */
@@ -258,21 +268,26 @@
 }
 
 /**
- * Gets the reply, or returns #NULL if none has been received yet. The
- * reference count is not incremented on the returned message, so you
- * have to keep a reference count on the pending call (or add one
- * to the message).
- *
- * @todo not thread safe? I guess it has to lock though it sucks
- * @todo maybe to make this threadsafe, it should be steal_reply(), i.e. only one thread can ever get the message
+ * Gets the reply, or returns #NULL if none has been received
+ * yet. Ownership of the reply message passes to the caller. This
+ * function can only be called once per pending call, since the reply
+ * message is tranferred to the caller.
  * 
  * @param pending the pending call
  * @returns the reply message or #NULL.
  */
 DBusMessage*
-dbus_pending_call_get_reply (DBusPendingCall *pending)
+dbus_pending_call_steal_reply (DBusPendingCall *pending)
 {
-  return pending->reply;
+  DBusMessage *message;
+  
+  _dbus_return_val_if_fail (pending->completed, NULL);
+  _dbus_return_val_if_fail (pending->reply != NULL, NULL);
+  
+  message = pending->reply;
+  pending->reply = NULL;
+
+  return message;
 }
 
 /**
@@ -292,20 +307,7 @@
 void
 dbus_pending_call_block (DBusPendingCall *pending)
 {
-  DBusMessage *message;
-
-  if (dbus_pending_call_get_completed (pending))
-    return;
-
-  /* message may be NULL if no reply */
-  message = _dbus_connection_block_for_reply (pending->connection,
-                                              pending->reply_serial,
-                                              dbus_timeout_get_interval (pending->timeout));
-
-  _dbus_connection_lock (pending->connection);
-  _dbus_pending_call_complete_and_unlock (pending, message);
-  if (message)
-    dbus_message_unref (message);
+  _dbus_connection_block_pending_call (pending);
 }
 
 static DBusDataSlotAllocator slot_allocator;

Index: dbus-pending-call.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-pending-call.h,v
retrieving revision 1.6
retrieving revision 1.6.2.1
diff -u -d -r1.6 -r1.6.2.1
--- dbus-pending-call.h	9 Sep 2004 10:20:17 -0000	1.6
+++ dbus-pending-call.h	16 Feb 2005 22:45:40 -0000	1.6.2.1
@@ -41,7 +41,7 @@
                                               DBusFreeFunction               free_user_data);
 void         dbus_pending_call_cancel        (DBusPendingCall               *pending);
 dbus_bool_t  dbus_pending_call_get_completed (DBusPendingCall               *pending);
-DBusMessage* dbus_pending_call_get_reply     (DBusPendingCall               *pending);
+DBusMessage* dbus_pending_call_steal_reply   (DBusPendingCall               *pending);
 void         dbus_pending_call_block         (DBusPendingCall               *pending);
 
 dbus_bool_t dbus_pending_call_allocate_data_slot (dbus_int32_t     *slot_p);

Index: dbus-server-protected.h
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-protected.h,v
retrieving revision 1.14
retrieving revision 1.14.2.1
diff -u -d -r1.14 -r1.14.2.1
--- dbus-server-protected.h	9 Sep 2004 10:20:17 -0000	1.14
+++ dbus-server-protected.h	16 Feb 2005 22:45:40 -0000	1.14.2.1
@@ -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 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 @@
   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 @@
                                          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
 

Index: dbus-server-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server-unix.c,v
retrieving revision 1.24
retrieving revision 1.24.2.1
diff -u -d -r1.24 -r1.24.2.1
--- dbus-server-unix.c	26 Nov 2004 01:53:13 -0000	1.24
+++ dbus-server-unix.c	16 Feb 2005 22:45:40 -0000	1.24.2.1
@@ -72,21 +72,29 @@
  */
 /* 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 @@
                                             (const char **) server->auth_mechanisms))
     {
       _dbus_transport_unref (transport);
+      SERVER_UNLOCK (server);
       return FALSE;
     }
   
@@ -103,19 +112,27 @@
   
   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 @@
   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 @@
           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 @@
       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_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;
@@ -366,7 +395,7 @@
 
   if (host == NULL)
     host = "localhost";
-  
+
   if (!_dbus_string_append (&address, "tcp:host=") ||
       !_dbus_string_append (&address, host) ||
       !_dbus_string_append (&address, ",port=") ||

Index: dbus-server.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-server.c,v
retrieving revision 1.36
retrieving revision 1.36.2.1
diff -u -d -r1.36 -r1.36.2.1
--- dbus-server.c	10 Aug 2004 03:07:00 -0000	1.36
+++ dbus-server.c	16 Feb 2005 22:45:40 -0000	1.36.2.1
@@ -67,7 +67,7 @@
                         const DBusString       *address)
 {
   server->vtable = vtable;
-  server->refcount = 1;
+  server->refcount.value = 1;
 
   server->address = NULL;
   server->watches = NULL;
@@ -75,6 +75,10 @@
   
   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 @@
   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 @@
  */
 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_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_server_add_watch (DBusServer *server,
                         DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   return _dbus_watch_list_add_watch (server->watches, watch);
 }
 
@@ -159,6 +171,7 @@
 _dbus_server_remove_watch  (DBusServer *server,
                             DBusWatch  *watch)
 {
+  HAVE_LOCK_CHECK (server);
   _dbus_watch_list_remove_watch (server->watches, watch);
 }
 
@@ -176,6 +189,8 @@
                            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_server_add_timeout (DBusServer  *server,
 			  DBusTimeout *timeout)
 {
+  HAVE_LOCK_CHECK (server);
+  
   return _dbus_timeout_list_add_timeout (server->timeouts, timeout);
 }
 
@@ -207,6 +224,8 @@
 _dbus_server_remove_timeout (DBusServer  *server,
 			     DBusTimeout *timeout)
 {
+  HAVE_LOCK_CHECK (server);
+  
   _dbus_timeout_list_remove_timeout (server->timeouts, timeout);  
 }
 
@@ -224,6 +243,8 @@
                              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 @@
 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 @@
 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 @@
 }
 
 /**
+ * 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 @@
 dbus_server_disconnect (DBusServer *server)
 {
   _dbus_return_if_fail (server != NULL);
+
+  SERVER_LOCK (server);
   
   _dbus_assert (server->vtable->disconnect != NULL);
 
@@ -507,6 +569,8 @@
   
   (* server->vtable->disconnect) (server);
   server->disconnected = TRUE;
+
+  SERVER_UNLOCK (server);
 }
 
 /**
@@ -517,9 +581,15 @@
 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 @@
 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 @@
                                          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 @@
                                  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 @@
 				   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 @@
   char **copy;
 
   _dbus_return_val_if_fail (server != NULL, FALSE);
+
+  SERVER_LOCK (server);
   
   if (mechanisms != NULL)
     {
@@ -660,6 +788,8 @@
   dbus_free_string_array (server->auth_mechanisms);
   server->auth_mechanisms = copy;
 
+  SERVER_UNLOCK (server);
+  
   return TRUE;
 }
 
@@ -732,19 +862,16 @@
   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_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;
 }

Index: dbus-transport-unix.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-transport-unix.c,v
retrieving revision 1.44
retrieving revision 1.44.2.1
diff -u -d -r1.44 -r1.44.2.1
--- dbus-transport-unix.c	26 Nov 2004 02:29:00 -0000	1.44
+++ dbus-transport-unix.c	16 Feb 2005 22:45:40 -0000	1.44.2.1
@@ -70,6 +70,8 @@
 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 @@
       _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 @@
 unix_disconnect (DBusTransport *transport)
 {
   DBusTransportUnix *unix_transport = (DBusTransportUnix*) transport;
+
+  _dbus_verbose ("%s\n", _DBUS_FUNCTION_NAME);
   
   free_watches (transport);
   
@@ -1004,7 +1012,10 @@
        * 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 @@
 	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)
         {

Index: dbus-transport.c
===================================================================
RCS file: /cvs/dbus/dbus/dbus/dbus-transport.c,v
retrieving revision 1.42
retrieving revision 1.42.2.1
diff -u -d -r1.42 -r1.42.2.1
--- dbus-transport.c	26 Nov 2004 01:53:13 -0000	1.42
+++ dbus-transport.c	16 Feb 2005 22:45:40 -0000	1.42.2.1
@@ -388,10 +388,12 @@
 {
   _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 @@
 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 @@
  * 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 @@
       
       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 @@
 
           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 @@
                                  " 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 @@
         }
 
       transport->authenticated = maybe_authenticated;
-      
-      return transport->authenticated;
+
+      _dbus_connection_unref_unlocked (transport->connection);
+      return maybe_authenticated;
     }
 }
 
@@ -670,6 +701,8 @@
   (* transport->vtable->do_iteration) (transport, flags,
                                        timeout_milliseconds);
   _dbus_transport_unref (transport);
+
+  _dbus_verbose ("%s end\n", _DBUS_FUNCTION_NAME);
 }
 
 static dbus_bool_t



More information about the dbus-commit mailing list