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 790963 - Thread safety issue in gtk_application_impl_dbus_startup
Thread safety issue in gtk_application_impl_dbus_startup
Status: RESOLVED OBSOLETE
Product: gtk+
Classification: Platform
Component: Class: GtkApplication
3.22.x
Other Linux
: Normal normal
: ---
Assigned To: gtk-bugs
gtk-bugs
Depends on:
Blocks:
 
 
Reported: 2017-11-28 22:27 UTC by Michael Catanzaro
Modified: 2018-05-02 19:29 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Avoid calling unsetenv too late (2.63 KB, patch)
2017-11-29 04:28 UTC, Matthias Clasen
none Details | Review

Description Michael Catanzaro 2017-11-28 22:27:18 UTC
gtk_application_impl_dbus_startup() calls g_unsetenv(). Here be dragons. As per the documentation of g_unsetenv(), and experience (e.g. bug #754951), this will explode randomly. g_unsetenv() may only be used before the first secondary thread is created, but GApplication starts working with GDBusConnection at GApplication register time, before GApplication startup, and certainly well before GtkApplication's startup.

This will be difficult to fix. One option is to document that the application must not create any threads before calling g_application_run(), and hope that applications get that right (they will not). Then store the desktop ID as global state way earlier than startup. I think GtkApplication class init would be a "good" place, since it's class data and not instance data. Alternatively, this could be done using a library constructor, which would make it very heard for applications to create a thread too soon, except then we'd have to leave Windows broken.

Another option would be to give up on unsetting the environment variable, and modify g_spawn() to always pass a custom envp to the child process with it removed. That would work so long as a GApplication always uses g_spawn() when spawning another GApplication. Eh.
Comment 1 Matthias Clasen 2017-11-29 04:28:59 UTC
Created attachment 364599 [details] [review]
Avoid calling unsetenv too late

Stash the DESKTOP_AUTOSTART_ID env var in a constructor,
before any threads have been created.
Comment 2 Matthias Clasen 2017-11-29 04:29:56 UTC
DESKTOP_STARTUP_ID in gdk/x11/gdkdisplay-x11.c may need the same treatment.
Comment 3 Matthias Clasen 2017-11-29 04:30:50 UTC
Leaving windows broken is not too big a concern here, since we are talking about x11/dbus-specific code
Comment 4 Michael Catanzaro 2017-11-29 04:38:54 UTC
DESKTOP_STARTUP_ID in gdk_wayland_display_make_default() as well. Again, not a concern for Windows, so yay for library constructors.
Comment 5 GNOME Infrastructure Team 2018-05-02 19:29:18 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/gtk/issues/979.