You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by gi...@apache.org on 2018/08/20 23:17:28 UTC

[mesos] branch 1.7.x updated (9e9ece6 -> 3172b96)

This is an automated email from the ASF dual-hosted git repository.

gilbert pushed a change to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git.


    from 9e9ece6  Added MESOS-9081 to 1.7.0 CHANGELOG.
     new c33c98d  Updated the ::pipe() system calls to pipe2 in posix subprocess.
     new 3172b96  Updated the ::pipe() system calls to pipe2 in lib_logrotate.

The 2 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 3rdparty/libprocess/src/posix/subprocess.cpp  | 22 ++++-----
 src/slave/container_loggers/lib_logrotate.cpp | 64 ++++++++++++++-------------
 2 files changed, 45 insertions(+), 41 deletions(-)


[mesos] 02/02: Updated the ::pipe() system calls to pipe2 in lib_logrotate.

Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gilbert pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit 3172b9698c39121cebe6a45887ccbeef17dc4c14
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Thu Aug 16 11:05:41 2018 -0700

    Updated the ::pipe() system calls to pipe2 in lib_logrotate.
    
    Review: https://reviews.apache.org/r/68397
---
 src/slave/container_loggers/lib_logrotate.cpp | 64 ++++++++++++++-------------
 1 file changed, 33 insertions(+), 31 deletions(-)

diff --git a/src/slave/container_loggers/lib_logrotate.cpp b/src/slave/container_loggers/lib_logrotate.cpp
index fa71e07..6a2839e 100644
--- a/src/slave/container_loggers/lib_logrotate.cpp
+++ b/src/slave/container_loggers/lib_logrotate.cpp
@@ -43,6 +43,7 @@
 #include <stout/os/environment.hpp>
 #include <stout/os/fcntl.hpp>
 #include <stout/os/killtree.hpp>
+#include <stout/os/pipe.hpp>
 
 #ifdef __linux__
 #include "linux/systemd.hpp"
@@ -55,6 +56,7 @@
 using namespace mesos;
 using namespace process;
 
+using std::array;
 using std::map;
 using std::string;
 
@@ -150,24 +152,16 @@ public:
     // of the pipe and will be solely responsible for closing that end.
     // The ownership of the write-end will be passed to the caller
     // of this function.
-    int pipefd[2];
-    if (::pipe(pipefd) == -1) {
-      return Failure(ErrnoError("Failed to create pipe").message);
+    Try<array<int, 2>> pipefd = os::pipe();
+    if (pipefd.isError()) {
+      return Failure("Failed to create pipe: " + pipefd.error());
     }
 
     Subprocess::IO::InputFileDescriptors outfds;
-    outfds.read = pipefd[0];
-    outfds.write = pipefd[1];
-
-    // NOTE: We need to `cloexec` this FD so that it will be closed when
-    // the child subprocess is spawned and so that the FD will not be
-    // inherited by the second child for stderr.
-    Try<Nothing> cloexec = os::cloexec(outfds.write.get());
-    if (cloexec.isError()) {
-      os::close(outfds.read);
-      os::close(outfds.write.get());
-      return Failure("Failed to cloexec: " + cloexec.error());
-    }
+    outfds.read = pipefd->at(0);
+    outfds.write = pipefd->at(1);
+
+    const std::vector<int_fd> whitelistOutFds{pipefd->at(0), pipefd->at(1)};
 
     // Spawn a process to handle stdout.
     mesos::internal::logger::rotate::Flags outFlags;
@@ -190,6 +184,12 @@ public:
     }
 #endif // __linux__
 
