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 780441 - Make the portal implementation of g_app_info_launch() synchronous
Make the portal implementation of g_app_info_launch() synchronous
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2017-03-23 11:26 UTC by Mario Sánchez Prada
Modified: 2017-03-23 14:22 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
appinfo: Don't leak the session bus in launch_default_with_portal_async (778 bytes, patch)
2017-03-23 11:37 UTC, Mario Sánchez Prada
committed Details | Review
appinfo: Launch the OpenURI portal using a synchronous D-Bus call (6.36 KB, patch)
2017-03-23 11:37 UTC, Mario Sánchez Prada
committed Details | Review

Description Mario Sánchez Prada 2017-03-23 11:26:48 UTC
As agreed during the GTK+ hackfest [1], "we decided to make the g_app_info_launch portal implementation a little more synch to help xdg-open", in order to "protect" the logic from not ever having the remote method running in case the xdg-desktop-portal process is not yet running and the caller quits quickly after the call.

This should not be a problem as the method returns immediately (regardless of the user making a selection), but making it synchronous would prevent situations where the OpenURI method would never be called because of D-Bus dropping the message after the caller dies, without explicitly waiting for a reply.

[1] https://wiki.gnome.org/Hackfests/GTK2017/Day3
Comment 1 Mario Sánchez Prada 2017-03-23 11:37:17 UTC
Created attachment 348560 [details] [review]
appinfo: Don't leak the session bus in launch_default_with_portal_async

The session bus object needs to be unreferenced before early returning.
Comment 2 Mario Sánchez Prada 2017-03-23 11:37:45 UTC
Created attachment 348561 [details] [review]
appinfo: Launch the OpenURI portal using a synchronous D-Bus call

Calling the D-Bus method for the OpenURI portal "protects" the logic from
not ever having the remote method running in case the xdg-desktop-portal
process is not yet running and the caller quits quickly after the call.

This should not be a problem as the method returns immediately (regardless
of the user making a selection), but making it synchronous would prevent
situations where the OpenURI method would never be called because of D-Bus
dropping the message after the caller dies, without explicitly waiting for
a reply.
Comment 3 Matthias Clasen 2017-03-23 11:56:09 UTC
Review of attachment 348560 [details] [review]:

oops, ok
Comment 4 Matthias Clasen 2017-03-23 11:58:52 UTC
Review of attachment 348561 [details] [review]:

ok
Comment 6 Simon McVittie 2017-03-23 14:22:01 UTC
(In reply to Mario Sánchez Prada from comment #0)
> making it synchronous would prevent
> situations where the OpenURI method would never be called because of D-Bus
> dropping the message after the caller dies, without explicitly waiting for a
> reply

I think the issue here might be the caller dying before it has even finished the authentication handshake. I believe dbus-daemon is meant to finish processing queued *messages* before removing the caller's connection (although of course bugs are possible), but it can't do that if the caller hasn't even finished authenticating yet, and in GDBus-world, the handshake might well be async too.

(If I am right about that, then libdbus has an API bug that makes it avoid that issue: opening the connection, which includes the auth handshake, is always similar to g_bus_get_sync(), and never g_bus_get().)