You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by ji...@apache.org on 2015/04/03 23:35:59 UTC

[1/3] mesos git commit: Terminated the perf subprocess once the parent exits.

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 <yu...@gmail.com>
Authored: Thu Apr 2 18:38:00 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
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());


[2/3] mesos git commit: Added os::signals::install to install signal handlers.

Posted by ji...@apache.org.
Added os::signals::install to install signal handlers.

Review: https://reviews.apache.org/r/32833


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d3bea443
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d3bea443
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d3bea443

Branch: refs/heads/master
Commit: d3bea4433fb06011f673335ec81fe6af2c7592b2
Parents: d66d30c
Author: Jie Yu <yu...@gmail.com>
Authored: Fri Apr 3 14:01:37 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Apr 3 14:33:59 2015 -0700

----------------------------------------------------------------------
 .../3rdparty/stout/include/stout/os/signals.hpp          | 11 +++++++++++
 1 file changed, 11 insertions(+)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d3bea443/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp b/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
index 30232f5..3523070 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/signals.hpp
@@ -24,6 +24,17 @@ namespace os {
 
 namespace signals {
 
+// Installs the given signal handler.
+inline int install(int signal, void(*handler)(int))
+{
+  struct sigaction action;
+  memset(&action, 0, sizeof(action));
+  sigemptyset(&action.sa_mask);
+  action.sa_handler = handler;
+  return sigaction(signal, &action, NULL);
+}
+
+
 // Resets the signal handler to the default handler of the signal.
 inline int reset(int signal)
 {


[3/3] mesos git commit: Fixed the non-POD global variable in perf sampler.

Posted by ji...@apache.org.
Fixed the non-POD global variable in perf sampler.

Review: https://reviews.apache.org/r/32820


Project: http://git-wip-us.apache.org/repos/asf/mesos/repo
Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/d66d30c8
Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/d66d30c8
Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/d66d30c8

Branch: refs/heads/master
Commit: d66d30c8bbdcc988f52449c149b7a9c309ceee33
Parents: 00318fc
Author: Jie Yu <yu...@gmail.com>
Authored: Thu Apr 2 12:16:01 2015 -0700
Committer: Jie Yu <yu...@gmail.com>
Committed: Fri Apr 3 14:33:59 2015 -0700

----------------------------------------------------------------------
 src/linux/perf.cpp | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/d66d30c8/src/linux/perf.cpp
----------------------------------------------------------------------
diff --git a/src/linux/perf.cpp b/src/linux/perf.cpp
index cad6c80..8d9a373 100644
--- a/src/linux/perf.cpp
+++ b/src/linux/perf.cpp
@@ -31,22 +31,22 @@
 
 #include "linux/perf.hpp"
 
+using namespace process;
+
 using std::list;
 using std::ostringstream;
 using std::set;
 using std::string;
 using std::vector;
 
-using namespace process;
-
 namespace perf {
 
 // Delimiter for fields in perf stat output.
-const string PERF_DELIMITER = ",";
+static const char PERF_DELIMITER[] = ",";
 
 // Use an empty string as the key for the parse output when sampling a
 // set of pids. No valid cgroup can be an empty string.
-const string PIDS_KEY = "";
+static const char PIDS_KEY[] = "";
 
 namespace internal {
 
@@ -202,8 +202,8 @@ private:
     // Start reading from stdout and stderr now. We don't use stderr
     // but must read from it to avoid the subprocess blocking on the
     // pipe.
-    output.push_back(process::io::read(perf.get().out().get()));
-    output.push_back(process::io::read(perf.get().err().get()));
+    output.push_back(io::read(perf.get().out().get()));
+    output.push_back(io::read(perf.get().err().get()));
 
     // Wait for the process to exit.
     perf.get().status()