Thanks for the bug report. I can reproduce it on F21.

Maybe there is relation with Bug 652262, because adding sleep in g_desktop_app_info_launch_uris_internal helps according the downstream bugzilla report, so we exits too early probably...

This happens when it launches the app via dbus. The problem is that it exits before the underlying dbus call properly finishes and so the message doesn't get through properly (I saw this a while ago and forgot to raise a bug).

I think it should be reassigned to glib since the underlying call to open the app is supposed to be a sync call but it hasn't finished by the time we exit.

I'm convinced this is bug in gio also, because g_app_info_launch_default_for_uri should return only after the application is executed - I mean in this case after dbus call is sent. But it seems we exit earlier then the call is sent despite g_dbus_connection_flush is called...

Only sync version of g_dbus_connection_call seems to fix this issue, but we don't want to wait for response probably according the comment:
  /* This is non-blocking API.  Similar to launching via fork()/exec()
   * we don't wait around to see if the program crashed during startup.
   * This is what startup-notification's job is...
   */

(gapplication tool also uses _sync version for launching)

Do you have any idea how to fix it?

Created attachment 300674 [details] [review]
gdesktopappinfo: use sync call to avoid dropping message

There is a patch to use _sync call with small timeout 250ms (just to be sure message is sent) as a result of discussion on #gtk+.

We are calling

g_dbus_connection_flush (session_bus, NULL, NULL, NULL);

after the async call, to ensure the message makes it out on our end before exit(). Is that not working ?

In the case of activation, it's insufficient to flush the bus -- D-Bus has an annoying habbit of discarding messages for pending activations if the sending application has exited.

In light of how difficult it would be to solve this problem, I almost consider that to be a bug in D-Bus...

Particularly when you consider that the message may even have the no-reply-needed flag.  In that case, why does it matter _at all_ if the sender has quit?

In fact, this "no-reply-needed" case is the case we are dealing with here -- we do g_dbus_connection_call with a NULL async callback, which means that the flag is getting set on the outgoing message.

That means that we have to wait for a reply that will never come.  This is a D-Bus bug...

(In reply to Matthias Clasen from comment #5)
> We are calling
> 
> g_dbus_connection_flush (session_bus, NULL, NULL, NULL);
> 
> after the async call, to ensure the message makes it out on our end before
> exit(). Is that not working ?

That's an async call as well though, so presumably we're exiting before it actually gets a chance to flush.

there's a related open dbus-daemon bug here, I think:

https://bugs.freedesktop.org/show_bug.cgi?id=896

comment 41 shows the same problem happening with dbus-send, and of course NetworkManager isn't normally activated, so that shows this problem can happen even if the service is already running.

Maybe there is "better" dbus-daemon bug report, see:
https://bugs.freedesktop.org/show_bug.cgi?id=52372

So it is probably dbus bug, but it is quite old and still it isn't fixed. But we need to find some solution quickly, because gvfs-open is now pretty unusable... 

gvfs-open works for me every second try, but it can be much worse according the following downstream bug report (10-100 tries is needed to start the application):
https://bugzilla.redhat.com/show_bug.cgi?id=1220349

Wouldn't be solution to apply my patch as a workaround?

I think a workaround would have to be a patch to gvfs-open. We don't want to make the gio api effectively synchronous.

(In reply to Dan Winship from comment #8)
> (In reply to Matthias Clasen from comment #5)
> > We are calling
> > 
> > g_dbus_connection_flush (session_bus, NULL, NULL, NULL);
> > 
> > after the async call, to ensure the message makes it out on our end before
> > exit(). Is that not working ?
> 
> That's an async call as well though, so presumably we're exiting before it
> actually gets a chance to flush.

Yeah...if you annotate you'll see that came from:

https://git.gnome.org/browse/glib/commit/?id=d6954c785d635be875b896ac9f4812e400b39455

So I'd say to combine what Dan and Matthias said...try calling g_dbus_connection_flush_sync() (and note the _sync() ) at the end of gvfs-open.c?

This was first thing I tried, however calling _flush_sync() doesn't fix it for me :-(

g_dbus_connection_flush documentation claims it should work, but it doesn't work:
"This is useful in programs that wants to emit a D-Bus signal and then exit immediately."

it's not going to work because the problem isn't just the message not going out in time, it's the message not getting processed by the dbus-daemon in time.

If you want to work around this in gvfs-open you need to block gvfs until dbus sends a message to you.  It won't send a message for the call made, though, since the call made is no-reply.

The easiest workaround would be to send a synchronous org.freedesktop.DBus.Peer.Ping message before exiting

It seems ridiculous to me to do workaround for this in gvfs-open, because we don't have any information whether the application is launched over d-bus, or spawn and also we don't know bus_name and object_patch for the application to send e.g. org.freedesktop.DBus.Peer.Ping as was suggested by halfline...

I think it should be sufficient to ping the message bus itself (or do any other call to the message bus).

As I understand it, the race that needs to be closed is the bus daemon dispatching the request before the client closes.  I believe that the bus daemon dispatches requests from a client in order, so if a client makes two requests the first will get dispatched before the second.

So you shouldn't need to ping the activating service, just the bus daemon (but make sure you wait for a reply!).

I realize it isn't pretty, and it's debatable whether the ugliness should go in the glib or gvfs, but unless you want to fix dbus-daemon it's got to go somewhere... and gvfs-open is one of the few programs out there that will hit this bug in practice, because most programs are long lived.

So Ondrej looked into this a bit and discovered the bus daemon doesn't implement the Peer interface for /org/freedesktop/DBus .  But if I understand the race correctly, any method should work, so he's going to try GetId instead.

fail:

<oholy> but it seems it isn't enought:
<oholy> method call time=1432131083.551780 sender=:1.272 -> destination=org.gnome.Nautilus serial=9 path=/org/gnome/Nautilus; interface=org.freedesktop.Application; member=Open
<oholy>    array [
<oholy>       string "file:///"
<oholy>    ]
<oholy>    array [
<oholy>    ]
<oholy> method call time=1432131083.552294 sender=:1.272 -> destination=org.freedesktop.DBus serial=10 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=GetId
<oholy> method return time=1432131083.552340 sender=org.freedesktop.DBus -> destination=:1.272 serial=8 reply_serial=10
<oholy>    string "8be56d0183ba3a5ac06b42a8555c26c6"
<oholy> signal time=1432131083.560777 sender=org.freedesktop.DBus -> destination=:1.272 serial=33 path=/org/freedesktop/DBus; interface=org.freedesktop.DBus; member=NameLost
<oholy>    string ":1.272"
<oholy> id is returned, but nautilus isn't started..
<halfline> well nautilus doesn't have to be started, it just has to be dispatched
<halfline> if i understand the problem right
<halfline> or are you saying you've tried the patch and it isn't working ?
<oholy> sorry, bad formulation
<oholy> nautilus doesn't start with this patch
<halfline> okay
<halfline> hmm

So i guess the three obvious choices left are:

1) add the ping to glib
2) add a delayed exit to gvfs
3) fix dbus-daemon

