* configure.in: add -Wdeclaration-after-statement

* dbus/dbus-connection.c: change all the pending call stuff to
  reflect the fact that pending call operations use the connection
  lock

* dbus/dbus-pending-call.c: add locking here

* dbus/dbus-errors.c (struct DBusRealError): don't make the name
  field const consistent with how message field is done

6 files changed, 405 insertions, 191 deletions

diff --git a/ChangeLog b/ChangeLog
index 0d81e25f..9d58a6a3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,16 @@
+2006-08-04  Havoc Pennington  <hp@redhat.com>
+
+	* configure.in: add -Wdeclaration-after-statement
+
+	* dbus/dbus-connection.c: change all the pending call stuff to
+	reflect the fact that pending call operations use the connection
+	lock
+
+	* dbus/dbus-pending-call.c: add locking here
+
+	* dbus/dbus-errors.c (struct DBusRealError): don't make the name
+	field const consistent with how message field is done
+
 2006-08-03  John (J5) Palmieri  <johnp@redhat.com>
 
 	* s/D-BUS/D-Bus/g
diff --git a/configure.in b/configure.in
index 65736a89..dfad86ed 100644
--- a/configure.in
+++ b/configure.in
@@ -138,6 +138,11 @@ if test "x$GCC" = "xyes"; then
   *) CFLAGS="$CFLAGS -Wsign-compare" ;;
   esac
 
+  case " $CFLAGS " in
+  *[\ \	]-Wdeclaration-after-statement[\ \	]*) ;;
+  *) CFLAGS="$CFLAGS -Wdeclaration-after-statement" ;;
+  esac
+
   if test "x$enable_ansi" = "xyes"; then
     case " $CFLAGS " in
     *[\ \	]-ansi[\ \	]*) ;;
diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index 73888d41..2e942064 100644
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -260,6 +260,7 @@ static void               _dbus_connection_update_dispatch_status_and_unlock (DB
 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 DBusDispatchStatus _dbus_connection_flush_unlocked                    (DBusConnection     *connection);
 
 static DBusMessageFilter *
 _dbus_message_filter_ref (DBusMessageFilter *filter)
@@ -378,11 +379,11 @@ _dbus_connection_queue_received_message_link (DBusConnection  *connection,
                                              reply_serial);
       if (pending != NULL)
 	{
-	  if (_dbus_pending_call_is_timeout_added (pending))
+	  if (_dbus_pending_call_is_timeout_added_unlocked (pending))
             _dbus_connection_remove_timeout_unlocked (connection,
-                                                      _dbus_pending_call_get_timeout (pending));
+                                                      _dbus_pending_call_get_timeout_unlocked (pending));
 
-	  _dbus_pending_call_set_timeout_added (pending, FALSE);
+	  _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE);
 	}
     }
   
@@ -783,11 +784,11 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
 
   HAVE_LOCK_CHECK (connection);
 
-  reply_serial = _dbus_pending_call_get_reply_serial (pending);
+  reply_serial = _dbus_pending_call_get_reply_serial_unlocked (pending);
 
   _dbus_assert (reply_serial != 0);
 
-  timeout = _dbus_pending_call_get_timeout (pending);
+  timeout = _dbus_pending_call_get_timeout_unlocked (pending);
 
   if (!_dbus_connection_add_timeout_unlocked (connection, timeout))
     return FALSE;
@@ -798,14 +799,14 @@ _dbus_connection_attach_pending_call_unlocked (DBusConnection  *connection,
     {
       _dbus_connection_remove_timeout_unlocked (connection, timeout);
 
-      _dbus_pending_call_set_timeout_added (pending, FALSE);
+      _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE);
       HAVE_LOCK_CHECK (connection);
       return FALSE;
     }
   
-  _dbus_pending_call_set_timeout_added (pending, TRUE);
+  _dbus_pending_call_set_timeout_added_unlocked (pending, TRUE);
 
-  dbus_pending_call_ref (pending);
+  _dbus_pending_call_ref_unlocked (pending);
 
   HAVE_LOCK_CHECK (connection);
   
@@ -816,39 +817,44 @@ static void
 free_pending_call_on_hash_removal (void *data)
 {
   DBusPendingCall *pending;
-  DBusConnection  *connection; 
- 
+  DBusConnection  *connection;
+  
   if (data == NULL)
     return;
 
   pending = data;
 
-  connection = _dbus_pending_call_get_connection (pending);
+  connection = _dbus_pending_call_get_connection_unlocked (pending);
 
-  if (connection)
+  HAVE_LOCK_CHECK (connection);
+  
+  if (_dbus_pending_call_is_timeout_added_unlocked (pending))
     {
-      if (_dbus_pending_call_is_timeout_added (pending))
-        {
-          _dbus_connection_remove_timeout_unlocked (connection,
-                                                    _dbus_pending_call_get_timeout (pending));
+      _dbus_connection_remove_timeout_unlocked (connection,
+                                                _dbus_pending_call_get_timeout_unlocked (pending));
       
-          _dbus_pending_call_set_timeout_added (pending, FALSE);
-        }
-     
-      dbus_pending_call_unref (pending);
+      _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE);
     }
