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

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

commit c8a7d1f687294a457905e16042858173f8122cef
Author: Gilbert Song <songzihao1...@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
    (cherry picked from commit 3f49cf4351d200e4ba0ac7aa2cb69791b2786a23)
---
 src/slave/container_loggers/lib_logrotate.cpp | 66 ++++++++++++++-------------
 1 file changed, 34 insertions(+), 32 deletions(-)

diff --git a/src/slave/container_loggers/lib_logrotate.cpp 
b/src/slave/container_loggers/lib_logrotate.cpp
index bc13e6a..1bc8b61 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;
 
@@ -151,24 +153,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;
@@ -189,6 +183,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},
@@ -198,7 +198,9 @@ public:
         &outFlags,
         environment,
         None(),
-        parentHooks);
+        parentHooks,
+        childHooks,
+        whitelistOutFds);
 
     if (outProcess.isError()) {
       os::close(outfds.write.get());
@@ -207,26 +209,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.get().pid(), SIGKILL);
-      return Failure(ErrnoError("Failed to create pipe").message);
+      os::killtree(outProcess->pid(), SIGKILL);
+      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.get().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;
@@ -236,6 +230,12 @@ public:
     errFlags.logrotate_path = flags.logrotate_path;
     errFlags.user = user;
 
+    // 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},
@@ -245,7 +245,9 @@ public:
         &errFlags,
         environment,
         None(),
-        parentHooks);
+        parentHooks,
+        childHooks,
+        whitelistErrFds);
 
     if (errProcess.isError()) {
       os::close(outfds.write.get());

Reply via email to