When attaching counters to messages, don't automatically notify callbacks

In all the places where counters are added, we're under a lock. The caller
knows what effect adding the counter might have, and can replicate it
in a lock-safe way if necessary.

Reviewed-by: Colin Walters <walters@verbum.org>
Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34393

diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
index a917f9a..3e09870 100644 (file)
--- a/dbus/dbus-connection.c
+++ b/dbus/dbus-connection.c
@@ -647,9 +647,12 @@ _dbus_connection_message_sent_unlocked (DBusConnection *connection,
                  dbus_message_get_signature (message),
                  connection, connection->n_outgoing);
 
-  /* Save this link in the link cache also */
+  /* It's OK that in principle we call the notify function, because for the
+   * outgoing limit, there isn't one */
   _dbus_message_remove_counter (message, connection->outgoing_counter,
                                 &link);
+
+  /* Save this link in the link cache also */
   _dbus_list_prepend_link (&connection->link_cache, link);
   
   dbus_message_unref (message);
@@ -1977,6 +1980,8 @@ _dbus_connection_send_preallocated_unlocked_no_update (DBusConnection       *con
   _dbus_list_prepend_link (&connection->outgoing_messages,
                            preallocated->queue_link);
 
+  /* It's OK that we'll never call the notify function, because for the
+   * outgoing limit, there isn't one */
   _dbus_message_add_counter_link (message,
                                   preallocated->counter_link);
 

diff --git a/dbus/dbus-message.c b/dbus/dbus-message.c
index 92c8325..faaf3e1 100644 (file)
--- a/dbus/dbus-message.c
+++ b/dbus/dbus-message.c
@@ -218,6 +218,11 @@ dbus_message_set_serial (DBusMessage   *message,
  * itself not incremented.  Ownership of link and counter refcount is
  * passed to the message.
  *
+ * This function may be called with locks held. As a result, the counter's
+ * notify function is not called; the caller is expected to either call
+ * _dbus_counter_notify() on the counter when they are no longer holding
+ * locks, or take the same action that would be taken by the notify function.
+ *
  * @param message the message
  * @param link link with counter as data
  */
@@ -254,8 +259,6 @@ _dbus_message_add_counter_link (DBusMessage  *message,
 #ifdef HAVE_UNIX_FD_PASSING
   _dbus_counter_adjust_unix_fd (link->data, message->unix_fd_counter_delta);
 #endif
-
-  _dbus_counter_notify (link->data);
 }
 
 /**
@@ -263,6 +266,11 @@ _dbus_message_add_counter_link (DBusMessage  *message,
  * of this message, and decremented by the size/unix fds of this
  * message when this message if finalized.
  *
+ * This function may be called with locks held. As a result, the counter's
+ * notify function is not called; the caller is expected to either call
+ * _dbus_counter_notify() on the counter when they are no longer holding
+ * locks, or take the same action that would be taken by the notify function.
+ *
  * @param message the message
  * @param counter the counter
  * @returns #FALSE if no memory

diff --git a/dbus/dbus-transport.c b/dbus/dbus-transport.c
index 3cbe799..d9acb20 100644 (file)
--- a/dbus/dbus-transport.c
+++ b/dbus/dbus-transport.c
@@ -1144,6 +1144,13 @@ _dbus_transport_queue_messages (DBusTransport *transport)
         }
       else
         {
+          /* We didn't call the notify function when we added the counter, so
+           * catch up now. Since we have the connection's lock, it's desirable
+           * that we bypass the notify function and call this virtual method
+           * directly. */
+          if (transport->vtable->live_messages_changed)
+            (* transport->vtable->live_messages_changed) (transport);
+
           /* pass ownership of link and message ref to connection */
           _dbus_connection_queue_received_message_link (transport->connection,
                                                         link);