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/02 01:38:14 UTC

[14/31] mesos git commit: Windows: Fixed `os::read()` to use `ReadFile()`.

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 <an...@schwartzmeyer.com>
Authored: Mon Mar 19 13:59:47 2018 -0700
Committer: Andrew Schwartzmeyer <an...@schwartzmeyer.com>
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);
     }