From 8ab229ddf2383ec6455836f342539b8c89356f76 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 19 Mar 2007 12:48:45 +0000 Subject: [PATCH] * Terminate build hooks and substitutes with a TERM signal, not a KILL signal. This is necessary because those processes may have joined the BDB environment, so they have to be given a chance to clean up. (NIX-85) --- src/libstore/build.cc | 19 +++++++++++++------ src/libutil/util.cc | 16 ++++++++++++---- src/libutil/util.hh | 2 ++ 3 files changed, 27 insertions(+), 10 deletions(-) diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 368d5751f..90ebeaa79 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -628,7 +628,7 @@ private: HookReply tryBuildHook(); /* Synchronously wait for a build hook to finish. */ - void terminateBuildHook(); + void terminateBuildHook(bool kill = false); /* Acquires locks on the output paths and gathers information about the build (e.g., the input closures). During this @@ -991,6 +991,7 @@ static string readLine(int fd) { string s; while (1) { + checkInterrupt(); char ch; ssize_t rd = read(fd, &ch, 1); if (rd == -1) { @@ -1018,6 +1019,7 @@ static void drain(int fd) { unsigned char buffer[1024]; while (1) { + checkInterrupt(); ssize_t rd = read(fd, buffer, sizeof buffer); if (rd == -1) { if (errno != EINTR) @@ -1124,6 +1126,7 @@ DerivationGoal::HookReply DerivationGoal::tryBuildHook() /* parent */ pid.setSeparatePG(true); + pid.setKillSignal(SIGTERM); logPipe.writeSide.close(); worker.childStarted(shared_from_this(), pid, singleton >(logPipe.readSide), false); @@ -1139,7 +1142,7 @@ DerivationGoal::HookReply DerivationGoal::tryBuildHook() try { reply = readLine(fromHook.readSide); } catch (Error & e) { - drain(logPipe.readSide); + terminateBuildHook(true); throw; } @@ -1147,7 +1150,6 @@ DerivationGoal::HookReply DerivationGoal::tryBuildHook() if (reply == "decline" || reply == "postpone") { /* Clean up the child. !!! hacky / should verify */ - drain(logPipe.readSide); terminateBuildHook(); return reply == "decline" ? rpDecline : rpPostpone; } @@ -1215,18 +1217,22 @@ DerivationGoal::HookReply DerivationGoal::tryBuildHook() } -void DerivationGoal::terminateBuildHook() +void DerivationGoal::terminateBuildHook(bool kill) { /* !!! drain stdout of hook */ debug("terminating build hook"); pid_t savedPid = pid; - pid.wait(true); + if (kill) + pid.kill(); + else + pid.wait(true); /* `false' means don't wake up waiting goals, since we want to - keep this build slot ourselves (at least if the hook reply XXX. */ + keep this build slot ourselves. */ worker.childTerminated(savedPid, false); fromHook.readSide.close(); toHook.writeSide.close(); fdLogFile.close(); + drain(logPipe.readSide); logPipe.readSide.close(); deleteTmpDir(true); /* get rid of the hook's temporary directory */ } @@ -2048,6 +2054,7 @@ void SubstitutionGoal::tryToRun() /* parent */ pid.setSeparatePG(true); + pid.setKillSignal(SIGTERM); logPipe.writeSide.close(); worker.childStarted(shared_from_this(), pid, singleton >(logPipe.readSide), true); diff --git a/src/libutil/util.cc b/src/libutil/util.cc index 1576e1e8b..ae5af2492 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -641,6 +641,7 @@ Pid::Pid() { pid = -1; separatePG = false; + killSignal = SIGKILL; } @@ -654,6 +655,7 @@ void Pid::operator =(pid_t pid) { if (this->pid != pid) kill(); this->pid = pid; + killSignal = SIGKILL; // reset signal to default } @@ -669,10 +671,10 @@ void Pid::kill() printMsg(lvlError, format("killing process %1%") % pid); - /* Send a KILL signal to the child. If it has its own process - group, send the signal to every process in the child process - group (which hopefully includes *all* its children). */ - if (::kill(separatePG ? -pid : pid, SIGKILL) != 0) + /* Send the requested signal to the child. If it has its own + process group, send the signal to every process in the child + process group (which hopefully includes *all* its children). */ + if (::kill(separatePG ? -pid : pid, killSignal) != 0) printMsg(lvlError, (SysError(format("killing process %1%") % pid).msg())); /* Wait until the child dies, disregarding the exit status. */ @@ -710,6 +712,12 @@ void Pid::setSeparatePG(bool separatePG) } +void Pid::setKillSignal(int signal) +{ + this->killSignal = signal; +} + + void killUser(uid_t uid) { debug(format("killing all processes running under uid `%1%'") % uid); diff --git a/src/libutil/util.hh b/src/libutil/util.hh index 3c4629957..4d284ccfd 100644 --- a/src/libutil/util.hh +++ b/src/libutil/util.hh @@ -213,6 +213,7 @@ class Pid { pid_t pid; bool separatePG; + int killSignal; public: Pid(); ~Pid(); @@ -221,6 +222,7 @@ public: void kill(); int wait(bool block); void setSeparatePG(bool separatePG); + void setKillSignal(int signal); };