+
+  /* FIXME this is sort of dangerous and undesirable to drop the lock here, but
+   * the pending call finalizer could in principle call out to application code
+   * so we pretty much have to... some larger code reorg might be needed.
+   */
+  _dbus_connection_ref_unlocked (connection);
+  _dbus_pending_call_unref_and_unlock (pending);
+  CONNECTION_LOCK (connection);
+  _dbus_connection_unref_unlocked (connection);
 }
 
 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);
+  /* This ends up unlocking to call the pending call finalizer, which is unexpected to
+   * say the least.
+   */
   _dbus_hash_table_remove_int (connection->pending_replies,
-                               _dbus_pending_call_get_reply_serial (pending));
-  dbus_pending_call_unref (pending);
+                               _dbus_pending_call_get_reply_serial_unlocked (pending));
 }
 
 static void
@@ -858,12 +864,14 @@ _dbus_connection_detach_pending_call_and_unlock (DBusConnection  *connection,
   /* The idea here is to avoid finalizing the pending call
    * with the lock held, since there's a destroy notifier
    * in pending call that goes out to application code.
+   *
+   * There's an extra unlock inside the hash table
+   * "free pending call" function FIXME...
    */
-  dbus_pending_call_ref (pending);
+  _dbus_pending_call_ref_unlocked (pending);
   _dbus_hash_table_remove_int (connection->pending_replies,
-                               _dbus_pending_call_get_reply_serial (pending));
-  CONNECTION_UNLOCK (connection);
-  dbus_pending_call_unref (pending);
+                               _dbus_pending_call_get_reply_serial_unlocked (pending));
+  _dbus_pending_call_unref_and_unlock (pending);
 }
 
 /**
@@ -2311,14 +2319,13 @@ reply_handler_timeout (void *data)
   DBusDispatchStatus status;
   DBusPendingCall *pending = data;
 
-  connection = _dbus_pending_call_get_connection (pending);
-  
-  CONNECTION_LOCK (connection);
-  _dbus_pending_call_queue_timeout_error (pending, 
-                                          connection);
+  connection = _dbus_pending_call_get_connection_and_lock (pending);
+
+  _dbus_pending_call_queue_timeout_error_unlocked (pending, 
+                                                   connection);
   _dbus_connection_remove_timeout_unlocked (connection,
-				            _dbus_pending_call_get_timeout (pending));
-  _dbus_pending_call_set_timeout_added (pending, FALSE);
+				            _dbus_pending_call_get_timeout_unlocked (pending));
+  _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE);
 
   _dbus_verbose ("%s middle\n", _DBUS_FUNCTION_NAME);
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
@@ -2394,9 +2401,9 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
       return TRUE;
     }
 
-  pending = _dbus_pending_call_new (connection,
-                                    timeout_milliseconds,
-                                    reply_handler_timeout);
+  pending = _dbus_pending_call_new_unlocked (connection,
+                                             timeout_milliseconds,
+                                             reply_handler_timeout);
 
   if (pending == NULL)
     {
@@ -2412,7 +2419,7 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
       _dbus_message_set_serial (message, serial);
     }
 
-  if (!_dbus_pending_call_set_timeout_error (pending, message, serial))
+  if (!_dbus_pending_call_set_timeout_error_unlocked (pending, message, serial))
     goto error;
     
   /* Insert the serial in the pending replies hash;
@@ -2431,19 +2438,23 @@ dbus_connection_send_with_reply (DBusConnection     *connection,
     }
 
   if (pending_return)
-    *pending_return = pending;
+    *pending_return = pending; /* hand off refcount */
   else
     {
       _dbus_connection_detach_pending_call_unlocked (connection, pending);
-      dbus_pending_call_unref (pending);
+      /* we still have a ref to the pending call in this case, we unref
+       * after unlocking, below
+       */
     }
 
-  _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);
 
+  if (pending_return == NULL)
+    dbus_pending_call_unref (pending);
+  
   return TRUE;
 
  error:
@@ -2485,62 +2496,65 @@ check_for_reply_unlocked (DBusConnection *connection,
 static void
 connection_timeout_and_complete_all_pending_calls_unlocked (DBusConnection *connection)
 {
-  DBusHashIter iter;
-
-  _dbus_hash_iter_init (connection->pending_replies, &iter);
-
-  /* create list while we remove the iters from the hash
-     because we need to go over it a couple of times */
-  while (_dbus_hash_iter_next (&iter))
+   /* We can't iterate over the hash in the normal way since we'll be
+    * dropping the lock for each item. So we restart the
+    * iter each time as we drain the hash table.
+    */
+   
+   while (_dbus_hash_table_get_n_entries (connection->pending_replies) > 0)
     {
       DBusPendingCall *pending;
- 
+      DBusHashIter iter;
+      
+      _dbus_hash_iter_init (connection->pending_replies, &iter);
+      _dbus_hash_iter_next (&iter);
+       
       pending = (DBusPendingCall *) _dbus_hash_iter_get_value (&iter);
-      dbus_pending_call_ref (pending);
-     
-      _dbus_pending_call_queue_timeout_error (pending, 
-                                              connection);
+      _dbus_pending_call_ref_unlocked (pending);
+       
+      _dbus_pending_call_queue_timeout_error_unlocked (pending, 
+                                                       connection);
       _dbus_connection_remove_timeout_unlocked (connection,
-                                                _dbus_pending_call_get_timeout (pending));
-
-      _dbus_pending_call_set_timeout_added (pending, FALSE); 
+                                                _dbus_pending_call_get_timeout_unlocked (pending));
+      _dbus_pending_call_set_timeout_added_unlocked (pending, FALSE);       
       _dbus_hash_iter_remove_entry (&iter);
 
-      dbus_pending_call_unref (pending);
+      _dbus_pending_call_unref_and_unlock (pending);
+      CONNECTION_LOCK (connection);
     }
+  HAVE_LOCK_CHECK (connection);
 }
 
 static void
