GNOME Bugzilla – Bug 710726
Work around D-Bus bug with path_namespace='/' match rules
Last modified: 2013-11-06 14:54:22 UTC
Currently, if a GDBusObjectManagerClient is constructed with an object_path of ‘/’, it will add the following match rule: string "type='signal',sender=':1.94',path_namespace='/'" It turns out that D-Bus (version 1.6.12) is buggy with path_namespace='/', and doesn’t propagate any signals from its sub-paths properly. I’ll link to an upstream bug report shortly. This can be worked around in GIO by removing the path_namespace='/' part of the match rule for this special case. This isn’t particularly ugly, because path_namespace='/' is a no-op in any case. Patch coming.
Created attachment 257928 [details] [review] gdbus: Work around a D-Bus bug with path_namespace='/' match rules D-Bus versions < 1.6.19 (i.e. all current versions) have a bug with the path_namespace='/' match rule key. It should conceptually match everything, but actually matches nothing. This results in no property change (or other) signals being forwarded to the D-Bus client. The work-around implemented in GDBusObjectManagerClient is to remove the path_namespace match key if its value is ‘/’. For the upstream D-Bus bug, see: https://bugs.freedesktop.org/show_bug.cgi?id=70799
(In reply to comment #1) > D-Bus versions < 1.6.19 (i.e. all current versions) Assuming someone reviews my dbus patch before the next stable release, this will be fixed in dbus 1.6.18, not 1.6.19. The comment in the patch has the same error. Other than that, looks fine to me!
Review of attachment 257928 [details] [review]: Looks good and is even a nice (small) optimisation in any case. Please commit after the small change requested. ::: gio/gdbusobjectmanagerclient.c @@ +1111,3 @@ + * + * See: https://bugs.freedesktop.org/show_bug.cgi?id=70799 */ + if (g_strcmp0 (manager->priv->object_path, "/") == 0) Please use g_str_equal().
Comment on attachment 257928 [details] [review] gdbus: Work around a D-Bus bug with path_namespace='/' match rules Committed with the requested changes. commit 3b28df1e008101341504f82c8e65f3109aca10cc Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Oct 23 15:07:46 2013 +0100 gdbus: Work around a D-Bus bug with path_namespace='/' match rules D-Bus versions < 1.6.18 (i.e. all current versions) have a bug with the path_namespace='/' match rule key. It should conceptually match everything, but actually matches nothing. This results in no property change (or other) signals being forwarded to the D-Bus client. The work-around implemented in GDBusObjectManagerClient is to remove the path_namespace match key if its value is ‘/’. For the upstream D-Bus bug, see: https://bugs.freedesktop.org/show_bug.cgi?id=70799 https://bugzilla.gnome.org/show_bug.cgi?id=710726 gio/gdbusobjectmanagerclient.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)
Looks like the match rule handling in gdbusdaemon.c also needs fixing. Patch coming up.
Created attachment 257930 [details] [review] gdbus: Ensure message matching always succeeds against path_namespace='/' This copies the fix from upstream D-Bus bug https://bugs.freedesktop.org/show_bug.cgi?id=70799 to the GDBusDaemon implementation, ensuring that matching against path_namespace='/' succeeds for all keys (i.e. it’s a no-op).
Review of attachment 257930 [details] [review]: ::: gio/gdbusdaemon.c @@ +600,2 @@ if (!(g_str_has_prefix (value, element->value) && + (len <= 1 || value[len] == 0 || value[len] == '/'))) This condition is already hideous enough. I wonder if we could expand it out a bit: /* '/' matches everything */ if (len == 1) break;
Created attachment 257932 [details] [review] gdbus: Ensure message matching always succeeds against path_namespace='/' This copies the fix from upstream D-Bus bug https://bugs.freedesktop.org/show_bug.cgi?id=70799 to the GDBusDaemon implementation, ensuring that matching against path_namespace='/' succeeds for all keys (i.e. it’s a no-op).
Review of attachment 257932 [details] [review]: Thanks.
It would be great if someone D-Bus-ish (desrt?) could double-check my patch over on the freedesktop bug too.
Comment on attachment 257932 [details] [review] gdbus: Ensure message matching always succeeds against path_namespace='/' commit 856d90156942d340513942126c2536339e00d04a Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Oct 23 15:45:15 2013 +0100 gdbus: Ensure message matching always succeeds against path_namespace='/' This copies the fix from upstream D-Bus bug https://bugs.freedesktop.org/show_bug.cgi?id=70799 to the GDBusDaemon implementation, ensuring that matching against path_namespace='/' succeeds for all keys (i.e. it’s a no-op). https://bugzilla.gnome.org/show_bug.cgi?id=710726 gio/gdbusdaemon.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)
In order to avoid a dependency on GLib 2.39.x in libfolks, can I backport this fix to GLib 2.38.x? It’s fairly self-contained and quite important.
For what it's worth, the underlying bug is fixed in dbus-daemon 1.6.18 and 1.7.8. Upgrading GLib is easier than upgrading dbus-daemon, though (on the system bus, it doesn't require a reboot), so it would be good to have this fixed in any relevant stable GLib branches.
Cherry picking commits 3b28df1e008101341504f82c8e65f3109aca10cc and 856d90156942d340513942126c2536339e00d04a to the glib-2-38 branch works fine (no changes necessary) and all tests pass. Can I go ahead and push?
Yes. Please do.
(In reply to comment #14) > Can I go ahead and push? ACKed by desrt on IRC. Pushed to glib-2-38: commit 6e18d05a0d7399f74084a29c378ea04373f0cb28 Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Oct 23 15:45:15 2013 +0100 gdbus: Ensure message matching always succeeds against path_namespace='/' This copies the fix from upstream D-Bus bug https://bugs.freedesktop.org/show_bug.cgi?id=70799 to the GDBusDaemon implementation, ensuring that matching against path_namespace='/' succeeds for all keys (i.e. it’s a no-op). https://bugzilla.gnome.org/show_bug.cgi?id=710726 gio/gdbusdaemon.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) commit ffb9bb1f79e97f151aa8cb0352db3e81b4be9caf Author: Philip Withnall <philip.withnall@collabora.co.uk> Date: Wed Oct 23 15:07:46 2013 +0100 gdbus: Work around a D-Bus bug with path_namespace='/' match rules D-Bus versions < 1.6.18 (i.e. all current versions) have a bug with the path_namespace='/' match rule key. It should conceptually match everything, but actually matches nothing. This results in no property change (or other) signals being forwarded to the D-Bus client. The work-around implemented in GDBusObjectManagerClient is to remove the path_namespace match key if its value is ‘/’. For the upstream D-Bus bug, see: https://bugs.freedesktop.org/show_bug.cgi?id=70799 https://bugzilla.gnome.org/show_bug.cgi?id=710726 gio/gdbusobjectmanagerclient.c | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-)