Windows: Fixed inheritance in `Subprocess::PIPE()`. `Subprocess::PIPE()` was creating inheritable pipes and files. On Windows, our policy is to make everything not inheritable and then have `CreateProcessW` set the the right pipes to be inheritable to avoid any handle leaks.
Review: https://reviews.apache.org/r/66958/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/07f5c000 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/07f5c000 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/07f5c000 Branch: refs/heads/master Commit: 07f5c00038d4ae22fbb72f8979f5f013b37780fd Parents: ebac13d Author: Akash Gupta <akash-gu...@hotmail.com> Authored: Wed May 23 14:01:49 2018 -0700 Committer: Andrew Schwartzmeyer <and...@schwartzmeyer.com> Committed: Wed May 23 14:07:47 2018 -0700 ---------------------------------------------------------------------- 3rdparty/libprocess/src/subprocess_windows.cpp | 47 +++++---------------- 1 file changed, 10 insertions(+), 37 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/07f5c000/3rdparty/libprocess/src/subprocess_windows.cpp ---------------------------------------------------------------------- diff --git a/3rdparty/libprocess/src/subprocess_windows.cpp b/3rdparty/libprocess/src/subprocess_windows.cpp index a1e6425..c497a03 100644 --- a/3rdparty/libprocess/src/subprocess_windows.cpp +++ b/3rdparty/libprocess/src/subprocess_windows.cpp @@ -36,8 +36,6 @@ #include <stout/os/pipe.hpp> #include <stout/os/strerror.hpp> -#include <stout/internal/windows/inherit.hpp> - using std::array; using std::string; @@ -46,45 +44,32 @@ namespace process { using InputFileDescriptors = Subprocess::IO::InputFileDescriptors; using OutputFileDescriptors = Subprocess::IO::OutputFileDescriptors; -// Opens an inheritable pipe[1] represented as a pair of file handles. On -// success, the first handle returned receives the 'read' handle of the pipe, -// while the second receives the 'write' handle. The pipe handles can then be -// passed to a child process, as exemplified in [2]. +// Opens a pair of file handles. On success, the first handle returned receives +// the 'read' handle of the pipe, while the second receives the 'write' handle. +// The pipe handles can then be passed to a child process, as shown in [1]. // -// [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa379560(v=vs.85).aspx -// [2] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682499(v=vs.85).aspx +// [1] https://msdn.microsoft.com/en-us/library/windows/desktop/ms682499(v=vs.85).aspx Subprocess::IO Subprocess::PIPE() { return Subprocess::IO( []() -> Try<InputFileDescriptors> { - const Try<array<int_fd, 2>> pipefd = os::pipe(); + // Create STDIN pipe and set the 'read' component to be not overlapped, + // because we're sending it to the child process. + const Try<array<int_fd, 2>> pipefd = os::pipe(false, true); if (pipefd.isError()) { return Error(pipefd.error()); } - // Create STDIN pipe and set the 'write' component to not be - // inheritable. - const Try<Nothing> inherit = - ::internal::windows::set_inherit(pipefd.get()[1], false); - if (inherit.isError()) { - return Error(inherit.error()); - } - return InputFileDescriptors{pipefd.get()[0], pipefd.get()[1]}; }, []() -> Try<OutputFileDescriptors> { - const Try<array<int_fd, 2>> pipefd = os::pipe(); + // Create STDOUT pipe and set the 'write' component to be not + // overlapped, because we're sending it to the child process. + const Try<array<int_fd, 2>> pipefd = os::pipe(true, false); if (pipefd.isError()) { return Error(pipefd.error()); } - // Create OUT pipe and set the 'read' component to not be inheritable. - const Try<Nothing> inherit = - ::internal::windows::set_inherit(pipefd.get()[0], false); - if (inherit.isError()) { - return Error(inherit.error()); - } - return OutputFileDescriptors{pipefd.get()[0], pipefd.get()[1]}; }); } @@ -100,12 +85,6 @@ Subprocess::IO Subprocess::PATH(const string& path) return Error(open.error()); } - const Try<Nothing> inherit = - ::internal::windows::set_inherit(open.get(), true); - if (inherit.isError()) { - return Error(inherit.error()); - } - return InputFileDescriptors{open.get(), None()}; }, [path]() -> Try<OutputFileDescriptors> { @@ -115,12 +94,6 @@ Subprocess::IO Subprocess::PATH(const string& path) return Error(open.error()); } - const Try<Nothing> inherit = - ::internal::windows::set_inherit(open.get(), true); - if (inherit.isError()) { - return Error(inherit.error()); - } - return OutputFileDescriptors{None(), open.get()}; }); }
