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 592962 - Fix gjs_crash_after_timeout() problems
Fix gjs_crash_after_timeout() problems
Status: RESOLVED FIXED
Product: gjs
Classification: Bindings
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gjs-maint
gjs-maint
Depends on:
Blocks:
 
 
Reported: 2009-08-25 01:20 UTC by Owen Taylor
Modified: 2009-08-25 17:21 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Fix gjs_crash_after_timeout() problems (3.43 KB, patch)
2009-08-25 01:20 UTC, Owen Taylor
none Details | Review

Description Owen Taylor 2009-08-25 01:20:54 UTC
I noticed that 'make distcheck' would run succesfully, then a few minutes later
I would get a whole bunch of horrible spew on the terminal where I ran
the distcheck.
Comment 1 Owen Taylor 2009-08-25 01:20:59 UTC
Created attachment 141603 [details] [review]
Fix gjs_crash_after_timeout() problems

Fix two major issues with gjs_crash_after_timeout():

 * It didn't notice the parent exiting normally so the forked off
   child would keep running until the timeout. This is fixed by
   using a pipe between the parent and child and watching for the
   pipe to get immediate notification of parent exit.

 * The forked off child process would return to the caller of
   gjs_crash_after_timeout() after the timeout completed and it
   killed the parent this would typically fail horribly. Call exit()
   instead.
Comment 2 Dan Winship 2009-08-25 16:17:41 UTC
owen asked me to look at this since no gjs people were around...

This seems very heavyweight for just a "make sure the regresssion tests eventually get killed" hack... It seems like it should work though.

You probably want _exit(), not exit(), in both places.

The use of spaces before parentheses in your code does not match the rest of the file.

You misspelled "FD_CLOEXEC" as "F_CLOEXEC" twice in non-code.
Comment 3 Owen Taylor 2009-08-25 17:21:43 UTC
Thanks for the review. It did occur to me after writing this "I probably could just have used sleep(1); kill(pid, 0) in a loop". But then I had already written it, and writing it twice seemed heavyweight for a "make sure regression tests eventually get killed hack" too.

Spaces, FD_CLOEXEC, _exit() fixed, pushed.