Repository: mesos Updated Branches: refs/heads/master 00318fc1b -> 740dcb3d5
Terminated the perf subprocess once the parent exits. Review: https://reviews.apache.org/r/32805 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/740dcb3d Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/740dcb3d Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/740dcb3d Branch: refs/heads/master Commit: 740dcb3d55944bc1410818d48efc49f0091b037d Parents: d3bea44 Author: Jie Yu <[email protected]> Authored: Thu Apr 2 18:38:00 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Fri Apr 3 14:33:59 2015 -0700 ---------------------------------------------------------------------- src/linux/perf.cpp | 93 +++++++++++++++++++++++++++++++++++++++++++++++-- 1 file changed, 91 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/740dcb3d/src/linux/perf.cpp ---------------------------------------------------------------------- diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp index 8d9a373..697b75e 100644 --- a/src/linux/perf.cpp +++ b/src/linux/perf.cpp @@ -16,6 +16,14 @@ * limitations under the License. */ +#include <signal.h> +#include <stdlib.h> +#include <unistd.h> + +#include <sys/prctl.h> +#include <sys/types.h> +#include <sys/wait.h> + #include <list> #include <ostream> #include <vector> @@ -28,6 +36,9 @@ #include <process/subprocess.hpp> #include <stout/strings.hpp> +#include <stout/unreachable.hpp> + +#include <stout/os/signals.hpp> #include "linux/perf.hpp" @@ -176,13 +187,88 @@ protected: // Kill the perf process if it's still running. if (perf.isSome() && perf.get().status().isPending()) { - kill(perf.get().pid(), SIGKILL); + kill(perf.get().pid(), SIGTERM); } promise.discard(); } private: + static void signalHandler(int signal) + { + // Send SIGKILL to every process in the process group of the + // calling process. This will terminate both the perf process + // (including its children) and the bookkeeping process. + kill(0, SIGKILL); + abort(); + } + + // This function is invoked right before each 'perf' is exec'ed. + // Note that this function needs to be async signal safe. In fact, + // all the library functions we used in this function are async + // signal safe. + static int setupChild() + { + // Send SIGTERM to the current process if the parent (i.e., the + // slave) exits. Note that this function should always succeed + // because we are passing in a valid signal. + prctl(PR_SET_PDEATHSIG, SIGTERM); + + // Put the current process into a separate process group so that + // we can kill it and all its children easily. + if (setpgid(0, 0) != 0) { + abort(); + } + + // Install a SIGTERM handler which will kill the current process + // group. Since we already setup the death signal above, the + // signal handler will be triggered when the parent (i.e., the + // slave) exits. + if (os::signals::install(SIGTERM, &signalHandler) != 0) { + abort(); + } + + pid_t pid = fork(); + if (pid == -1) { + abort(); + } else if (pid == 0) { + // Child. This is the process that is going to exec the perf + // process if zero is returned. + + // We setup death signal for the perf process as well in case + // someone, though unlikely, accidentally kill the parent of + // this process (the bookkeeping process). + prctl(PR_SET_PDEATHSIG, SIGKILL); + + // NOTE: We don't need to clear the signal handler explicitly + // because the subsequent 'exec' will clear them. + return 0; + } else { + // Parent. This is the bookkeeping process which will wait for + // the perf process to finish. + + // Close the files to prevent interference on the communication + // between the slave and the perf process. + close(STDIN_FILENO); + close(STDOUT_FILENO); + close(STDERR_FILENO); + + // Block until the perf process finishes. + int status = 0; + if (waitpid(pid, &status, 0) == -1) { + abort(); + } + + // Forward the exit status if the perf process exits normally. + if (WIFEXITED(status)) { + _exit(WEXITSTATUS(status)); + } + + abort(); + UNREACHABLE(); + } + } + void sample() { Try<Subprocess> _perf = subprocess( @@ -190,7 +276,10 @@ private: argv, Subprocess::PIPE(), Subprocess::PIPE(), - Subprocess::PIPE()); + Subprocess::PIPE(), + None(), + None(), + setupChild); if (_perf.isError()) { promise.fail("Failed to launch perf process: " + _perf.error());
