Attachment 43819 Details for Bug 34393 – When attaching counters to messages, don't automatically notify callbacks

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

0006-When-attaching-counters-to-messages-don-t-automatica.patch (text/plain), 4.20 KB, created by Simon McVittie on 2011-02-25 10:23:48 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2011-02-25 10:23:48 UTC

Size: 4.20 KB

patch

obsolete

>From c1a3921a0729c85617f1ce07e39e668292f5497d Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>Date: Fri, 25 Feb 2011 17:21:44 +0000
>Subject: [PATCH 06/10] 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.
>
>Bug: https://bugs.freedesktop.org/show_bug.cgi?id=34393
>---
> dbus/dbus-connection.c |    7 ++++++-
> dbus/dbus-message.c    |   12 ++++++++++--
> dbus/dbus-transport.c  |    7 +++++++
> 3 files changed, 23 insertions(+), 3 deletions(-)
>
>diff --git a/dbus/dbus-connection.c b/dbus/dbus-connection.c
>index f8605c3..4d08204 100644
>--- a/dbus/dbus-connection.c
>+++ b/dbus/dbus-connection.c
>@@ -637,9 +637,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);
>@@ -1985,6 +1988,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 6d0ebb8..22e71a6 100644
>--- a/dbus/dbus-message.c
>+++ b/dbus/dbus-message.c
>@@ -217,6 +217,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
>  */
>@@ -253,8 +258,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);
> }
> 
> /**
>@@ -262,6 +265,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 ba6bd4b..8287dfa 100644
>--- 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);
>-- 
>1.7.4.1
>

Actions: View | Splinter Review

Attachments on bug 34393: 43814 | 43815 | 43816 | 43817 | 43818 | 43819 | 43820 | 43821 | 43822 | 43823 | 44207 | 44314 | 44317 | 44353 | 46155 | 46157 | 48281

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.