daemon: stop eating SIGINTs

Daemon client handler processes are forked off of the main nix process
and thus will not have a signal handler thread anymore. This leads to a
high likelihood of bustage, since the Worker infrastructure expects the
interrupt infrastructure to actually, you know, work, to be able to get
interrupted.

The expected behaviour after fork is either:
- Start a signal handler thread if you expect to do complicated things
  that need ReceiveInterrupts.
- Call restoreProcessContext and don't handle signals specially
  otherwise.

Change-Id: I73d36b5bbf96dddd21d5e1c3bd0484d715c00e8b
This commit is contained in:
jade 2024-10-18 20:24:54 -07:00
parent 60578b4d7d
commit dccde94369
8 changed files with 63 additions and 13 deletions

View file

@ -3,7 +3,7 @@
namespace nix { namespace nix {
void commonChildInit() void commonExecveingChildInit()
{ {
logger = makeSimpleLogger(); logger = makeSimpleLogger();

View file

@ -4,8 +4,12 @@
namespace nix { namespace nix {
/** /**
* Common initialisation performed in child processes. * Common initialisation performed in child processes that are just going to
* execve.
*
* These processes may not use ReceiveInterrupts as they do not have an
* interrupt receiving thread.
*/ */
void commonChildInit(); void commonExecveingChildInit();
} }

View file

@ -45,7 +45,7 @@ HookInstance::HookInstance()
if (dup2(fromHook_.writeSide.get(), STDERR_FILENO) == -1) if (dup2(fromHook_.writeSide.get(), STDERR_FILENO) == -1)
throw SysError("cannot pipe standard error into log file"); throw SysError("cannot pipe standard error into log file");
commonChildInit(); commonExecveingChildInit();
if (chdir("/") == -1) throw SysError("changing into /"); if (chdir("/") == -1) throw SysError("changing into /");

View file

@ -1364,7 +1364,7 @@ void LocalDerivationGoal::runChild()
try { /* child */ try { /* child */
commonChildInit(); commonExecveingChildInit();
setupSyscallFilter(); setupSyscallFilter();

View file

@ -23,8 +23,9 @@ void setStackSize(rlim_t stackSize);
/** /**
* Restore the original inherited Unix process context (such as signal * Restore the original inherited Unix process context (such as signal
* masks, stack size). * masks, stack size). This should generally be called after fork for a process
* intending to simply execve.
*
* See startSignalHandlerThread(), saveSignalMask(). * See startSignalHandlerThread(), saveSignalMask().
*/ */
void restoreProcessContext(bool restoreMounts = true); void restoreProcessContext(bool restoreMounts = true);

View file

@ -118,11 +118,16 @@ void saveSignalMask() {
savedSignalMaskIsSet = true; savedSignalMaskIsSet = true;
} }
void startSignalHandlerThread() void startSignalHandlerThread(DoSignalSave doSave)
{ {
updateWindowSize(); updateWindowSize();
switch (doSave) {
case DoSignalSave::Save:
saveSignalMask(); saveSignalMask();
break;
case DoSignalSave::DontSaveBecauseAdvancedProcess: break;
}
sigset_t set; sigset_t set;
sigemptyset(&set); sigemptyset(&set);

View file

@ -1,5 +1,23 @@
#pragma once #pragma once
/// @file /** @file Signal handling in Lix
*
* Processes are expected to be either:
* - Advanced processes which call into Lix's logic, like the daemon processes.
* - Basic processes that are just going to execve.
*
* Processes should be set up accordingly following a fork:
* In the first case, such processes should have a signal handler thread that
* catches SIGINT and dispatches it to the rest of the system so they should
* call startSignalHandlerThread(). In the second case, processes should call
* restoreProcessContext(), possibly with `false` (depends on whether mounts
* should be restored), which will unmask SIGINT and other signals that were
* previously masked in an advanced process such as the one that started
* them, so the process can be interrupted.
*
* It is generally a mistake to fork a process without at least calling
* restoreSignals() or restoreProcessContext().
*/
#include "error.hh" #include "error.hh"
@ -36,13 +54,31 @@ MakeError(Interrupted, BaseError);
void restoreSignals(); void restoreSignals();
/**
* Whether to save the signal mask when starting the signal handler thread.
*
* The signal mask shouldn't be saved if the current signal mask is the one for
* processes with a signal handler thread.
*/
enum class DoSignalSave
{
Save,
DontSaveBecauseAdvancedProcess,
};
/** /**
* Start a thread that handles various signals. Also block those signals * Start a thread that handles various signals. Also block those signals
* on the current thread (and thus any threads created by it). * on the current thread (and thus any threads created by it).
* Saves the signal mask before changing the mask to block those signals. *
* See saveSignalMask(). * Optionally saves the signal mask before changing the mask to block those
* signals. See saveSignalMask().
*
* This should also be executed after certain forks from Lix processes that
* expect to be "advanced" (see file doc comment), since the signal thread will
* die on fork. Of course this whole situation is kind of unsound since we
* definitely violate async-signal-safety requirements, but, well.
*/ */
void startSignalHandlerThread(); void startSignalHandlerThread(DoSignalSave doSave = DoSignalSave::Save);
/** /**
* Saves the signal mask, which is the signal mask that nix will restore * Saves the signal mask, which is the signal mask that nix will restore

View file

@ -356,6 +356,10 @@ static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
if (setsid() == -1) if (setsid() == -1)
throw SysError("creating a new session"); throw SysError("creating a new session");
// Restart the signal handler thread since it met its untimely
// demise at fork time.
startSignalHandlerThread(DoSignalSave::DontSaveBecauseAdvancedProcess);
// Restore normal handling of SIGCHLD. // Restore normal handling of SIGCHLD.
setSigChldAction(false); setSigChldAction(false);