You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@mesos.apache.org by an...@apache.org on 2018/05/23 21:36:53 UTC
[09/10] mesos git commit: Windows: Fixed inheritance in
`Subprocess::PIPE()`.
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 <ak...@hotmail.com>
Authored: Wed May 23 14:01:49 2018 -0700
Committer: Andrew Schwartzmeyer <an...@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()};
});
}