* 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

1 files changed, 222 insertions, 70 deletions

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;
 }