Comment on attachment 8959924 Bug 336193 - P1: Use SignalPipeWatcher to detect signals SIGTERM/SIGINT/SIGHUP and quit application gracefully.
https://reviewboard.mozilla.org/r/228682/#review235302 (I'm not an XPCOM peer and so I considered passing the nsDumpUtils part to Nathan, but the initial implementation of SignalPipeWatcher was not reviewed by an XPCOM peer either, and so I guess it doesn't really matter who reviews.) ::: toolkit/xre/nsNativeAppSupportUnix.cpp:692 (Diff revision 1) > + static bool dispatchQuit = false; > + if (!dispatchQuit) { > + dispatchQuit = true; > + ForceAppQuitAsync(); > + } Is the |dispatchQuit| single quit logic necessary? It looks like nsAppStartup::mShuttingDown already ensures that multiple Quit() calls will be harmless. https://searchfox.org/mozilla- central/rev/b29daa46443b30612415c35be0a3c9c13b9dc5f6/toolkit/components/startup/nsAppStartup.cpp#393 ::: xpcom/base/nsDumpUtils.h:146 (Diff revision 1) > PipeCallback mCallback; > + struct sigaction oldAction; Please follow existing convention with |mOldAction|. ::: xpcom/base/nsDumpUtils.cpp:145 (Diff revision 1) > } > - SignalInfo signalInfo = { aSignal, aCallback }; > + > + // Save current signal action so it can be restored later > + struct sigaction oldAction = { 0 }; > + if (sigaction(aSignal, nullptr, &oldAction) != 0 || > + oldAction.sa_handler == SIG_IGN) { ::: xpcom/base/nsDumpUtils.cpp:145 (Diff revision 1) > } > - SignalInfo signalInfo = { aSignal, aCallback }; > + > + // Save current signal action so it can be restored later > + struct sigaction oldAction = { 0 }; > + if (sigaction(aSignal, nullptr, &oldAction) != 0 || > + oldAction.sa_handler == SIG_IGN) { Why prevent registering callbacks for signals where the old action is SIG_IGN? I guessing this is not necessary. ::: xpcom/base/nsDumpUtils.cpp:163 (Diff revision 1) > + // Restore previous signal action > + MutexAutoLock lock(mSignalInfoLock); > + for (SignalInfoArray::index_type i = 0; i < mSignalInfo.Length(); i++) { > + if (aSignal == mSignalInfo[i].mSignal) { > + struct sigaction* oldAction = &mSignalInfo[i].oldAction; > + if (oldAction->sa_handler && SIG_DFL is typically 0. I assume this is intended to detect when oldAction has not been set, but mSignalInfo elements are added only when the old action is successfully retrieved. Can this check just be dropped? -- You received this bug notification because you are a member of Desktop Packages, which is subscribed to firefox in Ubuntu. https://bugs.launchpad.net/bugs/73536 Title: MASTER Firefox crashes on instant X server shutdown Status in Mozilla Firefox: In Progress Status in firefox package in Ubuntu: Won't Fix Status in firefox-3.0 package in Ubuntu: Triaged Bug description: Firefox crashes when X server is forcefully torn down (e.g. by pressing ctrl-alt-backspace) and a crash report gets generated on next login. (Original Report: I've reproduced it once on my machine with the following steps. With, oh, about 5 tabs open, I just pressed ctrl-alt-backspace, logged back in when the X server restarted, and FF crashed with a bug report when Gnome tried to restore the session. It's not terribly important, I don't think anyone does this very often, but maybe it'll be helpful. ) To manage notifications about this bug go to: https://bugs.launchpad.net/firefox/+bug/73536/+subscriptions -- Mailing list: https://launchpad.net/~desktop-packages Post to : desktop-packages@lists.launchpad.net Unsubscribe : https://launchpad.net/~desktop-packages More help : https://help.launchpad.net/ListHelp
