Replaced clone system call with os::clone. Review: https://reviews.apache.org/r/38625
Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/3b1eeec8 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/3b1eeec8 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/3b1eeec8 Branch: refs/heads/master Commit: 3b1eeec85e51d548d9152da838a9cddebd14cf17 Parents: f279cf2 Author: haosdent huang <[email protected]> Authored: Tue Sep 22 23:03:41 2015 -0700 Committer: Jie Yu <[email protected]> Committed: Tue Sep 22 23:03:41 2015 -0700 ---------------------------------------------------------------------- src/slave/containerizer/linux_launcher.cpp | 55 ++++--------------------- src/tests/containerizer/launch_tests.cpp | 22 +--------- src/tests/containerizer/ns_tests.cpp | 52 +++-------------------- 3 files changed, 13 insertions(+), 116 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/slave/containerizer/linux_launcher.cpp ---------------------------------------------------------------------- diff --git a/src/slave/containerizer/linux_launcher.cpp b/src/slave/containerizer/linux_launcher.cpp index fd0ffcf..459af1b 100644 --- a/src/slave/containerizer/linux_launcher.cpp +++ b/src/slave/containerizer/linux_launcher.cpp @@ -162,53 +162,6 @@ Future<hashset<ContainerID>> LinuxLauncher::recover( } -// Helper for clone() which expects an int(void*). -static int childMain(void* _func) -{ - const lambda::function<int()>* func = - static_cast<const lambda::function<int()>*> (_func); - - return (*func)(); -} - - -// The customized clone function which will be used by 'subprocess()'. -static pid_t clone( - const lambda::function<int()>& func, - const Option<int>& namespaces) -{ - // Stack for the child. - // - unsigned long long used for best alignment. - // - static is ok because each child gets their own copy after the clone. - // - 8 MiB appears to be the default for "ulimit -s" on OSX and Linux. - // - // NOTE: We need to allocate the stack dynamically. This is because - // glibc's 'clone' will modify the stack passed to it, therefore the - // stack must NOT be shared as multiple 'clone's can be invoked - // simultaneously. - int stackSize = 8 * 1024 * 1024; - - unsigned long long *stack = - new unsigned long long[stackSize/sizeof(unsigned long long)]; - - int flags = namespaces.isSome() ? namespaces.get() : 0; - flags |= SIGCHLD; // Specify SIGCHLD as child termination signal. - - LOG(INFO) << "Cloning child process with flags = " - << ns::stringify(flags); - - pid_t pid = ::clone( - childMain, - &stack[stackSize/sizeof(stack[0]) - 1], // stack grows down. - flags, - (void*) &func); - - delete[] stack; - - return pid; -} - - static int childSetup( int pipes[2], const Option<lambda::function<int()>>& setup) @@ -284,6 +237,12 @@ Try<pid_t> LinuxLauncher::fork( // use CHECK. CHECK_EQ(0, ::pipe(pipes)); + int cloneFlags = namespaces.isSome() ? namespaces.get() : 0; + cloneFlags |= SIGCHLD; // Specify SIGCHLD as child termination signal. + + LOG(INFO) << "Cloning child process with flags = " + << ns::stringify(cloneFlags); + Try<Subprocess> child = subprocess( path, argv, @@ -293,7 +252,7 @@ Try<pid_t> LinuxLauncher::fork( flags, environment, lambda::bind(&childSetup, pipes, setup), - lambda::bind(&clone, lambda::_1, namespaces)); + lambda::bind(&os::clone, lambda::_1, cloneFlags)); if (child.isError()) { return Error("Failed to clone child process: " + child.error()); http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/launch_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/launch_tests.cpp b/src/tests/containerizer/launch_tests.cpp index dbe891c..de655ec 100644 --- a/src/tests/containerizer/launch_tests.cpp +++ b/src/tests/containerizer/launch_tests.cpp @@ -81,33 +81,13 @@ public: launchFlags, None(), None(), - lambda::bind(&clone, lambda::_1)); + lambda::bind(&os::clone, lambda::_1, CLONE_NEWNS | SIGCHLD)); close(launchFlags.pipe_read.get()); close(launchFlags.pipe_write.get()); return s; } - -private: - static pid_t clone(const lambda::function<int()>& f) - { - static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)]; - - return ::clone( - _clone, - &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down. - CLONE_NEWNS | SIGCHLD, // Specify SIGCHLD as child termination signal. - (void*) &f); - } - - static int _clone(void* f) - { - const lambda::function<int()>* _f = - static_cast<const lambda::function<int()>*> (f); - - return (*_f)(); - } }; http://git-wip-us.apache.org/repos/asf/mesos/blob/3b1eeec8/src/tests/containerizer/ns_tests.cpp ---------------------------------------------------------------------- diff --git a/src/tests/containerizer/ns_tests.cpp b/src/tests/containerizer/ns_tests.cpp index 3a22938..ac3c9ff 100644 --- a/src/tests/containerizer/ns_tests.cpp +++ b/src/tests/containerizer/ns_tests.cpp @@ -56,32 +56,6 @@ namespace internal { namespace tests { -// Helper for cloneChild() which expects an int(void*). -static int cloneChildHelper(void* _func) -{ - const lambda::function<int()>* func = - static_cast<const lambda::function<int()>*> (_func); - - return (*func)(); -} - - -static pid_t cloneChild( - int flags, - const lambda::function<int()>& func) - -{ - // 8 MiB stack for child. - static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)]; - - return ::clone( - cloneChildHelper, - &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down. - flags | SIGCHLD, - (void*) &func); -} - - // Test that a child in different namespace(s) can setns back to the // root namespace. We must fork a child to test this because setns // doesn't support multi-threaded processes (which gtest is). @@ -121,7 +95,7 @@ TEST(NsTest, ROOT_setns) None(), None(), None(), - lambda::bind(&cloneChild, flags, lambda::_1)); + lambda::bind(&os::clone, lambda::_1, flags | SIGCHLD)); // Continue in parent. ASSERT_SOME(s); @@ -164,9 +138,7 @@ TEST(NsTest, ROOT_setnsMultipleThreads) } -// Use a different child function for clone because it requires -// int(*)(void*). -static int childGetns(void* arg) +static int childGetns() { // Sleep until killed. while (true) { sleep(1); } @@ -192,14 +164,7 @@ TEST(NsTest, ROOT_getns) Try<int> nstype = ns::nstype(ns); ASSERT_SOME(nstype); - // 8 MiB stack for child. - static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)]; - - pid_t pid = clone( - childGetns, - &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down. - SIGCHLD | nstype.get(), - NULL); + pid_t pid = os::clone(childGetns, SIGCHLD | nstype.get()); ASSERT_NE(-1, pid); @@ -224,7 +189,7 @@ TEST(NsTest, ROOT_getns) } -static int childDestroy(void* arg) +static int childDestroy() { // Fork a bunch of children. ::fork(); @@ -251,14 +216,7 @@ TEST(NsTest, ROOT_destroy) Try<int> nstype = ns::nstype("pid"); ASSERT_SOME(nstype); - // 8 MiB stack for child. - static unsigned long long stack[(8*1024*1024)/sizeof(unsigned long long)]; - - pid_t pid = clone( - childDestroy, - &stack[sizeof(stack)/sizeof(stack[0]) - 1], // Stack grows down. - SIGCHLD | nstype.get(), - NULL); + pid_t pid = os::clone(childDestroy, SIGCHLD | nstype.get()); ASSERT_NE(-1, pid);
