(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.)
Is the |dispatchQuit| single quit logic necessary?
It looks like nsAppStartup::mShuttingDown already ensures that multiple Quit() calls will be harmless.
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.
Comment on attachment 8959924 SIGINT/ SIGHUP and quit application gracefully.
Bug 336193 - P1: Use SignalPipeWatcher to detect signals SIGTERM/
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/nsNativeApp SupportUnix. cpp:692 nc();
(Diff revision 1)
> + static bool dispatchQuit = false;
> + if (!dispatchQuit) {
> + dispatchQuit = true;
> + ForceAppQuitAsy
> + }
Is the |dispatchQuit| single quit logic necessary? :mShuttingDown already ensures that multiple Quit() calls will be harmless.
It looks like nsAppStartup:
https:/ /searchfox. org/mozilla- central/ rev/b29daa46443 b30612415c35be0 a3c9c13b9dc5f6/ 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 sa_handler == SIG_IGN) {
(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.
::: xpcom/base/ nsDumpUtils. cpp:145 sa_handler == SIG_IGN) {
(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.
Why prevent registering callbacks for signals where the old action is SIG_IGN?
I guessing this is not necessary.
::: xpcom/base/ nsDumpUtils. cpp:163 oLock); y::index_ type i = 0; i < mSignalInfo. Length( ); i++) { i].mSignal) { i].oldAction; >sa_handler &&
(Diff revision 1)
> + // Restore previous signal action
> + MutexAutoLock lock(mSignalInf
> + for (SignalInfoArra
> + if (aSignal == mSignalInfo[
> + struct sigaction* oldAction = &mSignalInfo[
> + if (oldAction-
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?