Windows: Fixed `os::read()` to use `ReadFile()`. This can eventually support overlapped I/O.
The Windows API `ReadFile()` returns an error if the pipe is broken, where `_read()` did not, but this is not an error for us as the data is still read correctly. So we ignore it. Review: https://reviews.apache.org/r/66431 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/bf585fa2 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/bf585fa2 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/bf585fa2 Branch: refs/heads/master Commit: bf585fa2b8a0b103dbfae31973d02bcfac127456 Parents: fc2e3e9 Author: Andrew Schwartzmeyer <[email protected]> Authored: Mon Mar 19 13:59:47 2018 -0700 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Tue May 1 18:36:04 2018 -0700 ---------------------------------------------------------------------- 3rdparty/stout/include/stout/os/read.hpp | 41 ++++++++++++++++---- .../stout/include/stout/os/windows/read.hpp | 24 +++++++++--- 2 files changed, 53 insertions(+), 12 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/bf585fa2/3rdparty/stout/include/stout/os/read.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/read.hpp b/3rdparty/stout/include/stout/os/read.hpp index 49878e4..98c6ad1 100644 --- a/3rdparty/stout/include/stout/os/read.hpp +++ b/3rdparty/stout/include/stout/os/read.hpp @@ -29,6 +29,7 @@ #include <stout/result.hpp> #include <stout/try.hpp> #ifdef __WINDOWS__ +#include <stout/stringify.hpp> #include <stout/windows.hpp> #endif // __WINDOWS__ @@ -36,6 +37,10 @@ #include <stout/os/socket.hpp> #ifdef __WINDOWS__ +#include <stout/internal/windows/longpath.hpp> +#endif // __WINDOWS__ + +#ifdef __WINDOWS__ #include <stout/os/windows/read.hpp> #else #include <stout/os/posix/read.hpp> @@ -56,17 +61,21 @@ inline Result<std::string> read(int_fd fd, size_t size) ssize_t length = os::read(fd, buffer + offset, size - offset); #ifdef __WINDOWS__ - int error = WSAGetLastError(); + // NOTE: There is no actual difference between `WSAGetLastError` and + // `GetLastError`, the former is an alias for the latter. As such, there is + // no difference between `WindowsError` and `WindowsSocketError`, so we can + // simply use the former here for both `HANDLE` and `SOCKET` types of + // `int_fd`. See MESOS-8764. + WindowsError error; #else - int error = errno; + ErrnoError error; #endif // __WINDOWS__ if (length < 0) { // TODO(bmahler): Handle a non-blocking fd? (EAGAIN, EWOULDBLOCK) - if (net::is_restartable_error(error)) { + if (net::is_restartable_error(error.code)) { continue; } - ErrnoError error; // Constructed before 'delete' to capture errno. delete[] buffer; return error; } else if (length == 0) { @@ -90,9 +99,9 @@ inline Result<std::string> read(int_fd fd, size_t size) } -// Returns the contents of the file. NOTE: getline is not available on Solaris -// or Windows, so we use STL. -#if defined(__sun) || defined(__WINDOWS__) +// Returns the contents of the file. +// NOTE: getline is not available on Solaris so we use STL. +#if defined(__sun) inline Try<std::string> read(const std::string& path) { std::ifstream file(path.c_str()); @@ -103,6 +112,24 @@ inline Try<std::string> read(const std::string& path) return std::string((std::istreambuf_iterator<char>(file)), (std::istreambuf_iterator<char>())); } +// NOTE: Windows needs Unicode long path support. +#elif defined(__WINDOWS__) +inline Try<std::string> read(const std::string& path) +{ + const std::wstring longpath = ::internal::windows::longpath(path); + // NOTE: The `wchar_t` constructor of `ifstream` is an MSVC + // extension. + // + // TODO(andschwa): This might need `io_base::binary` like other + // streams on Windows. + std::ifstream file(longpath.data()); + if (!file.is_open()) { + return Error("Failed to open file"); + } + + return std::string((std::istreambuf_iterator<char>(file)), + (std::istreambuf_iterator<char>())); +} #else inline Try<std::string> read(const std::string& path) { http://git-wip-us.apache.org/repos/asf/mesos/blob/bf585fa2/3rdparty/stout/include/stout/os/windows/read.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/read.hpp b/3rdparty/stout/include/stout/os/windows/read.hpp index b5b70ad..8f789de 100644 --- a/3rdparty/stout/include/stout/os/windows/read.hpp +++ b/3rdparty/stout/include/stout/os/windows/read.hpp @@ -13,11 +13,9 @@ #ifndef __STOUT_OS_WINDOWS_READ_HPP__ #define __STOUT_OS_WINDOWS_READ_HPP__ -#include <io.h> - #include <stout/result.hpp> #include <stout/unreachable.hpp> -#include <stout/windows.hpp> // For order-dependent networking headers. +#include <stout/windows.hpp> #include <stout/os/int_fd.hpp> #include <stout/os/socket.hpp> @@ -29,10 +27,26 @@ inline ssize_t read(const int_fd& fd, void* data, size_t size) CHECK_LE(size, UINT_MAX); switch (fd.type()) { - case WindowsFD::FD_CRT: - case WindowsFD::FD_HANDLE: { + // TODO(andschwa): Remove this when `FD_CRT` is removed, MESOS-8675. + case WindowsFD::FD_CRT: { return ::_read(fd.crt(), data, static_cast<unsigned int>(size)); } + case WindowsFD::FD_HANDLE: { + DWORD bytes; + // TODO(andschwa): Handle overlapped I/O. + const BOOL result = + ::ReadFile(fd, data, static_cast<DWORD>(size), &bytes, nullptr); + if (result == FALSE) { + // The pipe "breaks" when the other process closes its handle, but we + // still have the data and therefore do not want to return an error. + if (::GetLastError() != ERROR_BROKEN_PIPE) { + // Indicates an error, but we can't return a `WindowsError`. + return -1; + } + } + + return static_cast<ssize_t>(bytes); + } case WindowsFD::FD_SOCKET: { return ::recv(fd, (char*)data, static_cast<unsigned int>(size), 0); }