+    // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
+    std::vector<Subprocess::ChildHook> childHooks;
+    foreach (int_fd fd, whitelistOutFds) {
+      childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+    }
+
     Try<Subprocess> outProcess = subprocess(
         path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
         {mesos::internal::logger::rotate::NAME},
@@ -199,7 +199,9 @@ public:
         &outFlags,
         environment,
         None(),
-        parentHooks);
+        parentHooks,
+        childHooks,
+        whitelistOutFds);
 
     if (outProcess.isError()) {
       os::close(outfds.write.get());
@@ -208,26 +210,18 @@ public:
 
     // NOTE: We manually construct a pipe here to properly express
     // ownership of the FDs.  See the NOTE above.
-    if (::pipe(pipefd) == -1) {
+    pipefd = os::pipe();
+    if (pipefd.isError()) {
       os::close(outfds.write.get());
       os::killtree(outProcess->pid(), SIGKILL);
-      return Failure(ErrnoError("Failed to create pipe").message);
+      return Failure("Failed to create pipe: " + pipefd.error());
     }
 
     Subprocess::IO::InputFileDescriptors errfds;
-    errfds.read = pipefd[0];
-    errfds.write = pipefd[1];
+    errfds.read = pipefd->at(0);
+    errfds.write = pipefd->at(1);
 
-    // NOTE: We need to `cloexec` this FD so that it will be closed when
-    // the child subprocess is spawned.
-    cloexec = os::cloexec(errfds.write.get());
-    if (cloexec.isError()) {
-      os::close(outfds.write.get());
-      os::close(errfds.read);
-      os::close(errfds.write.get());
-      os::killtree(outProcess->pid(), SIGKILL);
-      return Failure("Failed to cloexec: " + cloexec.error());
-    }
+    const std::vector<int_fd> whitelistErrFds{pipefd->at(0), pipefd->at(1)};
 
     // Spawn a process to handle stderr.
     mesos::internal::logger::rotate::Flags errFlags;
@@ -239,6 +233,12 @@ public:
       ? Option<string>(containerConfig.user())
       : Option<string>::none();
 
+    // TODO(gilbert): libprocess should take care of this, see MESOS-9164.
+    childHooks.clear();
+    foreach (int_fd fd, whitelistErrFds) {
+      childHooks.push_back(Subprocess::ChildHook::UNSET_CLOEXEC(fd));
+    }
+
     Try<Subprocess> errProcess = subprocess(
         path::join(flags.launcher_dir, mesos::internal::logger::rotate::NAME),
         {mesos::internal::logger::rotate::NAME},
@@ -248,7 +248,9 @@ public:
         &errFlags,
         environment,
         None(),
-        parentHooks);
+        parentHooks,
+        childHooks,
+        whitelistErrFds);
 
     if (errProcess.isError()) {
       os::close(outfds.write.get());


[mesos] 01/02: Updated the ::pipe() system calls to pipe2 in posix subprocess.

Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

gilbert pushed a commit to branch 1.7.x
in repository https://gitbox.apache.org/repos/asf/mesos.git

commit c33c98d8243d78f747af02e3fb3e7233c26da2fc
Author: Gilbert Song <so...@gmail.com>
AuthorDate: Thu Aug 16 11:03:52 2018 -0700

    Updated the ::pipe() system calls to pipe2 in posix subprocess.
    
    Review: https://reviews.apache.org/r/68396
---
 3rdparty/libprocess/src/posix/subprocess.cpp | 22 ++++++++++++----------
 1 file changed, 12 insertions(+), 10 deletions(-)

diff --git a/3rdparty/libprocess/src/posix/subprocess.cpp b/3rdparty/libprocess/src/posix/subprocess.cpp
index 01e3272..caba704 100644
--- a/3rdparty/libprocess/src/posix/subprocess.cpp
+++ b/3rdparty/libprocess/src/posix/subprocess.cpp
@@ -28,10 +28,12 @@
 #include <stout/foreach.hpp>
 #include <stout/option.hpp>
 #include <stout/os.hpp>
+#include <stout/os/pipe.hpp>
 #include <stout/os/strerror.hpp>
 #include <stout/strings.hpp>
 #include <stout/try.hpp>
 
+using std::array;
 using std::map;
 using std::string;
 using std::vector;
@@ -46,25 +48,25 @@ Subprocess::IO Subprocess::PIPE()
 {
   return Subprocess::IO(
       []() -> Try<InputFileDescriptors> {
-        int pipefd[2];
-        if (::pipe(pipefd) == -1) {
-          return ErrnoError("Failed to create pipe");
+        Try<array<int, 2>> pipefd = os::pipe();
+        if (pipefd.isError()) {
+          return Error(pipefd.error());
         }
 
         InputFileDescriptors fds;
-        fds.read = pipefd[0];
-        fds.write = pipefd[1];
+        fds.read = pipefd->at(0);
+        fds.write = pipefd->at(1);
         return fds;
       },
       []() -> Try<OutputFileDescriptors> {
-        int pipefd[2];
-        if (::pipe(pipefd) == -1) {
-          return ErrnoError("Failed to create pipe");
+        Try<array<int, 2>> pipefd = os::pipe();
+        if (pipefd.isError()) {
+          return Error(pipefd.error());
         }
 
         OutputFileDescriptors fds;
-        fds.read = pipefd[0];
-        fds.write = pipefd[1];
+        fds.read = pipefd->at(0);
+        fds.write = pipefd->at(1);
         return fds;
       });
 }