From 120ed37a8c48619e2dc560b32655b15eb4c2b4d3 Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Thu, 29 Sep 2011 18:25:09 +0100 Subject: [PATCH] Don't subscribe to all D-Bus signals but only those the user is interested in See NB#189509, NB#141956, https://bugs.freedesktop.org/show_bug.cgi?id=23846 [Adjusted to apply to dbus-glib 0.96 -smcv] Bug: https://bugs.freedesktop.org/show_bug.cgi?id=23846 Bug-NB: NB#189509, NB#141956 --- dbus/dbus-gproxy.c | 227 +++++++++++++++++++++++++++++++++++++++++++--------- 1 files changed, 188 insertions(+), 39 deletions(-) diff --git a/dbus/dbus-gproxy.c b/dbus/dbus-gproxy.c index 16ee119..4ae5ecc 100644 --- a/dbus/dbus-gproxy.c +++ b/dbus/dbus-gproxy.c @@ -121,6 +121,20 @@ typedef struct { GSList *proxies; /**< The list of proxies */ + /** + * The application can connect several callbacks for the same quadruplet + * (name, path, interface, member). The callbacks can be on the same + * DBusGProxy or on several ones. In both cases, we add only one match rule + * to dbus-daemon. We need to count how many callbacks are registered so we + * know when to add or remove a match rule to dbus-daemon. + * + * signal_subscriptions counts the number of callbacks registered for each + * DBusGProxy. + * + * gchar *signal_name -> GHashTable* of (DBusGProxy *proxy -> guint count) + */ + GHashTable *signal_subscriptions; + char name[4]; /**< name (empty string for none), nul byte, * path, nul byte, * interface, nul byte @@ -467,6 +481,7 @@ g_proxy_list_new (DBusGProxy *first_proxy) priv->path, priv->interface); list->proxies = NULL; + list->signal_subscriptions = NULL; return list; } @@ -477,23 +492,35 @@ g_proxy_list_free (DBusGProxyList *list) /* we don't hold a reference to the proxies in the list, * as they ref the GProxyManager */ - g_slist_free (list->proxies); + g_slist_free (list->proxies); + if (list->signal_subscriptions != NULL) + { + g_hash_table_destroy (list->signal_subscriptions); + } g_free (list); } static char* -g_proxy_get_signal_match_rule (DBusGProxy *proxy) +g_proxy_get_signal_match_rule (DBusGProxy *proxy, const gchar *signal_name) { DBusGProxyPrivate *priv = DBUS_G_PROXY_GET_PRIVATE(proxy); /* FIXME Escaping is required here */ - + + g_assert (signal_name != NULL); if (priv->name) - return g_strdup_printf ("type='signal',sender='%s',path='%s',interface='%s'", - priv->name, priv->path, priv->interface); + { + return g_strdup_printf ("type='signal',sender='%s',path='%s'," + "interface='%s',member='%s'", + priv->name, priv->path, priv->interface, + signal_name); + } else - return g_strdup_printf ("type='signal',path='%s',interface='%s'", - priv->path, priv->interface); + { + return g_strdup_printf ("type='signal',path='%s',interface='%s'," + "member='%s'", + priv->path, priv->interface, signal_name); + } } static char * @@ -977,16 +1004,12 @@ dbus_g_proxy_manager_register (DBusGProxyManager *manager, * but only if the server is a message bus, * not if it's a peer. */ - char *rule; guint *refcount; - rule = g_proxy_get_signal_match_rule (proxy); - /* We don't check for errors; it's not like anyone would handle them, and - * we don't want a round trip here. - */ - dbus_bus_add_match (manager->connection, rule, NULL); - g_free (rule); - + g_assert (list->signal_subscriptions == NULL); + list->signal_subscriptions = g_hash_table_new_full (g_str_hash, + g_str_equal, g_free, (GDestroyNotify)g_hash_table_destroy); + refcount = g_hash_table_lookup (manager->owner_match_rules, priv->name); if (refcount != NULL) @@ -1105,17 +1128,33 @@ dbus_g_proxy_manager_unregister (DBusGProxyManager *manager, } } + if (list->signal_subscriptions != NULL) + { + GHashTableIter iter; + gpointer proxy_set; + gpointer signal_name; + + g_hash_table_iter_init (&iter, list->signal_subscriptions); + while (g_hash_table_iter_next (&iter, &signal_name, &proxy_set)) + { + g_hash_table_remove (proxy_set, proxy); + if (g_hash_table_size (proxy_set) == 0) + { + char *rule; + + rule = g_proxy_get_signal_match_rule (proxy, (gchar *) signal_name); + dbus_bus_remove_match (priv->manager->connection, + rule, NULL); + g_free (rule); + } + } + } + if (list->proxies == NULL) { - char *rule; g_hash_table_remove (manager->proxy_lists, tri); - rule = g_proxy_get_signal_match_rule (proxy); - dbus_bus_remove_match (manager->connection, - rule, NULL); - g_free (rule); - if (priv->name) { guint *refcount; @@ -1124,7 +1163,8 @@ dbus_g_proxy_manager_unregister (DBusGProxyManager *manager, if (*refcount == 0) { - rule = get_owner_match_rule (priv->name); + gchar *rule = get_owner_match_rule (priv->name); + dbus_bus_remove_match (manager->connection, rule, NULL); g_free (rule); @@ -3082,13 +3122,70 @@ dbus_g_proxy_connect_signal (DBusGProxy *proxy, GClosure *closure; GQuark q; DBusGProxyPrivate *priv; + char *tri; + DBusGProxyList *list; g_return_if_fail (DBUS_IS_G_PROXY (proxy)); g_return_if_fail (!DBUS_G_PROXY_DESTROYED (proxy)); g_return_if_fail (g_dbus_is_member_name (signal_name)); g_return_if_fail (handler != NULL); - + priv = DBUS_G_PROXY_GET_PRIVATE(proxy); + + tri = tristring_from_proxy (proxy); + list = g_hash_table_lookup (priv->manager->proxy_lists, tri); + g_free (tri); + /* We have to add match rules to the server, + * but only if the server is a message bus, + * not if it's a peer. + */ + if (priv->name) + { + gchar *orig_key; + GHashTable *proxy_set; + gboolean found; + gint handler_count; + gpointer value; + + g_assert (list->signal_subscriptions != NULL); + found = g_hash_table_lookup_extended + (list->signal_subscriptions, signal_name, (gpointer *) &orig_key, + (gpointer *) &proxy_set); + + if (!found || g_hash_table_size (proxy_set) == 0) + { + char *rule; + + rule = g_proxy_get_signal_match_rule (proxy, signal_name); + /* We don't check for errors; it's not like anyone would handle + * them, and we don't want a round trip here. + */ + dbus_bus_add_match (priv->manager->connection, + rule, NULL); + g_free (rule); + } + + if (!found) + { + proxy_set = g_hash_table_new (NULL, NULL); + orig_key = g_strdup (signal_name); + } + + if (g_hash_table_lookup_extended (proxy_set, proxy, NULL, + &value)) + { + handler_count = GPOINTER_TO_UINT (value) + 1; + } + else + { + handler_count = 1; + } + g_hash_table_insert (proxy_set, proxy, GINT_TO_POINTER (handler_count)); + + g_hash_table_steal (list->signal_subscriptions, signal_name); + g_hash_table_insert (list->signal_subscriptions, orig_key, proxy_set); + } + name = create_signal_name (priv->interface, signal_name); q = g_quark_try_string (name); @@ -3131,10 +3228,15 @@ dbus_g_proxy_disconnect_signal (DBusGProxy *proxy, GCallback handler, void *data) { + guint matched_count; + gint handler_count; char *name; GQuark q; DBusGProxyPrivate *priv; - + char *tri; + DBusGProxyList *list; + gchar *_signal_name; + g_return_if_fail (DBUS_IS_G_PROXY (proxy)); g_return_if_fail (!DBUS_G_PROXY_DESTROYED (proxy)); g_return_if_fail (g_dbus_is_member_name (signal_name)); @@ -3144,25 +3246,72 @@ dbus_g_proxy_disconnect_signal (DBusGProxy *proxy, name = create_signal_name (priv->interface, signal_name); q = g_quark_try_string (name); - - if (q != 0) - { - g_signal_handlers_disconnect_matched (G_OBJECT (proxy), - G_SIGNAL_MATCH_DETAIL | - G_SIGNAL_MATCH_FUNC | - G_SIGNAL_MATCH_DATA, - signals[RECEIVED], - q, - NULL, - G_CALLBACK (handler), data); - } - else + g_free (name); + + if (q == 0) { g_warning ("Attempt to disconnect from signal '%s' which is not registered\n", - name); + signal_name); + return; } - g_free (name); + /* signal_name may be freed by g_signal_handlers_disconnect_matched() :'( + * Keep a copy while we need it + */ + _signal_name = g_strdup (signal_name); + matched_count = g_signal_handlers_disconnect_matched (G_OBJECT (proxy), + G_SIGNAL_MATCH_DETAIL | G_SIGNAL_MATCH_FUNC | G_SIGNAL_MATCH_DATA, + signals[RECEIVED], q, NULL, G_CALLBACK (handler), data); + + tri = tristring_from_proxy (proxy); + list = g_hash_table_lookup (priv->manager->proxy_lists, tri); + g_free (tri); + + /* Remove a match rule */ + if (list->signal_subscriptions != NULL) + { + GHashTable *proxy_set = g_hash_table_lookup (list->signal_subscriptions, + _signal_name); + gpointer value; + + if (proxy_set == NULL || !g_hash_table_lookup_extended (proxy_set, proxy, + NULL, &value)) + { + g_warning ("Attempt to disconnect from signal '%s' which is not" + " registered\n", _signal_name); + g_free (_signal_name); + return; + } + + handler_count = GPOINTER_TO_INT (value); + handler_count -= matched_count; + g_assert (handler_count >= 0); + if (handler_count == 0) + { + g_hash_table_remove (proxy_set, proxy); + } + else + { + g_hash_table_insert (proxy_set, proxy, + GINT_TO_POINTER (handler_count)); + } + + if (g_hash_table_size (proxy_set) == 0) + { + char *rule; + + rule = g_proxy_get_signal_match_rule (proxy, _signal_name); + /* We don't check for errors; it's not like anyone would handle them, + * and we don't want a round trip here. + */ + dbus_bus_remove_match (priv->manager->connection, + rule, NULL); + g_free (rule); + + g_hash_table_remove (list->signal_subscriptions, _signal_name); + } + } + g_free (_signal_name); } /** -- 1.7.6.3