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());