Repository: mesos
Updated Branches:
  refs/heads/master 8b93e9aaa -> 3073bd4e6


os::killtree() supports case when root pid has exited.

Improve os::killtree() for the case when the root pid has exited and
it has been instructed to either follow process groups or sessions. It
will now kill all processes (and their trees) with pgid or session
matching the root pid.

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


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

Branch: refs/heads/master
Commit: 3073bd4e6fc119875fef22b364872056ef97efd3
Parents: 8b93e9a
Author: Ian Downes <[email protected]>
Authored: Fri Oct 31 13:06:13 2014 -0700
Committer: Vinod Kone <[email protected]>
Committed: Fri Oct 31 13:06:14 2014 -0700

----------------------------------------------------------------------
 .../stout/include/stout/os/killtree.hpp         | 37 ++++++--
 .../3rdparty/stout/tests/os_tests.cpp           | 98 ++++++++++++++++++++
 2 files changed, 127 insertions(+), 8 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/3073bd4e/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp 
b/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
index fa1950c..933a5c9 100644
--- a/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
+++ b/3rdparty/libprocess/3rdparty/stout/include/stout/os/killtree.hpp
@@ -46,6 +46,9 @@ inline Option<Process> process(pid_t, const 
std::list<Process>&);
 // Note that processes of the group and session of the parent of the
 // root process is not included unless they are part of the root
 // process tree.
+// Note that if the process 'pid' has exited we'll signal the process
+// tree(s) rooted at pids in the group or session led by the process
+// if groups = true or sessions = true, respectively.
 // Returns the process trees that were succesfully or unsuccessfully
 // signaled. Note that the process trees can be stringified.
 // TODO(benh): Allow excluding the root pid from stopping, killing,
@@ -66,10 +69,30 @@ inline Try<std::list<ProcessTree> > killtree(
 
   Result<Process> process = os::process(pid, processes.get());
 
+  std::queue<pid_t> queue;
+
+  // If the root process has already terminated we'll add in any pids
+  // that are in the process group originally led by pid or in the
+  // session originally led by pid, if instructed.
   if (process.isNone()) {
-    // We do not consider it an error if the process is not present since it
-    // can exit at any time.
-    return std::list<ProcessTree>();
+    foreach (const Process& _process, processes.get()) {
+      if (groups && _process.group == pid) {
+        queue.push(_process.pid);
+      } else if (sessions &&
+                 _process.session.isSome() &&
+                 _process.session.get() == pid) {
+        queue.push(_process.pid);
+      }
+    }
+
+    // Root process is not running and no processes found in the
+    // process group or session so nothing we can do.
+    if (queue.empty()) {
+      return std::list<ProcessTree>();
+    }
+  } else {
+    // Start the traversal from pid as the root.
+    queue.push(pid);
   }
 
   struct {
@@ -81,8 +104,9 @@ inline Try<std::list<ProcessTree> > killtree(
 
   // If we are following groups and/or sessions then we try and make
   // the group and session of the parent process "already visited" so
-  // that we don't kill "up the tree".
-  if (groups || sessions) {
+  // that we don't kill "up the tree". This can only be done if the
+  // process is present.
+  if (process.isSome() && (groups || sessions)) {
     Option<Process> parent =
       os::process(process.get().parent, processes.get());
 
@@ -96,9 +120,6 @@ inline Try<std::list<ProcessTree> > killtree(
     }
   }
 
-  std::queue<pid_t> queue;
-  queue.push(pid);
-
   while (!queue.empty()) {
     pid_t pid = queue.front();
     queue.pop();

http://git-wip-us.apache.org/repos/asf/mesos/blob/3073bd4e/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
----------------------------------------------------------------------
diff --git a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp 
b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
index e9f37df..3f39017 100644
--- a/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
+++ b/3rdparty/libprocess/3rdparty/stout/tests/os_tests.cpp
@@ -623,6 +623,104 @@ TEST_F(OsTest, killtree)
 }
 
 
+TEST_F(OsTest, killtreeNoRoot)
+{
+  Try<ProcessTree> tree =
+    Fork(dosetsid,        // Child.
+         Fork(None(),     // Grandchild.
+              Fork(None(),
+                   Exec("sleep 100")),
+              Exec("sleep 100")),
+         Exec("exit 0"))();
+  ASSERT_SOME(tree);
+
+  // The process tree we instantiate initially looks like this:
+  //
+  // -+- child exit 0             [new session and process group leader]
+  //  \-+- grandchild sleep 100
+  //   \-+- great grandchild sleep 100
+  //
+  // But becomes the following tree after the child exits:
+  //
+  // -+- child (exited 0)
+  //  \-+- grandchild sleep 100
+  //   \-+- great grandchild sleep 100
+  //
+  // And gets reparented to init when we reap the child:
+  //
+  // -+- init (pid 1)
+  //  \-+- grandchild sleep 100
+  //   \-+- great grandchild sleep 100
+
+  // Grab the pids from the instantiated process tree.
+  ASSERT_EQ(1u, tree.get().children.size());
+  ASSERT_EQ(1u, tree.get().children.front().children.size());
+
+  pid_t child = tree.get();
+  pid_t grandchild = tree.get().children.front();
+  pid_t greatGrandchild = tree.get().children.front().children.front();
+
+  // Wait for the child to exit.
+  Duration elapsed = Duration::zero();
+  while (true) {
+    Result<os::Process> process = os::process(child);
+    ASSERT_FALSE(process.isError());
+
+    if (process.get().zombie) {
+      break;
+    }
+
+    if (elapsed > Seconds(1)) {
+      FAIL() << "Child process " << stringify(child) << " did not terminate";
+    }
+
+    os::sleep(Milliseconds(5));
+    elapsed += Milliseconds(5);
+  }
+
+  // Ensure we reap our child now.
+  EXPECT_SOME(os::process(child));
+  EXPECT_TRUE(os::process(child).get().zombie);
+  ASSERT_EQ(child, waitpid(child, NULL, 0));
+
+  // Check the grandchild and great grandchild are still running.
+  ASSERT_TRUE(os::exists(grandchild));
+  ASSERT_TRUE(os::exists(greatGrandchild));
+
+  // Check the subtree has been reparented by init.
+  Result<os::Process> _grandchild = os::process(grandchild);
+  ASSERT_SOME(_grandchild);
+  ASSERT_EQ(1, _grandchild.get().parent);
+
+  // Kill the process tree. Even though the root process has exited,
+  // we specify to follow sessions and groups which should kill the
+  // grandchild and greatgrandchild.
+  Try<std::list<ProcessTree>> trees = os::killtree(child, SIGKILL, true, true);
+
+  ASSERT_SOME(trees);
+  EXPECT_FALSE(trees.get().empty());
+
+  // All processes should be reparented and reaped by init.
+  elapsed = Duration::zero();
+  while (true) {
+    if (os::process(grandchild).isNone() &&
+        os::process(greatGrandchild).isNone()) {
+      break;
+    }
+
+    if (elapsed > Seconds(10)) {
+      FAIL() << "Processes were not reaped after killtree invocation";
+    }
+
+    os::sleep(Milliseconds(5));
+    elapsed += Milliseconds(5);
+  }
+
+  EXPECT_NONE(os::process(grandchild));
+  EXPECT_NONE(os::process(greatGrandchild));
+}
+
+
 TEST_F(OsTest, pstree)
 {
   Try<ProcessTree> tree = os::pstree(getpid());

Reply via email to