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 784392 - GDBus can't own names with DO_NOT_QUEUE flag
GDBus can't own names with DO_NOT_QUEUE flag
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gdbus
unspecified
Other Linux
: Normal enhancement
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-06-30 16:56 UTC by Simon McVittie
Modified: 2018-05-24 19:39 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
GBusNameOwnerFlags: Note equivalence with D-Bus Specification (1012 bytes, patch)
2017-06-30 17:23 UTC, Simon McVittie
committed Details | Review
[untested] GBusNameOwnerFlags: Add DO_NOT_QUEUE flag (1.35 KB, patch)
2017-06-30 17:24 UTC, Simon McVittie
committed Details | Review
[untested] gdbus: Don't AddMatch() for NameAcquired, NameLost (1.85 KB, patch)
2017-06-30 17:24 UTC, Simon McVittie
reviewed Details | Review
[untested] gdbus: Check signature of NameAcquired, NameLost (1.23 KB, patch)
2017-06-30 17:26 UTC, Simon McVittie
committed Details | Review

Description Simon McVittie 2017-06-30 16:56:43 UTC
g_bus_own_name() and related functions cannot specify the DO_NOT_QUEUE flag, which means that a GDBus service cannot atomically carry out "take the name if there is no other owner" without resorting to direct method calls. The closest it can get is this pseudocode:

        result = RequestName(name)
    
        if result == IN_QUEUE:
            ReleaseName(name)
            result = EXISTS
    
        return result

which could result in returning EXISTS but briefly owning the name (if we are reported to be in the queue, then the primary owner drops it and we get it moments before we call ReleaseName).
Comment 1 Simon McVittie 2017-06-30 17:23:26 UTC
Created attachment 354741 [details] [review]
GBusNameOwnerFlags: Note equivalence with D-Bus  Specification

The implementation passes flags through directly to the RequestName()
call, so if any new values break that equivalence, the implementation
will have to be changed.
Comment 2 Simon McVittie 2017-06-30 17:24:02 UTC
Created attachment 354742 [details] [review]
[untested] GBusNameOwnerFlags: Add DO_NOT_QUEUE flag

PulseAudio and LibreOffice are among the services that use this flag.
Refusing to queue for a name lets you do this transaction,
but atomically, avoiding the transient state where you briefly join
the queue and then are given the name when its primary owner drops it:

    result = RequestName(name)
 
    if result == IN_QUEUE:
        ReleaseName(name)
        result = EXISTS

    return result
Comment 3 Simon McVittie 2017-06-30 17:24:47 UTC
Created attachment 354743 [details] [review]
[untested] gdbus: Don't AddMatch() for NameAcquired, NameLost

The dbus-daemon delivers these as unicast signals directed to the
unique name of the would-be name owner, whether it has added a match 
rule or not.

---

Something I noticed while looking at the patches above.
Comment 4 Simon McVittie 2017-06-30 17:26:08 UTC
Created attachment 354744 [details] [review]
[untested] gdbus: Check signature of NameAcquired, NameLost

Calling g_variant_get (parameters, "(&s)") when parameters has a
signature other than (s) is considered to be a programming error.
In practice the message bus (dbus-daemon or a reimplementation) should
always send the expected type, but be defensive.

---

Another thing I noticed while looking at this.

Sorry for the untested patches, but I'm shaving too many unrelated yaks right now to test these, and it seemed better to attach something than to attach nothing.
Comment 5 Philip Withnall 2017-07-03 10:36:14 UTC
Review of attachment 354741 [details] [review]:

++
Comment 6 Philip Withnall 2017-07-03 10:38:16 UTC
Review of attachment 354742 [details] [review]:

Needs to be added to the documentation comment just above the enum, including a ‘(Since: 2.54)’ at the end of the new docs line.
Comment 7 Philip Withnall 2017-07-03 10:41:18 UTC
Review of attachment 354743 [details] [review]:

Are all bus implementations guaranteed to send these signals as unicast? The spec is not abundantly clear about that, unless “This signal is sent to a specific application when it loses ownership of a name” definitely means ‘unicast and only unicast’. I’m happy with this patch if you’re happy that any bus implementation which doesn’t send those signals as unicast is wrong.
Comment 8 Philip Withnall 2017-07-03 10:46:48 UTC
Review of attachment 354744 [details] [review]:

Good catch.

::: gio/gdbusnameowning.c
@@ +274,3 @@
     goto out;
 
+  if (g_strcmp0 (g_variant_get_type_string (parameters), "(s)") != 0)

Would be better to do this as:
if (!g_variant_is_of_type (parameters, G_VARIANT_TYPE ("(s)")))
Comment 9 Philip Withnall 2017-08-07 16:33:31 UTC
I’ve updated and pushed all of these patches except the AddMatch() one, which I’d still like the question answered for.
Comment 10 GNOME Infrastructure Team 2018-05-24 19:39:47 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/1277.