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 687075 - g_spawn_sync diagnostic incorrectly complains about SIGCHLD
g_spawn_sync diagnostic incorrectly complains about SIGCHLD
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2012-10-28 23:48 UTC by Paul Eggert
Modified: 2012-10-29 14:25 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix ECHILD diagnostic and documentation. (1.49 KB, text/plain)
2012-10-28 23:48 UTC, Paul Eggert
  Details
patch in git format-patch format, with commit message (84.04 KB, patch)
2012-10-29 06:20 UTC, Paul Eggert
reviewed Details | Review

Description Paul Eggert 2012-10-28 23:48:55 UTC
Created attachment 227492 [details]
Fix ECHILD diagnostic and documentation.

I ran across a problem when trying to fix Emacs bug 8855
<http://bugs.gnu.org/8855>.  I've tracked the problem down to an
incompatibility between Emacs and glib.  Emacs uses waitpid(-1, ...)
to wait for all its children efficiently, but this races with the
waitpid(N, ...) in the glib thread that's invoking g_spawn_sync, and
when Emacs's waitpid(-1, ...) wins the race then g_spawn_sync outputs
a bogus message about SIGCHLD.  The problem has nothing to do with
SIGCHLD.

Also please see Red Hat bug 654027
<https://bugzilla.redhat.com/show_bug.cgi?id=654027>, in which glib's
SIGCHLD diagnostic confused everybody so much that the bug was closed
unfixed.

Also please see GNOME bug 687061, where it appears there's a fix in
the works for a related problem, a fix that doesn't look like it'll
solve this problem (though it may clash with the proposed patch).

I have attached a proposed patch to glib, to fix the diagnostic and
the documentation about this problem.
Comment 1 Colin Walters 2012-10-29 02:35:21 UTC
Comment on process: please write a commit message and attach the result of "git format-patch".  See:

https://live.gnome.org/GnomeLove/SubmittingPatches

(I can extract this comment into a commit message, but since you already made the patch against a git tree, please go through the extra bit to "git commit" and write a commit message yourself)
Comment 2 Colin Walters 2012-10-29 02:37:45 UTC
Comment on the code:  Emacs is broken.

Basically if you want to handle the case of "multiple middleware" (e.g. GTK+ and Qt, Java and GTK+, Emacs core and GTK+, etc.) linked into the same process, none of them can call waitpid(-1, ).  Emacs should be emulating what GTK+ and Qt both do, and only call waitpid() on pids that it explicitly spawned.

See also: http://www.macieira.org/blog/2012/07/forkfd-part-4-proposed-solutions/

for a proposed new kernel API, but not implemented.
Comment 3 Paul Eggert 2012-10-29 06:20:11 UTC
Created attachment 227501 [details] [review]
patch in git format-patch format, with commit message

OK, git format-patch attached, with commit message.  My Emacs automatically strips trailing white space so most of the patch consists of removing unnecessary white space -- if that's not OK for some reason I can resubmit it with the white space kept but that will be more work for me and I'd rather just give you what I have.
Comment 4 Colin Walters 2012-10-29 14:13:50 UTC
(In reply to comment #3)
> Created an attachment (id=227501) [details] [review]
> patch in git format-patch format, with commit message
> 
> OK, git format-patch attached, with commit message.  My Emacs automatically
> strips trailing white space so most of the patch consists of removing
> unnecessary white space -- if that's not OK for some reason

GLib is an old project.  We have lots of trailing whitespace that dates from before Linus invented git and made it complain about it.  We haven't yet done anything about this, but were we to do so, we'd do it all at once in a consistent manner.

> I can resubmit it
> with the white space kept but that will be more work for me 

Ok, I'll redo your patch.  

> and I'd rather just
> give you what I have.

Fair enough.  But for future patches, please do avoid stripping whitespace.  Though I do also hope to report we've fixed it at some point too.
Comment 5 Colin Walters 2012-10-29 14:24:05 UTC
Review of attachment 227501 [details] [review]:

Marking reviewed.
Comment 6 Colin Walters 2012-10-29 14:25:01 UTC
Pushed with just the code changes, not additional whitespace removal.