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 793831 - g_app_info_launch_default_for_uri_async is not really async
g_app_info_launch_default_for_uri_async is not really async
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: gio
2.55.x
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 780296
 
 
Reported: 2018-02-26 08:38 UTC by Carlos Soriano
Modified: 2018-05-24 20:15 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gappinfo: Make async API really async (3.11 KB, patch)
2018-02-27 10:39 UTC, Ondrej Holy
needs-work Details | Review

Description Carlos Soriano 2018-02-26 08:38:26 UTC
The first call is launch_default_for_uri which is all sync. Specifically, g_file_query_default_handler blocks in case the file is in a ssh server that is down.

Most probably the best approach would be to make the whole g_app_info_launch_default_for_uri_async a GTask wrapper of g_app_info_launch_default_for_uri.

See https://gitlab.gnome.org/GNOME/nautilus/issues/266 for the original report against Nautilus.

Here's the backtrace or where is blocking:

  • #0 poll
  • #1 g_main_context_iterate.isra
  • #2 g_main_loop_run
  • #3 g_dbus_connection_send_message_with_reply_sync
  • #4 g_dbus_connection_call_sync_internal
  • #5 g_dbus_proxy_call_sync_internal
  • #6 g_dbus_proxy_call_sync
  • #7 gvfs_dbus_mount_call_query_info_sync
  • #8 g_daemon_file_query_info
  • #9 g_file_query_default_handler
  • #10 launch_default_for_uri
  • #11 g_app_info_launch_default_for_uri_async
  • #12 on_window_handle_export
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-program-choosing.c line 594
  • #13 window_export_handle
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-program-choosing.c line 499
  • #14 nautilus_launch_default_for_uri_async
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-program-choosing.c line 652
  • #15 activate_files
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-mime-actions.c line 1875
  • #16 activate_callback
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-mime-actions.c line 2029
  • #17 file_list_file_ready_callback
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-file.c line 9144
  • #18 call_ready_callbacks_at_idle
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-directory-async.c line 1985
  • #19 g_idle_dispatch
  • #20 g_main_context_dispatch
  • #21 g_main_context_iterate.isra
  • #22 g_main_context_iteration
  • #23 g_application_run
  • #24 main
    at ../../../../jhbuild/checkout/nautilus/src/nautilus-main.c line 84

Comment 1 Ondrej Holy 2018-02-26 09:00:51 UTC
Just a note, that this slightly corresponds to the following bug and can end up with a common fix...

https://bugzilla.gnome.org/show_bug.cgi?id=780296
Comment 2 Ondrej Holy 2018-02-27 10:39:17 UTC
Created attachment 368999 [details] [review]
gappinfo: Make async API really async

g_app_info_launch_default_for_uri_async calls various GFile and
GDesktopAppInfo synchronous methods. This causes troubles especially
in case of unaccessible filesystem. Given the fact that the whole
GDesktopAppInfo API is synchronous, let's simply use GTask thread
to call g_app_info_launch_default_for_uri.
Comment 3 Carlos Soriano 2018-02-27 14:25:14 UTC
Review of attachment 368999 [details] [review]:

LGTM fwiw
Comment 4 Philip Withnall 2018-02-27 14:43:23 UTC
Review of attachment 368999 [details] [review]:

::: gio/gappinfo.c
@@ +778,3 @@
+  context = g_object_get_data (G_OBJECT (task), "context");
+
+  if (g_app_info_launch_default_for_uri (uri, context, &error))

The GAppLaunchContext isn’t thread-safe, so you can’t technically do this.

I suspect the only way to fix this is to add launch_uris_async and launch_uris_finish vfuncs to GAppInfoIface and plumb them all the way through to here. Then the implementation of GAppInfo can decide whether to use a thread (or whether to poll); and if it uses a thread, it can use appropriate locking.

@@ +815,3 @@
+  g_object_set_data_full (G_OBJECT (task), "uri", g_strdup (uri), g_free);
+  if (context != NULL)
+    g_object_set_data_full (G_OBJECT (task), "context", g_object_ref (context), g_object_unref);

Use g_task_set_task_data() to avoid creating a pointless qdata hash table in the GTask:

typedef struct
{
  gchar *uri;
  GAppLaunchContext *context;
} LaunchDefaultForUriData;

static void
launch_default_for_uri_data_free (LaunchDefaultForUriData *data)
{
  g_free (data->uri);
  g_object_unref (data->context);
  g_free (data);
}

{
  LaunchDefaultForUriData *data = NULL;

  …

  data = g_new0 (LaunchDefaultForUriData, 1);
  data->uri = g_strdup (uri);
  data->context = g_object_ref (context);
  g_task_set_task_data (task, g_steal_pointer (&data), (GDestroyNotify) launch_default_for_uri_data_free);

  …
}
Comment 5 GNOME Infrastructure Team 2018-05-24 20:15:54 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/1347.