Windows: Enabled creating overlapped pipes with `os::pipe`. `os::pipe` was using `CreatePipe`, which does not support overlapped I/O. Now, the implementation of `os::pipe` has been changed to use `CreateNamedPipe`, which supports overlapped I/O. The named pipe is created with an unique random name, so it is effectively anonymous.
Review: https://reviews.apache.org/r/66957/ Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/ebac13d8 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/ebac13d8 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/ebac13d8 Branch: refs/heads/master Commit: ebac13d83184363dd261b455b878ab613cb85587 Parents: bd4a800 Author: Akash Gupta <akash-gu...@hotmail.com> Authored: Wed May 23 14:01:26 2018 -0700 Committer: Andrew Schwartzmeyer <and...@schwartzmeyer.com> Committed: Wed May 23 14:07:46 2018 -0700 ---------------------------------------------------------------------- .../stout/include/stout/os/windows/pipe.hpp | 80 +++++++++++++++----- 1 file changed, 63 insertions(+), 17 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/ebac13d8/3rdparty/stout/include/stout/os/windows/pipe.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/pipe.hpp b/3rdparty/stout/include/stout/os/windows/pipe.hpp index a3574fd..7b94889 100644 --- a/3rdparty/stout/include/stout/os/windows/pipe.hpp +++ b/3rdparty/stout/include/stout/os/windows/pipe.hpp @@ -14,38 +14,84 @@ #define __STOUT_OS_WINDOWS_PIPE_HPP__ #include <array> +#include <string> +#include <stout/check.hpp> #include <stout/error.hpp> #include <stout/try.hpp> +#include <stout/uuid.hpp> +#include <stout/windows.hpp> // For `windows.h`. #include <stout/os/int_fd.hpp> namespace os { -// Create pipes for interprocess communication. Since the pipes cannot -// be used directly by Posix `read/write' functions they are wrapped -// in file descriptors, a process-local concept. -inline Try<std::array<int_fd, 2>> pipe() +// Returns an "anonymous" pipe that can be used for interprocess communication. +// Since anonymous pipes do not support overlapped IO, we emulate it with an +// uniquely named pipe. +// +// NOTE: Overlapped pipes passed to child processes behave weirdly, so we have +// the ability to create non overlapped pipe handles. +inline Try<std::array<int_fd, 2>> pipe( + bool read_overlapped = true, bool write_overlapped = true) { - // Create inheritable pipe, as described in MSDN[1]. - // - // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365782(v=vs.85).aspx - SECURITY_ATTRIBUTES securityAttr; - securityAttr.nLength = sizeof(SECURITY_ATTRIBUTES); - securityAttr.bInheritHandle = TRUE; - securityAttr.lpSecurityDescriptor = nullptr; + const DWORD read_flags = read_overlapped ? FILE_FLAG_OVERLAPPED : 0; + const DWORD write_flags = write_overlapped ? FILE_FLAG_OVERLAPPED : 0; + std::wstring name = + wide_stringify("\\\\.\\pipe\\mesos-pipe-" + id::UUID::random().toString()); - HANDLE read_handle; - HANDLE write_handle; + // The named pipe name must be at most 256 characters [1]. It doesn't say if + // it includes the null terminator, so we limit to 255 to be safe. Since the + // UUID name is fixed length, this is just a sanity check. + // + // [1] https://msdn.microsoft.com/en-us/library/windows/desktop/aa365150(v=vs.85).aspx // NOLINT(whitespace/line_length) + CHECK_LE(name.size(), 255); - const BOOL result = - ::CreatePipe(&read_handle, &write_handle, &securityAttr, 0); + // Create the named pipe. To avoid the time-of-check vs time-of-use attack, + // we have the `FILE_FLAG_FIRST_PIPE_INSTANCE` flag to fail if the pipe + // already exists. + // + // TODO(akagup): The buffer size is currently required, since we don't have + // IOCP yet. When testing IOCP, it should be 0, but after that, we can try + // restoring the buffer for an optimization. + const HANDLE read_handle = ::CreateNamedPipeW( + name.data(), + PIPE_ACCESS_INBOUND | FILE_FLAG_FIRST_PIPE_INSTANCE | read_flags, + PIPE_TYPE_BYTE | PIPE_WAIT | PIPE_REJECT_REMOTE_CLIENTS, + 1, // Max pipe instances. + 4096, // Inbound buffer size. + 4096, // Outbound buffer size. + 0, // Pipe timeout for connections. + nullptr); // Security attributes (not inheritable). - if (result == FALSE) { + if (read_handle == INVALID_HANDLE_VALUE) { return WindowsError(); } - return std::array<int_fd, 2>{int_fd(read_handle), int_fd(write_handle)}; + // To create a client named pipe, we use the generic `CreateFile` API. We + // don't use the other named pipe APIs, since they are used for different + // purposes. For example, `ConnectNamedPipe` is similar to a server calling + // `accept` for sockets and `CallNamedPipe` is used for message type pipes, + // but we want a byte stream pipe. + // + // See https://msdn.microsoft.com/en-us/library/windows/desktop/aa365598%28v=vs.85%29.aspx?f=255&MSPPError=-2147217396 // NOLINT(whitespace/line_length) + const HANDLE write_handle = ::CreateFileW( + name.data(), + GENERIC_WRITE, + 0, + nullptr, + OPEN_EXISTING, + FILE_ATTRIBUTE_NORMAL | write_flags, + nullptr); + + const WindowsError error; + if (write_handle == INVALID_HANDLE_VALUE) { + ::CloseHandle(read_handle); + return error; + } + + return std::array<int_fd, 2>{int_fd(read_handle, read_overlapped), + int_fd(write_handle, write_overlapped)}; } } // namespace os {