1) could better be stated as "block for the reply to finish" but it's not really an option given the api doesn't have an async ready callback

Created attachment 303713 [details] [review]
gvfs-open: add hack to close up dbus-daemon race

If gvfs-open exits before the program it starts fully activates, then
the dbus-daemon may avoid doing the activating method call.

This commit works around the problem by pinging the activated application,
and waiting for a reply.

the attachment 303713 [details] [review] is pretty yucky but it closes up the race without changes to glib or dbus-daemon.

Review of attachment 303713 [details] [review]:

::: programs/gvfs-open.c
@@ +103,3 @@
+  g_clear_object (&app_info);
+  g_clear_pointer (&basename, g_free);
+  g_clear_pointer (&object_path, g_free);

there's a harmless unnecessary g_clear_pointer here. we should either drop it or add another harmless, unneccessary g_clear_pointer for bus_name.

Review of attachment 303713 [details] [review]:

It seems the patch fixes the problem. It also leaks output from g_file_get_uri (I know there is same leak in the gnome-3-14 code, which is already fixed in newer versions). However the patch is the example, why I think it is ridiculous to fix this in gvfs (gvfs-open is almost double sized now and contains lot of copy&pasted code from glib)... :-(

Created attachment 303751 [details] [review]
gappinfo: add hack to close up dbus-daemon race

Might be better to fix it in g_app_info_launch_default_for_uri, which is already using synchronous I/O, to safe some copy&pasted code, mightn't it?

Review of attachment 303751 [details] [review]:

::: gio/gappinfo.c
@@ +21,3 @@
 #include "config.h"
 
+#include <string.h>

is redundant

Review of attachment 303751 [details] [review]:

well, Matthias / Ryan can chime in on whether or not this is acceptable, but it makes sense to me as a "right now" answer. Some might argue that extending the GAppInfo interface would be better, but I think doing that would only make sense if we did it in a way that fixed the three warts in the existing async api (no callback, no cancellable, GList of strings instead of array of strings).

I also disagree that putting a workaround in gvfs-open is ridiculous. Yucky, but good enough near-term, since it's one of the very few programs that would ever be affected.

::: gio/gappinfo.c
@@ +729,3 @@
+      const char *app_id;
+
+      app_id = g_desktop_app_info_get_app_id (app_info);

I think if you're going to be introducing private api between gappinfo and gdesktopappinfo it should be as contained as possible.  e.g., g_desktop_app_info_ping_peer_sync rather than exporting the app id and the api for converting the app id into an object path

::: gio/gappinfoprivate.h
@@ +26,3 @@
+gchar *object_path_from_appid (const gchar *app_id);
+
+const char *g_desktop_app_info_get_app_id (GAppInfo *appinfo);

this should probably go in its own header i think.

Review of attachment 303713 [details] [review]:

So this is okayish, but a couple of comments:

 1) in addition to checking if the desktop file looks like a bus name you should check the DBusActivatable key to make sure it is true.

 2) at this point you almost may as well send the Open or Activate method for yourself...

 3) it could make sense to add some sort of g_app_info_sync() sort of call to deal with this, but...
    ...that would be silly, if eventually this will be fixed in D-Bus

