Skip to content

Commit

Permalink
Use process group signal handling to reach past the shell
Browse files Browse the repository at this point in the history
Spawn processes is their own process group and then signal the
process group intstead of just the process we spawned to ensure
the signals also reach any subprocesses started by the process we
spawned (which is always the case as we are wrapping all our
processes spawning with "/bin/sh -c ...").

This make sending SIGINT work again, so revert back to that instead
of SIGKILL. The reason SIGINT wasn't working was the shell logic
is that this is expected to be sent to the process group, so it
doesn't need to pass it along or anything. See the bash signals
documentation page for more details

https://www.gnu.org/software/bash/manual/html_node/Signals.html
  • Loading branch information
twhitehead committed Oct 8, 2024
1 parent 0f8e18f commit a022e79
Showing 1 changed file with 4 additions and 3 deletions.
7 changes: 4 additions & 3 deletions src/tools/Subprocess.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -51,19 +51,19 @@ class SubprocessPid {
void stop() noexcept {
// Signals give problems with MPI on Travis.
// I disable them for now.
if(SubprocessPidGetenvSignals()) kill(pid,SIGSTOP);
if(SubprocessPidGetenvSignals()) kill(-pid,SIGSTOP);
}
void cont() noexcept {
// Signals give problems with MPI on Travis.
// I disable them for now.
if(SubprocessPidGetenvSignals()) kill(pid,SIGCONT);
if(SubprocessPidGetenvSignals()) kill(-pid,SIGCONT);
}
~SubprocessPid() {
// the destructor implies we do not need the subprocess anymore, so SIGKILL
// is the fastest exit.
// if we want to gracefully kill the process with a delay, it would be cleaner
// to have another member function
kill(pid,SIGKILL);
kill(-pid,SIGINT);
// Wait for the child process to terminate
// This is anyway required to avoid leaks
int status;
Expand Down Expand Up @@ -99,6 +99,7 @@ Subprocess::Subprocess(const std::string & cmd) {
if(dup(pc[0])<0) plumed_error()<<"error duplicating file";
if(close(pc[1])<0) plumed_error()<<"error closing file";
if(close(cp[0])<0) plumed_error()<<"error closing file";
if(setpgid(0,0)<0) plumed_error()<<"error setting process group";;
auto err=execv(arr[0],arr);
plumed_error()<<"error in script file " << cmd << ", execv returned "<<err;
}
Expand Down

0 comments on commit a022e79

Please sign in to comment.