Updated Branches: refs/heads/master c4d96d021 -> 699d70857
Improved the ReaperTest by using os::Fork. From: Jiang Yan Xu <y...@jxu.me> Review: https://reviews.apache.org/r/13389 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/b26a133c Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/b26a133c Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/b26a133c Branch: refs/heads/master Commit: b26a133c599ab342e570ecf050518d2f3b08dea1 Parents: f80698f Author: Benjamin Mahler <bmah...@twitter.com> Authored: Fri Aug 9 17:22:16 2013 -0700 Committer: Benjamin Mahler <bmah...@twitter.com> Committed: Fri Aug 9 17:32:23 2013 -0700 ---------------------------------------------------------------------- src/tests/reaper_tests.cpp | 108 ++++++++++++++-------------------------- 1 file changed, 37 insertions(+), 71 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/b26a133c/src/tests/reaper_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/reaper_tests.cpp b/src/tests/reaper_tests.cpp index d3e1107..608ec0e 100644 --- a/src/tests/reaper_tests.cpp +++ b/src/tests/reaper_tests.cpp @@ -34,6 +34,7 @@ #include "slave/reaper.hpp" +using namespace os; using namespace mesos; using namespace mesos::internal; using namespace mesos::internal::slave; @@ -48,67 +49,40 @@ using testing::DoDefault; // This test checks that the Reaper can monitor a non-child process. TEST(ReaperTest, NonChildProcess) { - // Use pipes to determine the pid of the grand child process. - int pipes[2]; - ASSERT_NE(-1, pipe(pipes)); - - pid_t pid = fork(); - ASSERT_NE(-1, pid); - - if (pid > 0) { - // In parent process. - close(pipes[1]); - - // Get the grand child's pid via the pipe. - ASSERT_NE(-1, read(pipes[0], &pid, sizeof(pid))); - - close(pipes[0]); - } else { - // In child process. - close(pipes[0]); - - // Double fork! - if ((pid = fork()) == -1) { - perror("Failed to fork a grand child process"); - abort(); - } - - if (pid > 0) { - // Still in child process. - exit(0); - } else { - // In grand child process. - - // Ensure the parent process exits before we write the pid. - while (getppid() != 1); - - pid = getpid(); - if (write(pipes[1], &pid, sizeof(pid)) != sizeof(pid)) { - perror("Failed to write PID on pipe"); - abort(); - } - close(pipes[1]); - while (true); // Keep waiting till we get a signal. - } - } - - // In parent process. - LOG(INFO) << "Grand child process " << pid; + // The child process creates a grandchild and then exits. The + // grandchild sleeps for 10 seconds. The process tree looks like: + // -+- child exit 0 + // \-+- grandchild sleep 10 + + // After the child exits, the grandchild is going to be re-parented + // by 'init', like this: + // -+- child (exit 0) + // -+- grandchild sleep 10 + Try<ProcessTree> tree = Fork(None(), + Fork(Exec("sleep 10")), + Exec("exit 0"))(); + ASSERT_SOME(tree); + ASSERT_EQ(1u, tree.get().children.size()); + pid_t grandchild = tree.get().children.front(); Reaper reaper; Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor); // Ask the reaper to monitor the grand child process. - Future<Option<int> > status = reaper.monitor(pid); + Future<Option<int> > status = reaper.monitor(grandchild); AWAIT_READY(monitor); + // This makes sure the status only becomes ready after the + // grandchild is killed. + EXPECT_TRUE(status.isPending()); + // Now kill the grand child. // NOTE: We send a SIGKILL here because sometimes the grand child // process seems to be in a hung state and not responding to // SIGTERM/SIGINT. - EXPECT_EQ(0, kill(pid, SIGKILL)); + EXPECT_EQ(0, kill(grandchild, SIGKILL)); Clock::pause(); @@ -134,28 +108,24 @@ TEST(ReaperTest, ChildProcess) { ASSERT_TRUE(GTEST_IS_THREADSAFE); - pid_t pid = fork(); - ASSERT_NE(-1, pid); + // The child process sleeps and will be killed by the parent. + Try<ProcessTree> tree = Fork(None(), + Exec("sleep 10"))(); - if (pid == 0) { - // In child process. Keep waiting till we get a signal. - while (true); - } - - // In parent process. - LOG(INFO) << "Child process " << pid; + ASSERT_SOME(tree); + pid_t child = tree.get(); Reaper reaper; Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor); // Ask the reaper to monitor the grand child process. - Future<Option<int> > status = reaper.monitor(pid); + Future<Option<int> > status = reaper.monitor(child); AWAIT_READY(monitor); // Now kill the child. - EXPECT_EQ(0, kill(pid, SIGKILL)); + EXPECT_EQ(0, kill(child, SIGKILL)); Clock::pause(); @@ -184,18 +154,14 @@ TEST(ReaperTest, TerminatedChildProcess) { ASSERT_TRUE(GTEST_IS_THREADSAFE); - pid_t pid = fork(); - ASSERT_NE(-1, pid); - - if (pid == 0) { - // In child process. Return directly - exit(EXIT_SUCCESS); - } + // The child process immediately exits. + Try<ProcessTree> tree = Fork(None(), + Exec("exit 0"))(); - // In parent process. - LOG(INFO) << "Child process " << pid; + ASSERT_SOME(tree); + pid_t child = tree.get(); - ASSERT_SOME(os::process(pid)); + ASSERT_SOME(os::process(child)); Clock::pause(); @@ -203,7 +169,7 @@ TEST(ReaperTest, TerminatedChildProcess) // Because reaper reaps all child processes even if they aren't // registered, we advance time until that happens. - while (os::process(pid).isSome()) { + while (os::process(child).isSome()) { Clock::advance(Seconds(1)); Clock::settle(); } @@ -214,7 +180,7 @@ TEST(ReaperTest, TerminatedChildProcess) Future<Nothing> monitor = FUTURE_DISPATCH(_, &ReaperProcess::monitor); // Ask the reaper to monitor the child process. - Future<Option<int> > status = reaper.monitor(pid); + Future<Option<int> > status = reaper.monitor(child); AWAIT_READY(monitor);
