After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 781524 - gdbus ObjectManager confusion with Peer/Introspectable/Properties/ObjectManager interfaces (sd-bus)
gdbus ObjectManager confusion with Peer/Introspectable/Properties/ObjectManag...
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gdbus
2.50.x
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-04-20 10:38 UTC by David Härdeman
Modified: 2018-05-24 19:29 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description David Härdeman 2017-04-20 10:38:57 UTC
I'm currently trying to write a server using sd-bus (from systemd) and a client using gdbus and I've come across something I believe is a bug in gdbus.

I've implemented an ObjectManager server living at org.gnome.TestManager with the path /org/gnome/TestManager which exports objects under /org/gnome/TestManager/fooX with interface org.gnome.TestManager.Device.

sd-bus appears to interpret the ObjectManager API differently from gdbus, when an object is removed, an sd-bus based ObjectManager server sends the following signal:

signal time=1492642227.714223 sender=:1.104 -> destination=(null destination) serial=90 path=/org/gnome/TestManager; interface=org.freedesktop.DBus.ObjectMana
   object path "/org/gnome/TestManager/foo0"
   array [
      string "org.freedesktop.DBus.Peer"
      string "org.freedesktop.DBus.Introspectable"
      string "org.freedesktop.DBus.Properties"
      string "org.freedesktop.DBus.ObjectManager"
      string "org.gnome.TestManager.Device"
   ]

While a gdbus ObjectManager server sends:

signal time=1492642227.714223 sender=:1.104 -> destination=(null destination) serial=90 path=/org/gnome/TestManager; interface=org.freedesktop.DBus.ObjectMana
   object path "/org/gnome/TestManager/foo0"
   array [
      string "org.gnome.TestManager.Device"
   ]

I've already reported the difference in number of interfaces to the systemd mailing list. However, the difference also seems to expose a bug in gdbus.

If the sd-bus based server is already running and exposing an object (/org/gnome/TestManager/foo0) when I start the client, the client will report that the object has a single interface (org.gnome.TestManager.Device). If an object is added *while* the client is running, it will report the object as having five interfaces (and for the org.freedesktop.DBus.* interfaces, g_dbus_interface_get_info() will return NULL).

This means that when an object which existed at client startup is removed, gdbus will not trigger the "object-removed" signal because there is a mismatch in the interface count. This is checked in ./gio/gdbusobjectmanagerclient.c:remove_interfaces(), see the comparison of num_interfaces_to_remove == num_interfaces.

Now, the additional interfaces may be an sd-bus bug, but there also seems to be a bug in gdbus given the confusion over how many interfaces such an object is reported as having.

(Tested using the version of glib2.0 in Debian unstable, currently 2.50.0)
Comment 1 David Härdeman 2017-04-25 08:44:37 UTC
See also this thread on the systemd-devel mailing list:
https://lists.freedesktop.org/archives/systemd-devel/2017-April/038712.html

And the resulting system pull request:
https://github.com/systemd/systemd/pull/5799

That still leaves the questions of whether gdbus should be amended to report the "standard" interfaces for objects and the inconsistencies in g_dbus_interface_get_info() and robustness in remove_interfaces() when a *greater* number of interfaces are removed (maybe a simple num_interfaces_to_remove == num_interfaces is a bit too simple?).

I'll submit test cases later.
Comment 2 Philip Withnall 2017-04-25 10:09:34 UTC
Looks to me like GDBus is at fault here and should be reporting the built-in interfaces from ObjectManager signals and methods.

The specification is not explicit about this, although it does say that the intent of ObjectManager is “to make it easy to write a robust client implementation”, which we’re probably not achieving here.

https://dbus.freedesktop.org/doc/dbus-specification.html#standard-interfaces-objectmanager

smcv, do you have any clarifications to make on the spec?
Comment 3 Simon McVittie 2017-04-25 10:34:08 UTC
(In reply to Philip Withnall from comment #2)
> smcv, do you have any clarifications to make on the spec?

I didn't write that part of the spec - I think it was imported "whole" from text written by davidz.

I think it would be sensible to say that if an entirely new object is created, the server must signal that with an InterfacesAdded signal that lists the same interfaces that would have been listed by a GetManagedObjects() call made immediately after the addition; and if an object is removed entirely, the server must signal that as the removal of every interface that would have been listed by a GetManagedObjects() call made immediately before the removal.

Whether to include Properties, Peer, Introspectable and ObjectManager is a server implementation decision, but it would be reasonable to say the server must make that decision consistently.

sd-bus behaves as though every object has a (potentially empty) set of properties, which is a little unusual but definitely OK; it also behaves as though every object is a (possibly empty) ObjectManager, which I think is acceptable but weird. I don't want to require implementations of ObjectManager to match sd-bus exactly there.

I also think it would be OK to skip Introspectable and (particularly) Peer when describing objects in an ObjectManager, even if the object actually responds to those interfaces, because it would be acceptable for an ObjectManager implementer to think of Peer as part of the D-Bus implementation rather than part of the object (in particular the Peer interface even "works" for objects that don't actually exist).

> to make it easy to write a robust client implementation

With hindsight, this goal would have been better achieved by having an ObjectRemoved() signal, and *maybe* adding InterfacesRemoved() later if there was demand.

> robustness in remove_interfaces() when a *greater* number of
> interfaces are removed

I think (pseudocode)

    if (n_interfaces_removed >= n_interfaces_known)
      object was removed;

would be more robust than

    if (n_interfaces_removed == n_interfaces_known)
      object was removed;

but I don't think the client side needs to be hardened against pathological servers. In most interfaces, and ObjectManager is no exception IMO, it's the server's responsibility to be internally consistent.
Comment 4 David Härdeman 2017-04-25 22:31:08 UTC
(In reply to Simon McVittie from comment #3)
...
> I think (pseudocode)
> 
>     if (n_interfaces_removed >= n_interfaces_known)
>       object was removed;
> 
> would be more robust than
> 
>     if (n_interfaces_removed == n_interfaces_known)
>       object was removed;
>

I agree, that would improve the robustness. A more complete one would be to keep an array of added interfaces and remove entries from that array as matching interfaces are removed. Once the array is empty the object removed logic can trigger.

Anyhow, I've created some simple test cases which can be found here:
https://github.com/Alphix/gdbus-sdbus-test

Note that without a patched libsystemd0 you'll experience the sd-bus bug that was fixed with https://github.com/systemd/systemd/pull/5799

Also note that the additional systemd interfaces seem to confuse gdbus (g_dbus_interface_get_info() returns NULL for those interfaces, as noted above).
Comment 5 GNOME Infrastructure Team 2018-05-24 19:29:49 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to GNOME's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/1262.