* 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)
This commit is contained in:
		
							parent
							
								
									b2b6cf3fc8
								
							
						
					
					
						commit
						8ab229ddf2
					
				
					 3 changed files with 27 additions and 10 deletions
				
			
		|  | @ -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<set<int> >(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; | ||||
|     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<set<int> >(logPipe.readSide), true); | ||||
|  |  | |||
|  | @ -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); | ||||
|  |  | |||
|  | @ -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); | ||||
| }; | ||||
| 
 | ||||
| 
 | ||||
|  |  | |||
		Loading…
	
	Add table
		Add a link
		
	
		Reference in a new issue