so, really, just check DBusActivatable and I guess this is the best we can do.

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

I wrote another set of similar patches on bug 748063 that try to fix the problem in gdesktopappinfo.c itself.

Comment on attachment 300674 [details] [review]
gdesktopappinfo: use sync call to avoid dropping message

Changing status of Attachment 300674 [details] as per Comment 11.

Review of attachment 303713 [details] [review]:

::: programs/gvfs-open.c
@@ +61,3 @@
+
+  if (app_info != NULL && !G_IS_DESKTOP_APP_INFO (app_info))
+    return FALSE;

This check can be omitted...

@@ +197,3 @@
+	  char *object_path = NULL;
+
+	  if (get_bus_name_and_path_from_uri (g_file_get_uri (file), &bus_name, &object_path))

This works only if location is relative or absolute path. File is NULL if location is proper uri...

Created attachment 304844 [details] [review]
gvfs-open: add hack to close up dbus-daemon race (gnome-3-14)

Let's fix it in gvfs-open, there is amended patch from halfline with removed unnecessary g_clear_pointer, fixed uri leak, removed redundant app_info check, fixed for uri locations.

Created attachment 304845 [details] [review]
gvfs-open: add hack to close up dbus-daemon race (master)

Review of attachment 304844 [details] [review]:

So I'm clearly okay with this patch, and it looks like you addressed all the things Ryan said. I will say, though, that I sort of like your approach in attachment 303751 [details] [review] better (well with the changes I mentioned).  It doesn't make an async api suddenly synchronous so it addresses mclasen's concern in comment 11.  It isn't 'pretty', but workarounds seldom are.  Anyway, either way seems okayish to me, and if you just want to do it in gvfs, that's cool.  Still, might be worth pinging mclasen/ryan one more time to get their takes on comment 25.

Created attachment 304862 [details] [review]
gappinfo: add hack to close up dbus-daemon race

Ok, there is also modified fix for gappinfo for comparation.
gdesktopappinfoprivate.h is added and g_desktop_app_info_ping_peer_sync is introduced...

Review of attachment 304862 [details] [review]:

This effectively causes the call to block, which is really in no way better than just doing a blocking D-Bus call in the first place...

I really think any fix needs to be done inside of the commandline tool itself.

that call already blocks

<halfline> desrt: g_app_info_launch_default_for_uri is documented as a sync call
<desrt> oh.  curious.
<desrt> still not a fan :(
<desrt> opening a file on the disk and reading a couple of bytes is not the same as blocking on an app to startup
<halfline> not the same, but both can result in the program seemingly freezing up
<halfline> anyway, the workaround is ugly no matter where it goes, and i don't really care where it goes

Ok, so, I'm resigned to push the gvfs-open patches...

Comment on attachment 304844 [details] [review]
gvfs-open: add hack to close up dbus-daemon race (gnome-3-14)

gnome-3-14:
commit 7bee91695bbe5d9228392e6da496186f02df5c39

Comment on attachment 304845 [details] [review]
gvfs-open: add hack to close up dbus-daemon race (master)

master:
commit 9de6328f97a75b3ac4431810b33605f75778f1f2

gnome-3-16:
commit 9297474c33192fe210f0cbc9ffc349f19604516a

Thanks to all participants!

-> glib

Reopening and moving to GLib, because I'm now hitting the same exact issue when my application (Sushi) calls gtk_show_uri() and immediately quits afterwards. If the application is DBus-activated, its window won't be shown until gtk_show_uri() is called a second time.

My patches from bug 748063 (or other similar patches that fixed this at the GLib level) would fix this problem.

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

It seems to me gtk-launch utility has also same problem...

filed bug 763103 for gtk-launch

As per https://bbs.archlinux.org/viewtopic.php?id=190252 this bug existed since GNOME 3.14 and was finally fixed about 1 year later in GNOME 3.17.3. However, since GNOME 3.24, gvfs-open has been depreciated and we are supposed to use gio open. But gio open has this same old bug :( Calling gio open only works when Nautilus is already running so presumably the same fix done to gvfs-open (https://github.com/GNOME/gvfs/commit/9de6328f97a75b3ac4431810b33605f75778f1f2) needs to be done to gio.

I just discovered bug 780296 is more appropriate to my comment above. The responsible developer should close this bug here given gvfs-open has been depreciated away now since GNOME 3.24.

*** This bug has been marked as a duplicate of bug 780296 ***