Windows: Replaced `_wopen()` with `CreateFileW()` in `os::open()`. Instead of using the CRT implementation of `_wopen()` for the `os::open()` API, we now use the Windows API `CreateFileW()`, mapping each of the Linux `open()` flags to their semantic equivalents. This will make implementing overlapped I/O possible, and is a step toward removing the use of integer file descriptors on Windows.
Note that instead of redefining the C flags like `O_RDONLY`, we just use them directly in our mapping logic, and set the used but unsupported flags to zero. This change uncovered several bugs such as incorrect access flags, and used-but-not-included headers. We currently ignore creation permissions as they will be handled in a broader project to map permissions to Windows correctly. Review: https://reviews.apache.org/r/66424 Project: http://git-wip-us.apache.org/repos/asf/mesos/repo Commit: http://git-wip-us.apache.org/repos/asf/mesos/commit/86bb9641 Tree: http://git-wip-us.apache.org/repos/asf/mesos/tree/86bb9641 Diff: http://git-wip-us.apache.org/repos/asf/mesos/diff/86bb9641 Branch: refs/heads/master Commit: 86bb96413cba3b2f349d21e08b30cac78d9f7f5a Parents: 8b7798f Author: Andrew Schwartzmeyer <[email protected]> Authored: Fri Mar 16 11:38:17 2018 -0700 Committer: Andrew Schwartzmeyer <[email protected]> Committed: Tue May 1 18:36:04 2018 -0700 ---------------------------------------------------------------------- 3rdparty/stout/include/stout/net.hpp | 1 + .../stout/include/stout/os/windows/fcntl.hpp | 10 -- .../stout/include/stout/os/windows/mktemp.hpp | 3 +- .../stout/include/stout/os/windows/open.hpp | 114 ++++++++++++++++--- 3rdparty/stout/include/stout/windows.hpp | 5 - 5 files changed, 100 insertions(+), 33 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/net.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/net.hpp b/3rdparty/stout/include/stout/net.hpp index d2992c0..52fa09b 100644 --- a/3rdparty/stout/include/stout/net.hpp +++ b/3rdparty/stout/include/stout/net.hpp @@ -56,6 +56,7 @@ #include <stout/try.hpp> #include <stout/os/int_fd.hpp> +#include <stout/os/close.hpp> #include <stout/os/open.hpp> #ifdef __WINDOWS__ http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/fcntl.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/fcntl.hpp b/3rdparty/stout/include/stout/os/windows/fcntl.hpp index bf8c38a..0e8fa8d 100644 --- a/3rdparty/stout/include/stout/os/windows/fcntl.hpp +++ b/3rdparty/stout/include/stout/os/windows/fcntl.hpp @@ -22,16 +22,6 @@ #include <stout/os/socket.hpp> #include <stout/os/windows/fd.hpp> -#define O_RDONLY _O_RDONLY -#define O_WRONLY _O_WRONLY -#define O_RDWR _O_RDWR -#define O_CREAT _O_CREAT -#define O_TRUNC _O_TRUNC -#define O_APPEND _O_APPEND -// NOTE: Windows does not support the semantics of close-on-exec. Instead, by -// default we set all handles to be non-inheritable. -#define O_CLOEXEC 0 - namespace os { inline Try<Nothing> cloexec(const WindowsFD& fd) http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/mktemp.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/mktemp.hpp b/3rdparty/stout/include/stout/os/windows/mktemp.hpp index 5c775c4..b4f5279 100644 --- a/3rdparty/stout/include/stout/os/windows/mktemp.hpp +++ b/3rdparty/stout/include/stout/os/windows/mktemp.hpp @@ -61,7 +61,8 @@ inline Try<std::string> mktemp( // attempt to match POSIX's specification of `mkstemp`. We use `_S_IREAD` and // `_S_IWRITE` here instead of the POSIX equivalents. On Windows the file is // is not present, we use `_O_CREAT` option when opening the file. - Try<int_fd> fd = os::open(temp_file, _O_CREAT, _S_IREAD | _S_IWRITE); + Try<int_fd> fd = + os::open(temp_file, O_RDWR | O_CREAT | O_EXCL, _S_IREAD | _S_IWRITE); if (fd.isError()) { return Error(fd.error()); } http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/os/windows/open.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/os/windows/open.hpp b/3rdparty/stout/include/stout/os/windows/open.hpp index 9598fdd..7bfaa28 100644 --- a/3rdparty/stout/include/stout/os/windows/open.hpp +++ b/3rdparty/stout/include/stout/os/windows/open.hpp @@ -16,38 +16,118 @@ #include <string> #include <stout/error.hpp> -#include <stout/nothing.hpp> #include <stout/try.hpp> -#include <stout/windows.hpp> // For `mode_t`. +#include <stout/windows.hpp> // For `mode_t`. -#include <stout/os/close.hpp> -#include <stout/os/fcntl.hpp> // For `oflag` values. #include <stout/os/int_fd.hpp> #include <stout/internal/windows/longpath.hpp> -#ifndef O_CLOEXEC -#error "missing O_CLOEXEC support on this platform" -// NOTE: On Windows, `fnctl.hpp` defines `O_CLOEXEC` to a no-op. -#endif +// TODO(andschwa): Windows does not support the Linux extension +// O_NONBLOCK, as asynchronous I/O is done through other mechanisms. +// Overlapped I/O will be implemented later. +constexpr int O_NONBLOCK = 0; + +// Windows does not support the Linux extension O_SYNC, as buffering +// is done differently. +// TODO(andschwa): This could be equivalent to +// `FILE_FLAG_WRITE_THROUGH`, but we don't seem to need it. +constexpr int O_SYNC = 0; + +// Windows does not support the Linux extension O_CLOEXEC. Instead, by +// default we set all handles to be non-inheritable. +constexpr int O_CLOEXEC = 0; namespace os { +// TODO(andschwa): Handle specified creation permissions in `mode_t mode`. See +// MESOS-3176. inline Try<int_fd> open(const std::string& path, int oflag, mode_t mode = 0) { std::wstring longpath = ::internal::windows::longpath(path); - // By default, Windows will perform "text translation" meaning that it will - // automatically write CR/LF instead of LF line feeds. To prevent this, and - // use the POSIX semantics, we open with `O_BINARY`. + + // Map the POSIX `oflag` access flags. + + // O_APPEND: Write only appends. // - // Also by default, we will mimic the Windows (non-CRT) APIs and make all - // opened handles non-inheritable. - int_fd fd = ::_wopen(longpath.data(), oflag | O_BINARY | O_NOINHERIT, mode); - if (fd < 0) { - return ErrnoError(); + // NOTE: We choose a `write` flag here because emulating `O_APPEND` + // requires granting the `FILE_APPEND_DATA` access right, but not + // the `FILE_WRITE_DATA` access right, which `GENERIC_WRITE` would + // otherwise grant. + const DWORD write = (oflag & O_APPEND) ? FILE_APPEND_DATA : GENERIC_WRITE; + + DWORD access; + switch (oflag & (O_RDONLY | O_WRONLY | O_RDWR)) { + case O_RDONLY: { + access = GENERIC_READ; + break; + } + case O_WRONLY: { + access = write; + break; + } + case O_RDWR: { + access = GENERIC_READ | write; + break; + } + default: { + return Error("Access mode not specified."); + } + } + + // Map the POSIX `oflag` creation flags. + DWORD create; + switch (oflag & (O_CREAT | O_EXCL | O_TRUNC)) { + case O_CREAT: { + // Create a new file or open an existing file. + create = OPEN_ALWAYS; + break; + } + case O_CREAT | O_EXCL: + case O_CREAT | O_EXCL | O_TRUNC: { + // Create a new file, but fail if it already exists. + // Ignore `O_TRUNC` with `O_CREAT | O_EXCL` + create = CREATE_NEW; + break; + } + case O_CREAT | O_TRUNC: { + // Truncate file if it already exists. + create = CREATE_ALWAYS; + break; + } + case O_EXCL: + case O_EXCL | O_TRUNC: { + return Error("`O_EXCL` is undefined without `O_CREAT`."); + } + case O_TRUNC: { + // Truncate file if it exists, otherwise fail. + create = TRUNCATE_EXISTING; + break; + } + default: { + // Open file if it exists, otherwise fail. + create = OPEN_EXISTING; + break; + } + } + + const HANDLE handle = ::CreateFileW( + longpath.data(), + access, + // Share all access so we don't lock the file. + FILE_SHARE_READ | FILE_SHARE_WRITE | FILE_SHARE_DELETE, + // Disable inheritance by default. + nullptr, + create, + FILE_ATTRIBUTE_NORMAL, + // No template file. + nullptr); + + if (handle == INVALID_HANDLE_VALUE) { + return WindowsError(); } - return fd; + return handle; } } // namespace os { http://git-wip-us.apache.org/repos/asf/mesos/blob/86bb9641/3rdparty/stout/include/stout/windows.hpp ---------------------------------------------------------------------- diff --git a/3rdparty/stout/include/stout/windows.hpp b/3rdparty/stout/include/stout/windows.hpp index 1bfcdf4..8056ad8 100644 --- a/3rdparty/stout/include/stout/windows.hpp +++ b/3rdparty/stout/include/stout/windows.hpp @@ -335,11 +335,6 @@ const mode_t S_ISUID = 0x08000000; // No-op. const mode_t S_ISGID = 0x04000000; // No-op. const mode_t S_ISVTX = 0x02000000; // No-op. - -// Flags not supported by Windows. -const mode_t O_SYNC = 0x00000000; // No-op. -const mode_t O_NONBLOCK = 0x00000000; // No-op. - // Even though SIGKILL doesn't exist on Windows, we define // it here, because Docker defines it. So, the docker // executor needs this signal value to properly kill containers.