-complete_pending_call_and_unlock (DBusPendingCall *pending,
+complete_pending_call_and_unlock (DBusConnection  *connection,
+                                  DBusPendingCall *pending,
                                   DBusMessage     *message)
 {
-  _dbus_pending_call_set_reply (pending, message);
-  dbus_pending_call_ref (pending); /* in case there's no app with a ref held */
-  _dbus_connection_detach_pending_call_and_unlock (_dbus_pending_call_get_connection (pending), pending);
-  
+  _dbus_pending_call_set_reply_unlocked (pending, message);
+  _dbus_pending_call_ref_unlocked (pending); /* in case there's no app with a ref held */
+  _dbus_connection_detach_pending_call_and_unlock (connection, pending);
+ 
   /* Must be called unlocked since it invokes app callback */
   _dbus_pending_call_complete (pending);
   dbus_pending_call_unref (pending);
 }
 
 static dbus_bool_t
-check_for_reply_and_update_dispatch_unlocked (DBusPendingCall *pending)
+check_for_reply_and_update_dispatch_unlocked (DBusConnection  *connection,
+                                              DBusPendingCall *pending)
 {
   DBusMessage *reply;
   DBusDispatchStatus status;
-  DBusConnection *connection;
-
-  connection = _dbus_pending_call_get_connection (pending);
 
   reply = check_for_reply_unlocked (connection, 
-                                    _dbus_pending_call_get_reply_serial (pending));
+                                    _dbus_pending_call_get_reply_serial_unlocked (pending));
   if (reply != NULL)
     {
       _dbus_verbose ("%s checked for reply\n", _DBUS_FUNCTION_NAME);
 
       _dbus_verbose ("dbus_connection_send_with_reply_and_block(): got reply\n");
 
-      complete_pending_call_and_unlock (pending, reply);
+      complete_pending_call_and_unlock (connection, pending, reply);
       dbus_message_unref (reply);
 
       CONNECTION_LOCK (connection);
@@ -2606,24 +2620,21 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
   if (dbus_pending_call_get_completed (pending))
     return;
 
-  connection = _dbus_pending_call_get_connection (pending);
-  if (connection == NULL)
-    return; /* call already detached */
-
   dbus_pending_call_ref (pending); /* necessary because the call could be canceled */
-  client_serial = _dbus_pending_call_get_reply_serial (pending);
+
+  connection = _dbus_pending_call_get_connection_and_lock (pending);
+  
+  /* Flush message queue - note, can affect dispatch status */
+  _dbus_connection_flush_unlocked (connection);
+
+  client_serial = _dbus_pending_call_get_reply_serial_unlocked (pending);
 
   /* note that timeout_milliseconds is limited to a smallish value
    * in _dbus_pending_call_new() so overflows aren't possible
    * below
    */
-  timeout_milliseconds = dbus_timeout_get_interval (_dbus_pending_call_get_timeout (pending));
-
-  /* Flush message queue */
-  dbus_connection_flush (connection);
-
-  CONNECTION_LOCK (connection);
-
+  timeout_milliseconds = dbus_timeout_get_interval (_dbus_pending_call_get_timeout_unlocked (pending));
+  
   _dbus_get_current_time (&start_tv_sec, &start_tv_usec);
   end_tv_sec = start_tv_sec + timeout_milliseconds / 1000;
   end_tv_usec = start_tv_usec + (timeout_milliseconds % 1000) * 1000;
@@ -2638,7 +2649,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
 
   /* check to see if we already got the data off the socket */
   /* from another blocked pending call */
-  if (check_for_reply_and_update_dispatch_unlocked (pending))
+  if (check_for_reply_and_update_dispatch_unlocked (connection, pending))
     return;
 
   /* Now we wait... */
@@ -2661,7 +2672,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
   /* 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))
+  if (_dbus_pending_call_get_completed_unlocked (pending))
     {
       _dbus_verbose ("Pending call completed by dispatch in %s\n", _DBUS_FUNCTION_NAME);
       _dbus_connection_update_dispatch_status_and_unlock (connection, status);
@@ -2669,9 +2680,10 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
       return;
     }
   
-  if (status == DBUS_DISPATCH_DATA_REMAINS)
-    if (check_for_reply_and_update_dispatch_unlocked (pending))
-      return;  
+  if (status == DBUS_DISPATCH_DATA_REMAINS) {
+    if (check_for_reply_and_update_dispatch_unlocked (connection, pending))
+      return;
+  }
   
   _dbus_get_current_time (&tv_sec, &tv_usec);
   
@@ -2682,7 +2694,7 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
        * confusing
        */
       
-      complete_pending_call_and_unlock (pending, NULL);
+      complete_pending_call_and_unlock (connection, pending, NULL);
       dbus_pending_call_unref (pending);
       return;
     }
@@ -2723,10 +2735,10 @@ _dbus_connection_block_pending_call (DBusPendingCall *pending)
   _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);
 
