[7/8] mesos git commit: Refactored 'os::clone()' to take an optional stack.

[be...@apache.org]

Refactored 'os::clone()' to take an optional stack.

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


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

Branch: refs/heads/master
Commit: dda5fdf3f34a334240e28bd7d7fb26a2741c7115
Parents: 6b5e99d
Author: Benjamin Hindman <be...@gmail.com>
Authored: Tue Aug 30 03:41:36 2016 -0700
Committer: Benjamin Hindman <be...@gmail.com>
Committed: Mon Sep 26 22:41:21 2016 -0700

----------------------------------------------------------------------
 3rdparty/stout/include/stout/os/linux.hpp | 57 ++++++++++++++++++--------
 1 file changed, 40 insertions(+), 17 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/dda5fdf3/3rdparty/stout/include/stout/os/linux.hpp
----------------------------------------------------------------------
diff --git a/3rdparty/stout/include/stout/os/linux.hpp b/3rdparty/stout/include/stout/os/linux.hpp
index fc44700..fd9b0ed 100644
--- a/3rdparty/stout/include/stout/os/linux.hpp
+++ b/3rdparty/stout/include/stout/os/linux.hpp
@@ -49,7 +49,19 @@ static int childMain(void* _func)
 }
 
 
-inline pid_t clone(const lambda::function<int()>& func, int flags)
+// Helper that captures information about a stack to be used when
+// invoking clone.
+struct Stack
+{
+  size_t size;
+  unsigned long long* address;
+};
+
+
+inline pid_t clone(
+    const lambda::function<int()>& func,
+    int flags,
+    Option<Stack> stack = None())
 {
   // Stack for the child.
   // - unsigned long long used for best alignment.
@@ -59,22 +71,33 @@ inline pid_t clone(const lambda::function<int()>& func, int flags)
   // 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)];
-
-  pid_t pid = ::clone(
-      childMain,
-      &stack[stackSize/sizeof(stack[0]) - 1],  // stack grows down.
-      flags,
-      (void*) &func);
-
-  // If CLONE_VM is not set, ::clone would create a process which runs in a
-  // separate copy of the memory space of the calling process. So we destroy the
-  // stack here to avoid memory leak. If CLONE_VM is set, ::clone would create a
-  // thread which runs in the same memory space with the calling process.
-  if (!(flags & CLONE_VM)) {
-    delete[] stack;
+  bool cleanup = false;
+  if (stack.isNone()) {
+    stack = Stack();
+    stack->size = 8 * 1024 * 1024;
+    stack->address =
+      new unsigned long long[stack->size/sizeof(unsigned long long)];
+    cleanup = true;
+  }
+
+  // Compute the address of the stack given that it grows down.
+  void* address = &stack->address[stack->size / sizeof(stack->address[0]) - 1];
+
+  pid_t pid = ::clone(childMain, address, flags, (void*) &func);
+
+  // Given we allocated the stack ourselves, there are two
+  // circumstances where we need to delete the allocated stack to
+  // avoid a memory leak:
+  //
+  // (1) Failed to clone.
+  //
+  // (2) CLONE_VM is not set implying ::clone will create a process
+  //     which runs in it's own copy of the memory space of the
+  //     calling process. If CLONE_VM is set ::clone will create a
+  //     thread which runs in the same memory space with the calling
+  //     process, in which case we don't want to call delete!
+  if (cleanup && (pid < 0 || !(flags & CLONE_VM))) {
+    delete[] stack->address;
   }
 
   return pid;