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 658999 - make fork()ing GMainContext explode sooner
make fork()ing GMainContext explode sooner
Status: RESOLVED OBSOLETE
Product: glib
Classification: Platform
Component: mainloop
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
: 647817 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2011-09-13 23:57 UTC by Allison Karlitskaya (desrt)
Modified: 2018-05-24 13:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
mainloop: detect fork() and abort (2.61 KB, patch)
2011-09-13 23:58 UTC, Allison Karlitskaya (desrt)
committed Details | Review
GApplication test: fix testcase to avoid fork() (8.46 KB, patch)
2011-09-14 15:08 UTC, Allison Karlitskaya (desrt)
committed Details | Review

Description Allison Karlitskaya (desrt) 2011-09-13 23:57:18 UTC
trying to have a GMainContext across a fork() is a recipe for intense amounts of pain.  first: the worker thread doesn't follow you.  second: you're now sharing an eventfd with your parent.

sometimes you can kind of get away with it and end up with weird bugs that are difficult to track down.  we should make a point of asserting quite rapidly if we detect this situation.
Comment 1 Allison Karlitskaya (desrt) 2011-09-13 23:58:10 UTC
Created attachment 196453 [details] [review]
mainloop: detect fork() and abort

Abort if the child process returns to the mainloop after a fork().
Comment 2 Allison Karlitskaya (desrt) 2011-09-14 02:10:14 UTC
This patch exposes problems with the following (already-broken) tests:

  - /gapplication/basic
  - /gdbus/non-socket
  - /gdbus/exit-on-close
Comment 3 Colin Walters 2011-09-14 14:35:02 UTC
Review of attachment 196453 [details] [review]:

This sounds like a really good idea.  One minor comment:

::: glib/gmain.c
@@ +178,3 @@
+ *
+ * The GLib mainloop is incompatible with fork().  Any program using the
+ * mainloop must either exec() or exit() from the child without

I'd prefix this with "On Unix,"
Comment 4 Allison Karlitskaya (desrt) 2011-09-14 15:08:55 UTC
Created attachment 196505 [details] [review]
GApplication test: fix testcase to avoid fork()

The GApplication test case tried to fork() while using GMainLoop,
causing problems.  Avoid doing that by splitting the child process into
a separate program and spawning it in the usual way.
Comment 5 Allison Karlitskaya (desrt) 2011-09-14 18:10:53 UTC
Comment on attachment 196453 [details] [review]
mainloop: detect fork() and abort

Committed to master with the suggested fix.
Comment 6 Allison Karlitskaya (desrt) 2011-09-14 18:11:09 UTC
Comment on attachment 196505 [details] [review]
GApplication test: fix testcase to avoid fork()

Committed to master.
Comment 7 Allison Karlitskaya (desrt) 2011-09-16 15:19:26 UTC
on master now, by David's request:

commit 05ef173466e32d8b3d212803e4a72239913a362d
Author: Ryan Lortie <desrt@desrt.ca>
Date:   Fri Sep 16 11:06:50 2011 -0400

    Disable two GDBus tests
    
    These tests try to use GMainContext across fork() which now fails a lot
    more violently than it used to.
    
    See https://bugzilla.gnome.org/show_bug.cgi?id=658999 for information.
Comment 8 David Zeuthen (not reading bugmail) 2011-10-05 15:00:41 UTC
I recently added some more tests (see bug 660886) that works fine on glib-2-30 but has to be disabled on master because we are now insistent on complaining about fork() with e.g.

 GLib:ERROR:gmain.c:2988:g_main_context_iterate: assertion failed: (!g_main_context_fork_detected)

The commit to disable those tests are here

 http://git.gnome.org/browse/glib/commit/?id=af55ff5a2b604399aa216bd731df97cba4474ad0

I don't think it makes sense to rewrite this test (I know what I'm doing here) - I think either that g_error() should be removed or there should be a way to remove it. I could of course use

 http://developer.gnome.org/glib/unstable/glib-Testing.html#g-test-log-set-fatal-handler

to allow it, but the problem with that is that the error will still be printed on stderr. Maybe that's not such a big deal. Thoughts?
Comment 9 David Zeuthen (not reading bugmail) 2011-10-05 15:11:27 UTC
(In reply to comment #8)
> I don't think it makes sense to rewrite this test (I know what I'm doing here)

Just to clarify, this "I know what I'm doing here" is only for the recently added tests - e.g. all I want (and all I do) is to be able to run the mainloop in the thread where I'm calling fork() from and check that the gdbus signal dispatcher complains on stderr.

The other tests, using fork(), should probably be rewritten (haven't looked into it in detail).
Comment 10 Allison Karlitskaya (desrt) 2011-10-05 18:31:56 UTC
You have two choices:

  - use a maincontext *only* before the fork

  - use a maincontext *only* after the fork

If you create any instances of GMainContext before the fork then you absolutely cannot use any instances (even new ones) after.

This may seem needlessly restrictive, but it's a consequence of the fact that the worker thread is shared between all of the contexts.

I think the check is currently applied in the way that I describe.
Comment 11 Matthias Clasen 2011-12-17 05:06:07 UTC
*** Bug 647817 has been marked as a duplicate of this bug. ***
Comment 12 Dan Winship 2012-03-13 14:10:46 UTC
(In reply to comment #8)
> I could of course use
> 
> http://developer.gnome.org/glib/unstable/glib-Testing.html#g-test-log-set-fatal-handler
> 
> to allow it, but the problem with that is that the error will still be printed
> on stderr. Maybe that's not such a big deal. Thoughts?

You can use g_log_set_default_handler() to override the default log handler with one that just sets a global flag confirming that you got the expected error.
Comment 13 Dan Winship 2013-06-04 23:36:02 UTC
so these tests could be rewritten using g_test_trap_subprocess() now
Comment 14 GNOME Infrastructure Team 2018-05-24 13:21:44 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/449.