-  _dbus_assert (!dbus_pending_call_get_completed (pending));
+  _dbus_assert (!_dbus_pending_call_get_completed_unlocked (pending));
   
   /* unlock and call user code */
-  complete_pending_call_and_unlock (pending, NULL);
+  complete_pending_call_and_unlock (connection, pending, NULL);
 
   /* update user code on dispatch status */
   CONNECTION_LOCK (connection);
@@ -2801,11 +2813,14 @@ dbus_connection_send_with_reply_and_block (DBusConnection     *connection,
 
 /**
  * Blocks until the outgoing message queue is empty.
+ * Assumes connection lock already held.
  *
+ * If you call this, you MUST call update_dispatch_status afterword...
+ * 
  * @param connection the connection.
  */
-void
-dbus_connection_flush (DBusConnection *connection)
+DBusDispatchStatus
+_dbus_connection_flush_unlocked (DBusConnection *connection)
 {
   /* We have to specify DBUS_ITERATION_DO_READING here because
    * otherwise we could have two apps deadlock if they are both doing
@@ -2814,9 +2829,8 @@ dbus_connection_flush (DBusConnection *connection)
    */
   DBusDispatchStatus status;
 
-  _dbus_return_if_fail (connection != NULL);
+  HAVE_LOCK_CHECK (connection);
   
-  CONNECTION_LOCK (connection);
   while (connection->n_outgoing > 0 &&
          _dbus_connection_get_is_connected_unlocked (connection))
     {
@@ -2834,6 +2848,31 @@ dbus_connection_flush (DBusConnection *connection)
   status = _dbus_connection_get_dispatch_status_unlocked (connection);
 
   HAVE_LOCK_CHECK (connection);
+  return status;
+}
+
+/**
+ * Blocks until the outgoing message queue is empty.
+ *
+ * @param connection the connection.
+ */
+void
+dbus_connection_flush (DBusConnection *connection)
+{
+  /* We have to specify DBUS_ITERATION_DO_READING here because
+   * otherwise we could have two apps deadlock if they are both doing
+   * a flush(), and the kernel buffers fill up. This could change the
+   * dispatch status.
+   */
+  DBusDispatchStatus status;
+
+  _dbus_return_if_fail (connection != NULL);
+  
+  CONNECTION_LOCK (connection);
+
+  status = _dbus_connection_flush_unlocked (connection);
+  
+  HAVE_LOCK_CHECK (connection);
   /* Unlocks and calls out to user code */
   _dbus_connection_update_dispatch_status_and_unlock (connection, status);
 
@@ -3590,7 +3629,7 @@ dbus_connection_dispatch (DBusConnection *connection)
   if (pending)
     {
       _dbus_verbose ("Dispatching a pending reply\n");
-      complete_pending_call_and_unlock (pending, message);
+      complete_pending_call_and_unlock (connection, pending, message);
       pending = NULL; /* it's probably unref'd */
       
       CONNECTION_LOCK (connection);
diff --git a/dbus/dbus-errors.c b/dbus/dbus-errors.c
index d8f8aa60..fb230852 100644
--- a/dbus/dbus-errors.c
+++ b/dbus/dbus-errors.c
@@ -40,7 +40,7 @@
  */
 typedef struct
 {
-  const char *name; /**< error name */
+  char *name; /**< error name */
   char *message; /**< error message */
 
   unsigned int const_message : 1; /**< Message is not owned by DBusError */
@@ -219,7 +219,7 @@ dbus_set_error_const (DBusError  *error,
   
   real = (DBusRealError *)error;
   
-  real->name = name;
+  real->name = (char*) name;
   real->message = (char *)message;
   real->const_message = TRUE;
 }
diff --git a/dbus/dbus-pending-call-internal.h b/dbus/dbus-pending-call-internal.h
index 0a5aa25c..03fbdb46 100644
--- a/dbus/dbus-pending-call-internal.h
+++ b/dbus/dbus-pending-call-internal.h
@@ -31,30 +31,35 @@
 
 DBUS_BEGIN_DECLS
 
-dbus_bool_t     _dbus_pending_call_is_timeout_added  (DBusPendingCall  *pending);
-void            _dbus_pending_call_set_timeout_added (DBusPendingCall  *pending,
-                                                      dbus_bool_t       is_added);
-DBusTimeout    *_dbus_pending_call_get_timeout       (DBusPendingCall  *pending);
-dbus_uint32_t   _dbus_pending_call_get_reply_serial  (DBusPendingCall  *pending);
-void            _dbus_pending_call_set_reply_serial  (DBusPendingCall *pending,
-                                                      dbus_uint32_t serial);
-DBusConnection *_dbus_pending_call_get_connection    (DBusPendingCall *pending);
-
-void              _dbus_pending_call_complete                  (DBusPendingCall    *pending);
-void              _dbus_pending_call_set_reply                 (DBusPendingCall    *pending,
-                                                                DBusMessage        *message);
-void              _dbus_pending_call_clear_connection          (DBusPendingCall *pending);
-
-void              _dbus_pending_call_queue_timeout_error       (DBusPendingCall *pending, 
-                                                                DBusConnection  *connection);
-void		  _dbus_pending_call_set_reply_serial  (DBusPendingCall *pending,
-                                                        dbus_uint32_t serial);
-dbus_bool_t       _dbus_pending_call_set_timeout_error (DBusPendingCall *pending,
-                                                        DBusMessage *message,
-                                                        dbus_uint32_t serial);
-DBusPendingCall*  _dbus_pending_call_new               (DBusConnection     *connection,
-                                                        int                 timeout_milliseconds,
-                                                        DBusTimeoutHandler  timeout_handler);
+dbus_bool_t      _dbus_pending_call_is_timeout_added_unlocked    (DBusPendingCall    *pending);
+void             _dbus_pending_call_set_timeout_added_unlocked   (DBusPendingCall    *pending,
+                                                                  dbus_bool_t         is_added);
+DBusTimeout    * _dbus_pending_call_get_timeout_unlocked         (DBusPendingCall    *pending);
+dbus_uint32_t    _dbus_pending_call_get_reply_serial_unlocked    (DBusPendingCall    *pending);
+void             _dbus_pending_call_set_reply_serial_unlocked    (DBusPendingCall    *pending,
+                                                                  dbus_uint32_t       serial);
+DBusConnection * _dbus_pending_call_get_connection_and_lock      (DBusPendingCall    *pending);
+DBusConnection * _dbus_pending_call_get_connection_unlocked      (DBusPendingCall    *pending);
+dbus_bool_t      _dbus_pending_call_get_completed_unlocked       (DBusPendingCall    *pending);
+void             _dbus_pending_call_complete                     (DBusPendingCall    *pending);
+void             _dbus_pending_call_set_reply_unlocked           (DBusPendingCall    *pending,
+                                                                  DBusMessage        *message);
+void             _dbus_pending_call_queue_timeout_error_unlocked (DBusPendingCall    *pending,
+                                                                  DBusConnection     *connection);
+void             _dbus_pending_call_set_reply_serial_unlocked    (DBusPendingCall    *pending,
+                                                                  dbus_uint32_t       serial);
+dbus_bool_t      _dbus_pending_call_set_timeout_error_unlocked   (DBusPendingCall    *pending,
+                                                                  DBusMessage        *message,
+                                                                  dbus_uint32_t       serial);
+DBusPendingCall* _dbus_pending_call_new_unlocked                 (DBusConnection     *connection,
+                                                                  int                 timeout_milliseconds,
+                                                                  DBusTimeoutHandler  timeout_handler);
+DBusPendingCall* _dbus_pending_call_ref_unlocked                 (DBusPendingCall    *pending);
+void             _dbus_pending_call_unref_and_unlock             (DBusPendingCall    *pending);
+dbus_bool_t      _dbus_pending_call_set_data_unlocked            (DBusPendingCall    *pending,
+                                                                  dbus_int32_t        slot,
+                                                                  void               *data,
+                                                                  DBusFreeFunction    free_data_func);
 
 
 DBUS_END_DECLS
diff --git a/dbus/dbus-pending-call.c b/dbus/dbus-pending-call.c
index da8a3100..e6ece9dd 100644
--- a/dbus/dbus-pending-call.c
+++ b/dbus/dbus-pending-call.c
@@ -44,6 +44,9 @@
  *
  * Opaque object representing a reply message that we're waiting for.
  */
+#define CONNECTION_LOCK(connection)   _dbus_connection_lock(connection)
+#define CONNECTION_UNLOCK(connection) _dbus_connection_unlock(connection)
+
 struct DBusPendingCall
 {
   DBusAtomic refcount;                            /**< reference count */
@@ -75,9 +78,9 @@ static dbus_int32_t notify_user_data_slot = -1;
  * @returns a new #DBusPendingCall or #NULL if no memory.
  */
 DBusPendingCall*
-_dbus_pending_call_new (DBusConnection    *connection,
-                        int                timeout_milliseconds,
-                        DBusTimeoutHandler timeout_handler)
+_dbus_pending_call_new_unlocked (DBusConnection    *connection,
+                                 int                timeout_milliseconds,
+                                 DBusTimeoutHandler timeout_handler)
 {
   DBusPendingCall *pending;
   DBusTimeout *timeout;
@@ -138,8 +141,8 @@ _dbus_pending_call_new (DBusConnection    *connection,
  *  to time out the call
  */
 void
-_dbus_pending_call_set_reply (DBusPendingCall *pending,
-                              DBusMessage     *message)
+_dbus_pending_call_set_reply_unlocked (DBusPendingCall *pending,
+                                       DBusMessage     *message)
 {
   if (message == NULL)
     {
@@ -187,9 +190,11 @@ _dbus_pending_call_complete (DBusPendingCall *pending)
 }
 
 void
-_dbus_pending_call_queue_timeout_error (DBusPendingCall *pending, 
-                                        DBusConnection *connection)
+_dbus_pending_call_queue_timeout_error_unlocked (DBusPendingCall *pending, 
+                                                 DBusConnection  *connection)
 {
+  _dbus_assert (connection == pending->connection);
+  
   if (pending->timeout_link)
     {
       _dbus_connection_queue_synthesized_message_link (connection,
@@ -205,7 +210,7 @@ _dbus_pending_call_queue_timeout_error (DBusPendingCall *pending,
  * @returns #TRUE if there is a timeout or #FALSE if not
  */
 dbus_bool_t 
-_dbus_pending_call_is_timeout_added (DBusPendingCall  *pending)
+_dbus_pending_call_is_timeout_added_unlocked (DBusPendingCall  *pending)
 {
   _dbus_assert (pending != NULL);
 
@@ -220,8 +225,8 @@ _dbus_pending_call_is_timeout_added (DBusPendingCall  *pending)
  * @param is_added whether or not a timeout is added
  */
 void
-_dbus_pending_call_set_timeout_added (DBusPendingCall  *pending,
-                                      dbus_bool_t       is_added)
+_dbus_pending_call_set_timeout_added_unlocked (DBusPendingCall  *pending,
+                                               dbus_bool_t       is_added)
 {
   _dbus_assert (pending != NULL);
 
@@ -236,7 +241,7 @@ _dbus_pending_call_set_timeout_added (DBusPendingCall  *pending,
  * @returns a timeout object 
  */
 DBusTimeout *
-_dbus_pending_call_get_timeout (DBusPendingCall  *pending)
+_dbus_pending_call_get_timeout_unlocked (DBusPendingCall  *pending)
 {
   _dbus_assert (pending != NULL);
 
@@ -250,7 +255,7 @@ _dbus_pending_call_get_timeout (DBusPendingCall  *pending)
  * @returns a serial number for the reply or 0 
  */
 dbus_uint32_t 
-_dbus_pending_call_get_reply_serial (DBusPendingCall  *pending)
+_dbus_pending_call_get_reply_serial_unlocked (DBusPendingCall  *pending)
 {
   _dbus_assert (pending != NULL);
 
@@ -264,8 +269,8 @@ _dbus_pending_call_get_reply_serial (DBusPendingCall  *pending)
  * @param serial the serial number 
  */
 void
-_dbus_pending_call_set_reply_serial  (DBusPendingCall *pending,
-                                      dbus_uint32_t serial)
+_dbus_pending_call_set_reply_serial_unlocked  (DBusPendingCall *pending,
+                                               dbus_uint32_t serial)
 {
   _dbus_assert (pending != NULL);
   _dbus_assert (pending->reply_serial == 0);
@@ -274,17 +279,32 @@ _dbus_pending_call_set_reply_serial  (DBusPendingCall *pending,
 }
 
 /**
- * Gets the connection associated with this pending call
+ * Gets the connection associated with this pending call.
+ *
+ * @param pending the pending_call
+ * @returns the connection associated with the pending call
+ */
+DBusConnection *
+_dbus_pending_call_get_connection_and_lock (DBusPendingCall *pending)
+{
+  _dbus_assert (pending != NULL);
+ 
+  CONNECTION_LOCK (pending->connection);
+  return pending->connection;
+}
+
+/**
+ * Gets the connection associated with this pending call.
  *
  * @param pending the pending_call
  * @returns the connection associated with the pending call
  */
 DBusConnection *
-_dbus_pending_call_get_connection (DBusPendingCall *pending)
+_dbus_pending_call_get_connection_unlocked (DBusPendingCall *pending)
 {
   _dbus_assert (pending != NULL);
  
-  return pending->connection; 
+  return pending->connection;
 }
 
 /**
@@ -296,9 +316,9 @@ _dbus_pending_call_get_connection (DBusPendingCall *pending)
  * @return #FALSE on OOM
  */
 dbus_bool_t
-_dbus_pending_call_set_timeout_error (DBusPendingCall *pending,
-                                      DBusMessage *message,
-                                      dbus_uint32_t serial)
+_dbus_pending_call_set_timeout_error_unlocked (DBusPendingCall *pending,
+                                               DBusMessage     *message,
+                                               dbus_uint32_t    serial)
 { 
   DBusList *reply_link;
   DBusMessage *reply;
@@ -321,7 +341,7 @@ _dbus_pending_call_set_timeout_error (DBusPendingCall *pending,
 
   pending->timeout_link = reply_link;
 
-  _dbus_pending_call_set_reply_serial (pending, serial);
+  _dbus_pending_call_set_reply_serial_unlocked (pending, serial);
   
   return TRUE;
 }
@@ -347,6 +367,21 @@ _dbus_pending_call_set_timeout_error (DBusPendingCall *pending,
  */
 
 /**
+ * Increments the reference count on a pending call,
+ * while the lock on its connection is already held.
+ *
+ * @param pending the pending call object
+ * @returns the pending call object
+ */
+DBusPendingCall *
+_dbus_pending_call_ref_unlocked (DBusPendingCall *pending)
+{
+  pending->refcount.value += 1;
+  
+  return pending;
+}
+
+/**
  * Increments the reference count on a pending call.
  *
  * @param pending the pending call object
@@ -357,11 +392,87 @@ dbus_pending_call_ref (DBusPendingCall *pending)
 {
   _dbus_return_val_if_fail (pending != NULL, NULL);
 
+  /* The connection lock is better than the global
+   * lock in the atomic increment fallback
+   */
+#ifdef DBUS_HAVE_ATOMIC_INT
   _dbus_atomic_inc (&pending->refcount);
+#else
+  CONNECTION_LOCK (pending->connection);
+  _dbus_assert (pending->refcount.value > 0);
 
+  pending->refcount.value += 1;
+  CONNECTION_UNLOCK (pending->connection);
+#endif
+  
   return pending;
 }
 
+static void
+_dbus_pending_call_last_unref (DBusPendingCall *pending)
+{
+  DBusConnection *connection;
+  
+  /* If we get here, we should be already detached
+   * from the connection, or never attached.
+   */
+  _dbus_assert (!pending->timeout_added);  
+
+  connection = pending->connection;
+
+  /* this assumes we aren't holding connection lock... */
+  _dbus_data_slot_list_free (&pending->slot_list);
+
+  if (pending->timeout != NULL)
+    _dbus_timeout_unref (pending->timeout);
+      
+  if (pending->timeout_link)
+    {
+      dbus_message_unref ((DBusMessage *)pending->timeout_link->data);
+      _dbus_list_free_link (pending->timeout_link);
+      pending->timeout_link = NULL;
+    }
+
+  if (pending->reply)
+    {
+      dbus_message_unref (pending->reply);
+      pending->reply = NULL;
+    }
+      
+  dbus_free (pending);
+
+  dbus_pending_call_free_data_slot (&notify_user_data_slot);
+
+  /* connection lock should not be held. */
+  /* Free the connection last to avoid a weird state while
+   * calling out to application code where the pending exists
+   * but not the connection.
+   */
+  dbus_connection_unref (connection);
+}
+
+/**
+ * Decrements the reference count on a pending call,
+ * freeing it if the count reaches 0. Assumes
+ * connection lock is already held.
+ *
+ * @param pending the pending call object
+ */
+void
+_dbus_pending_call_unref_and_unlock (DBusPendingCall *pending)
+{
+  dbus_bool_t last_unref;
+  
+  _dbus_assert (pending->refcount.value > 0);
+
+  pending->refcount.value -= 1;
+  last_unref = pending->refcount.value == 0;
+
+  CONNECTION_UNLOCK (pending->connection);
+  if (last_unref)
+    _dbus_pending_call_last_unref (pending);
+}
+
 /**
  * Decrements the reference count on a pending call,
  * freeing it if the count reaches 0.
@@ -375,40 +486,21 @@ dbus_pending_call_unref (DBusPendingCall *pending)
 
   _dbus_return_if_fail (pending != NULL);
 
+  /* More efficient to use the connection lock instead of atomic
+   * int fallback if we lack atomic int decrement
+   */
+#ifdef DBUS_HAVE_ATOMIC_INT
   last_unref = (_dbus_atomic_dec (&pending->refcount) == 1);
-
+#else
+  CONNECTION_LOCK (pending->connection);
+  _dbus_assert (pending->refcount.value > 0);
+  pending->refcount.value -= 1;
+  last_unref = pending->refcount.value == 0;
+  CONNECTION_UNLOCK (pending->connection);
+#endif
+  
   if (last_unref)
-    {
-      /* If we get here, we should be already detached
-       * from the connection, or never attached.
-       */
-      _dbus_assert (!pending->timeout_added);  
-     
-      dbus_connection_unref (pending->connection);
-
-      /* this assumes we aren't holding connection lock... */
-      _dbus_data_slot_list_free (&pending->slot_list);
-
-      if (pending->timeout != NULL)
-        _dbus_timeout_unref (pending->timeout);
-      
-      if (pending->timeout_link)
-        {
-          dbus_message_unref ((DBusMessage *)pending->timeout_link->data);
-          _dbus_list_free_link (pending->timeout_link);
-          pending->timeout_link = NULL;
-        }
-
-      if (pending->reply)
-        {
-          dbus_message_unref (pending->reply);
-          pending->reply = NULL;
-        }
-      
-      dbus_free (pending);
-
-      dbus_pending_call_free_data_slot (&notify_user_data_slot);
-    }
+    _dbus_pending_call_last_unref(pending);
 }
 
 /**
@@ -429,13 +521,17 @@ dbus_pending_call_set_notify (DBusPendingCall              *pending,
 {
   _dbus_return_val_if_fail (pending != NULL, FALSE);
 
+  CONNECTION_LOCK (pending->connection);
+  
   /* could invoke application code! */
-  if (!dbus_pending_call_set_data (pending, notify_user_data_slot,
-                                   user_data, free_user_data))
+  if (!_dbus_pending_call_set_data_unlocked (pending, notify_user_data_slot,
+                                             user_data, free_user_data))
     return FALSE;
   
   pending->function = function;
 
+  CONNECTION_UNLOCK (pending->connection);
+  
   return TRUE;
 }
 
@@ -451,23 +547,40 @@ dbus_pending_call_set_notify (DBusPendingCall              *pending,
 void
 dbus_pending_call_cancel (DBusPendingCall *pending)
 {
-  if (pending->connection)
-    _dbus_connection_remove_pending_call (pending->connection,
-                                          pending);
+  _dbus_connection_remove_pending_call (pending->connection,
+                                        pending);
 }
 
 /**
  * Checks whether the pending call has received a reply
- * yet, or not.
+ * yet, or not. Assumes connection lock is held.
  *
- * @todo not thread safe? I guess it has to lock though it sucks
+ * @param pending the pending call
+ * @returns #TRUE if a reply has been received
+ */
+dbus_bool_t
+_dbus_pending_call_get_completed_unlocked (DBusPendingCall    *pending)
+{
+  return pending->completed;
+}
+
+/**
+ * Checks whether the pending call has received a reply
+ * yet, or not.
  *
  * @param pending the pending call
- * @returns #TRUE if a reply has been received */
+ * @returns #TRUE if a reply has been received
+ */
 dbus_bool_t
 dbus_pending_call_get_completed (DBusPendingCall *pending)
 {
-  return pending->completed;
+  dbus_bool_t completed;
+  
+  CONNECTION_LOCK (pending->connection);
+  completed = pending->completed;
+  CONNECTION_UNLOCK (pending->connection);
+
+  return completed;
 }
 
 /**
@@ -486,10 +599,14 @@ dbus_pending_call_steal_reply (DBusPendingCall *pending)
   
   _dbus_return_val_if_fail (pending->completed, NULL);
   _dbus_return_val_if_fail (pending->reply != NULL, NULL);
+
+  CONNECTION_LOCK (pending->connection);
   
   message = pending->reply;
   pending->reply = NULL;
 
+  CONNECTION_UNLOCK (pending->connection);
+  
   return message;
 }
 
@@ -553,7 +670,7 @@ void
 dbus_pending_call_free_data_slot (dbus_int32_t *slot_p)
 {
   _dbus_return_if_fail (*slot_p >= 0);
-  
+
   _dbus_data_slot_allocator_free (&slot_allocator, slot_p);
 }
 
@@ -571,29 +688,62 @@ dbus_pending_call_free_data_slot (dbus_int32_t *slot_p)
  * @returns #TRUE if there was enough memory to store the data
  */
 dbus_bool_t
-dbus_pending_call_set_data (DBusPendingCall  *pending,
-                            dbus_int32_t      slot,
-                            void             *data,
-                            DBusFreeFunction  free_data_func)
+_dbus_pending_call_set_data_unlocked (DBusPendingCall  *pending,
+                                     dbus_int32_t      slot,
+                                     void             *data,
+                                     DBusFreeFunction  free_data_func)
 {
   DBusFreeFunction old_free_func;
   void *old_data;
   dbus_bool_t retval;
 
-  _dbus_return_val_if_fail (pending != NULL, FALSE);
-  _dbus_return_val_if_fail (slot >= 0, FALSE);
-
   retval = _dbus_data_slot_list_set (&slot_allocator,
                                      &pending->slot_list,
                                      slot, data, free_data_func,
                                      &old_free_func, &old_data);
 
+  /* Drop locks to call out to app code */
+  CONNECTION_UNLOCK (pending->connection);
+  
   if (retval)
     {
       if (old_free_func)
         (* old_free_func) (old_data);
     }
 
+  CONNECTION_LOCK (pending->connection);
+  
+  return retval;
+}
+
+/**
+ * Stores a pointer on a #DBusPendingCall, along
+ * with an optional function to be used for freeing
+ * the data when the data is set again, or when
+ * the pending call is finalized. The slot number
+ * must have been allocated with dbus_pending_call_allocate_data_slot().
+ *
+ * @param pending the pending_call
+ * @param slot the slot number
+ * @param data the data to store
+ * @param free_data_func finalizer function for the data
+ * @returns #TRUE if there was enough memory to store the data
+ */
+dbus_bool_t
+dbus_pending_call_set_data (DBusPendingCall  *pending,
+                            dbus_int32_t      slot,
+                            void             *data,
+                            DBusFreeFunction  free_data_func)
+{
+  dbus_bool_t retval;
+  
+  _dbus_return_val_if_fail (pending != NULL, FALSE);
+  _dbus_return_val_if_fail (slot >= 0, FALSE);
+
+  
+  CONNECTION_LOCK (pending->connection);
+  retval = _dbus_pending_call_set_data_unlocked (pending, slot, data, free_data_func);
+  CONNECTION_UNLOCK (pending->connection);
   return retval;
 }
 
@@ -613,9 +763,11 @@ dbus_pending_call_get_data (DBusPendingCall   *pending,
 
   _dbus_return_val_if_fail (pending != NULL, NULL);
 
+  CONNECTION_LOCK (pending->connection);
   res = _dbus_data_slot_list_get (&slot_allocator,
                                   &pending->slot_list,
                                   slot);
+  CONNECTION_UNLOCK (pending->connection);
 
   return res;
 }