Created attachment 184708 [details] [review]
dbus: Add connect_sync() method to bus objects

Created attachment 184731 [details] [review]
dbus: Allow synchronous connections

In gnome-shell, we added a call to DBus.session.flush() to attempt to
ensure we were connected immediately.  But this has multiple problems,
and actually it never quite worked because gjs-dbus had a hidden
"process in idle".

Add a connect_sync() function, and make it really synchronous.

Review of attachment 184731 [details] [review]:

Hmm, I think there is a problem here - presumably the reason it's done in an idle now is to avoid reentrancy, but by making check_bus() force the connections, you've created a more sever reentrancy - if you try to acquire a name, then the next outgoing call can cause name ownership monitors and watchers to fire, which is pretty unexpected.

I'd suggest that there probably needs to be a separate gjs-dbus method which basically does:

if (<blah>connect_idle) {
   unset <blah>connect-idle;
    connect_idle(...);
}

Rather than wedging it into gjs_dbus_try_connecting_now() - I'd also suggest that the gjs exported method name should be something other than connect_sync() since it makes sense at any time you need to acquire a name now - connect_idle (perhaps oddly) can be set up an run for an existing connection when we acquire a new name.

Created attachment 184888 [details] [review]
dbus: Do connection processing in a high priority timeout

Otherwise, we risk race conditions if dbus-glib processes
messages before we've had a chance to set up our exports.

<owen> OK, I think this patch is problmatical
 it means that any outgoing call can cause reentrancy to your name ownership monitors
 I'll comment on the bug
<walters> owen: so i agree you're right in theory, but in practice, with this patch, we connect to the session bus before we set up monitors
...
<walters> owen: no the problem is simply that exports are set up in an idle, which is lower priority than reading from the dbus socket
 so that's why we see this nearly 100% of the time
...
<owen> Hmm, OK, so I think it's basically broken if I call dbus.exportObject and it doesn't take effect immediatley for subsequent incoming calls once I return to the main loop, so I can't see that we should require any changes to gnome-shell - this seems like something we should fix in gjs

Review of attachment 184888 [details] [review]:

Good except for comment

::: gjs-dbus/dbus.c
@@ +353,3 @@
+             * potentially reading any messages from the socket.  If we
+             * didn't, this could lead to race conditions.  See
+             * https://bugzilla.gnome.org/show_bug.cgi?id=646246

This comment is confusing, since this code is completely generic and the export handling is added in a different layer. I'd say:

 "so that deferred work, such as the
  gjs_dbus_add_connect_funcs() callbacks that are used
  to set up exports by the DBus JS module runs *before*..."

Comment on attachment 184888 [details] [review]
dbus: Do connection processing in a high priority timeout

I'm not very familiar with gjs-dbus, but this seems right.

You don't actually need to switch from an idle to a 0-timeout btw. An "idle" with G_PRIORITY_HIGH is just as high-priority as a timeout with G_PRIORITY_HIGH.

*** Bug 646177 has been marked as a duplicate of this bug. ***

Pushed with r-t approval and released in 0.7.14