You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by jp...@apache.org on 2017/11/15 20:29:30 UTC

[4/4] mesos git commit: Added a non-allocating variant of `os::clone`.

Added a non-allocating variant of `os::clone`.

Added a non-allocating variant of `os::clone` in the
`os::signal_safe` namespace. This encapsulates the lambda
trampoline, but requires the caller to bring their own stack
since we can't know when it is safe to implicitly allocate
a stack.

Now that we have `os::signal_safe::clone`, there is no need for
`os::clone` to take an optional `os::Stack` argument. We can
remove that and simplify the cleanup logic.

Review: https://reviews.apache.org/r/63675/
(cherry picked from commit ec91293091be64919ca0e7f0f535fc98da397bcf)


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

Branch: refs/heads/1.4.x
Commit: 12682b1894417cf5e1488ba293613004581759da
Parents: cf4f4c0
Author: James Peach <jp...@apache.org>
Authored: Wed Nov 15 07:58:53 2017 -0800
Committer: James Peach <jp...@apache.org>
Committed: Wed Nov 15 12:28:56 2017 -0800

----------------------------------------------------------------------
 3rdparty/stout/include/stout/os/linux.hpp | 63 ++++++++++++++++++--------
 1 file changed, 43 insertions(+), 20 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/mesos/blob/12682b18/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 68b4090..e4d9666 100644
--- a/3rdparty/stout/include/stout/os/linux.hpp
+++ b/3rdparty/stout/include/stout/os/linux.hpp
@@ -62,19 +62,29 @@ public:
   {
     Stack stack(size);
 
+    if (!stack.allocate()) {
+      return ErrnoError("Failed to allocate and align stack");
+    }
+
+    return stack;
+  }
+
+  bool allocate()
+  {
     // Allocate and align the memory to 16 bytes.
     // x86, x64, and AArch64/ARM64 all expect a 16 byte aligned stack.
     // ARM64/aarch64 enforces the alignment where x86/x64 does not.
     // Without this alignment Mesos will crash with a SIGBUS on ARM64/aarch64.
     int err = ::posix_memalign(
-                reinterpret_cast<void**>(&stack.address),
+                reinterpret_cast<void**>(&address),
                 os::pagesize(),
-                stack.size);
-    if (err) {
-      return ErrnoError("Failed to allocate and align stack");
+                size);
+
+    if (err != 0) {
+        errno = err;
     }
 
-    return stack;
+    return err == 0;
   }
 
   // Explicitly free the stack.
@@ -87,23 +97,36 @@ public:
   }
 
   // Stack grows down, return the first usable address.
-  char* start()
+  char* start() const
   {
     return address + size;
   }
 
-private:
   explicit Stack(size_t size_) : size(size_) {}
 
+private:
   size_t size;
   char* address;
 };
 
 
+namespace signal_safe {
+
+
 inline pid_t clone(
-    const lambda::function<int()>& func,
+    const Stack& stack,
     int flags,
-    Option<Stack> stack = None())
+    const lambda::function<int()>& func)
+{
+  return ::clone(childMain, stack.start(), flags, (void*) &func);
+}
+
+} // namespace signal_safe {
+
+
+inline pid_t clone(
+    const lambda::function<int()>& func,
+    int flags)
 {
   // Stack for the child.
   //
@@ -111,18 +134,16 @@ inline pid_t clone(
   // 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.
+  Stack stack(Stack::DEFAULT_SIZE);
 
-  bool cleanup = false;
-  if (stack.isNone()) {
-    Try<Stack> _stack = Stack::create(Stack::DEFAULT_SIZE);
-    if (_stack.isError()) {
-        return -1;
-    }
-    stack = _stack.get();
-    cleanup = true;
+  if (!stack.allocate()) {
+    // TODO(jpeach): In MESOS-8155, we will return an
+    // ErrnoError() here, but for now keep the interface
+    // compatible.
+    return -1;
   }
 
-  pid_t pid = ::clone(childMain, stack->start(), flags, (void*) &func);
+  pid_t pid = signal_safe::clone(stack, flags, func);
 
   // Given we allocated the stack ourselves, there are two
   // circumstances where we need to delete the allocated stack to
@@ -135,8 +156,10 @@ inline pid_t clone(
   //     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))) {
-    stack->deallocate();
+  //
+  // TODO(jpeach): In case (2) we will leak the stack memory.
+  if (pid < 0 || !(flags & CLONE_VM)) {
+    stack.deallocate();
   }
 
   return pid;