Closed Bug 984196 Opened 7 years ago Closed 7 years ago
glxtest should use _exit instead of exit
(This bug might not have been observable for a bit while bug 97906 existed, so might not have shown up in testing of bug 981126.) When I start up a debug build on Linux (in a VNC connection, possibly this is specific to running over VNC, but I'm not sure), the browser hangs. The main Firefox process is doing this: #0 0x00007f769504188d in __libc_waitpid (pid=<optimized out>, stat_loc=<optimized out>, options=<optimized out>) at ../sysdeps/unix/sysv/linux/waitpid.c:41 #1 0x00007f768fee2fdf in mozilla::widget::GfxInfo::GetData (this=0x1ec2f20) at /home/dbaron/builds/ssd/mozilla-central/mozilla/widget/xpwidgets/GfxInfoX11.cpp:87 #2 0x00007f768fede759 in nsBaseWidget::ComputeShouldAccelerate (this=0x1be8330, aDefault=false) at /home/dbaron/builds/ssd/mozilla-central/mozilla/widget/xpwidgets/nsBaseWidget.cpp:884 #3 0x00007f768fede23c in nsBaseWidget::GetLayerManager (this=<optimized out>, aShadowManager=0x0, aBackendHint=<optimized out>, aPersistence=<optimized out>, aAllowRetaining=0x7fff9b9dff0f) at /home/dbaron/builds/ssd/mozilla-central/mozilla/widget/xpwidgets/nsBaseWidget.cpp:1025 ... And the glxtest process is doing this: #0 0x00007f769432a08d in nanosleep () at ../sysdeps/unix/syscall-template.S:82 #1 0x00007f7694329f2c in __sleep (seconds=0) at ../sysdeps/unix/sysv/linux/sleep.c:138 #2 0x00007f7690b2e004 in ah_crap_handler (signum=6) at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsSigHandlers.cpp:90 #3 0x00007f7690b2f060 in exit (status=<optimized out>) at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsSigHandlers.cpp:110 #4 0x00007f7690b2f0d5 in fire_glxtest_process () at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/glxtest.cpp:270 #5 0x00007f7690b262f9 in XREMain::XRE_mainInit (this=0x7fff9b9e0890, aExitFlag= 0x7fff9b9e085f) at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsAppRunner.cpp:2859 #6 0x00007f7690b2a80b in XREMain::XRE_main (this=0x7fff9b9e0890, argc=4, argv=<optimized out>, aAppData=0x7fff9b9e0a40) at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsAppRunner.cpp:4059 #7 0x00007f7690b2ab04 in XRE_main (argc=4, argv=0x7fff9b9e1da8, aAppData= 0x7fff9b9e0a40, aFlags=<optimized out>) at /home/dbaron/builds/ssd/mozilla-central/mozilla/toolkit/xre/nsAppRunner.cpp:4291 #8 0x00000000004035bb in do_main (argc=4, argv=0x7fff9b9e1da8, xreDirectory= 0x773010) at /home/dbaron/builds/ssd/mozilla-central/mozilla/browser/app/nsBrowserApp.cpp:282 #9 0x0000000000403a7c in main (argc=4, argv=0x7fff9b9e1da8) at /home/dbaron/builds/ssd/mozilla-central/mozilla/browser/app/nsBrowserApp.cpp:643 I think this should be using _exit() rather than exit() like the other uses in glxtest, so that this doesn't happen.
(In reply to David Baron [:dbaron] (needinfo? me) (UTC+8) from comment #0) > (This bug might not have been observable for a bit while bug 97906 existed, ... er, while bug 979069 existed
Comment on attachment 8391989 [details] [diff] [review] Again use _exit() rather than exit() in glxtest to avoid hang in debug builds Review of attachment 8391989 [details] [diff] [review]: ----------------------------------------------------------------- *reads up on the difference between _exit vs. exit* Ok cool. Thanks for the patch!
Attachment #8391989 - Flags: review?(bgirard) → review+
Sorry, this was actually the result of a local patch I had to trap exit with our crash hooks; I'll merge this into that patch (which I'd forgotten about, but should probably land at some point).
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → INVALID
I couldn't find a credible source but common sources like stackoverflow and other Q&A forums seem to indicate that calling exit in the child can lead to temp files being deleted, buffers being flushed twice amongst other things. That we should use _exit. Are you sure we don't still want this even if it didn't cause your problem? Viktor can do the follow up if we agree that we still want this.
I'm not sure; I ended up merging it into the patch at https://hg.mozilla.org/users/dbaron_mozilla.com/patches/raw-file/5f375e3858e7/trap-exit which I had locally to debug an unexpected exit() call at one point, but never landed.
I think this might still be needed. Viktor can you do the follow up.
Status: RESOLVED → REOPENED
Resolution: INVALID → ---
I guess this is the relevant stack overflow question: http://stackoverflow.com/questions/2329640/how-to-exit-a-child-process-exit-vs-exit It seems to me that we still want to use _exit() because the exit is in a fork. The communication it does over a pipe with the parent process is done without buffers, so it should be safe to use _exit for all of the exit calls in the child. I can make a patch for this. I would push this to try first just in case though... Which tests make sense to run on try to make sure this didn't break anything?
(In reply to Viktor Stanchev [:vikstrous] from comment #8) > it should be safe to use _exit for all of the exit calls > in the child. just the glxtest exit(s) is fine. Unless we can find other problematic areas. > I can make a patch for this. I would push this to try first > just in case though... Which tests make sense to run on try to make sure > this didn't break anything? I don't know. Maybe all the linux desktop tests.
Oops didn't select debug builds too: https://tbpl.mozilla.org/?tree=Try&rev=7c2c0ebcb88c
Attachment #8393099 - Flags: review?(bgirard) → review+
Summary: Linux debug builds hang on startup because glxtest triggers ah_crap_handler and hangs for 300 seconds → glxtest should use _exit instead of exit
This is on my outbound queue. Waiting for inbound to open.
I had to back this out in http://hg.mozilla.org/integration/mozilla-inbound/rev/4d5074dfad5d for breaking Valgrind builds: https://tbpl.mozilla.org/php/getParsedLog.php?id=36628765&tree=Mozilla-Inbound
I had to somehow prevent name mangling to be able to match the function name in the valgrind error suppression config, so I used extern "C". https://tbpl.mozilla.org/?tree=Try&rev=63bc11258143
Attachment #8397220 - Flags: review?(bgirard) → review+
Status: REOPENED → RESOLVED
Closed: 7 years ago → 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
You need to log in before you can comment on or make changes to this bug.