From 28056a6d161417bd7b3aed8099f59f4ac164b351 Mon Sep 17 00:00:00 2001 From: Samuel Williams Date: Wed, 24 May 2023 09:15:20 +0900 Subject: [PATCH] Add support for pread/pwrite on windows. (#7827) --- .github/workflows/windows.yml | 2 +- include/ruby/win32.h | 18 +++-- spec/ruby/core/io/pread_spec.rb | 72 ++++++++++---------- spec/ruby/core/io/pwrite_spec.rb | 60 ++++++++--------- spec/ruby/optional/capi/thread_spec.rb | 3 +- win32/win32.c | 91 +++++++++++++++++++------- 6 files changed, 150 insertions(+), 96 deletions(-) diff --git a/.github/workflows/windows.yml b/.github/workflows/windows.yml index 75f026074f..79564e5834 100644 --- a/.github/workflows/windows.yml +++ b/.github/workflows/windows.yml @@ -130,7 +130,7 @@ jobs: - run: nmake - run: nmake test timeout-minutes: 5 - - run: nmake test-spec + - run: nmake test-spec MSPECOPT="-V -fspec" timeout-minutes: 10 - run: nmake test-all env: diff --git a/include/ruby/win32.h b/include/ruby/win32.h index 197eb8a802..50713e9d6c 100644 --- a/include/ruby/win32.h +++ b/include/ruby/win32.h @@ -147,8 +147,16 @@ typedef int clockid_t; #define open rb_w32_uopen #define close(h) rb_w32_close(h) #define fclose(f) rb_w32_fclose(f) -#define read(f, b, s) rb_w32_read(f, b, s) -#define write(f, b, s) rb_w32_write(f, b, s) + +#define read(f, b, s) rb_w32_read(f, b, s, NULL) +#define write(f, b, s) rb_w32_write(f, b, s, NULL) + +#define HAVE_PREAD +#define pread(f, b, s, o) rb_w32_pread(f, b, s, o) + +#define HAVE_PWRITE +#define pwrite(f, b, s, o) rb_w32_pwrite(f, b, s, o) + #define getpid() rb_w32_getpid() #undef HAVE_GETPPID #define HAVE_GETPPID 1 @@ -714,8 +722,10 @@ int rb_w32_wopen(const WCHAR *, int, ...); int rb_w32_close(int); int rb_w32_fclose(FILE*); int rb_w32_pipe(int[2]); -ssize_t rb_w32_read(int, void *, size_t); -ssize_t rb_w32_write(int, const void *, size_t); +ssize_t rb_w32_read(int, void *, size_t, rb_off_t *offset); +ssize_t rb_w32_write(int, const void *, size_t, rb_off_t *offset); +ssize_t rb_w32_pread(int, void *, size_t, rb_off_t offset); +ssize_t rb_w32_pwrite(int, const void *, size_t, rb_off_t offset); rb_off_t rb_w32_lseek(int, rb_off_t, int); int rb_w32_uutime(const char *, const struct utimbuf *); int rb_w32_uutimes(const char *, const struct timeval *); diff --git a/spec/ruby/core/io/pread_spec.rb b/spec/ruby/core/io/pread_spec.rb index 43071d6a31..ac51545fc9 100644 --- a/spec/ruby/core/io/pread_spec.rb +++ b/spec/ruby/core/io/pread_spec.rb @@ -1,50 +1,48 @@ # -*- encoding: utf-8 -*- require_relative '../../spec_helper' -platform_is_not :windows do - describe "IO#pread" do - before :each do - @fname = tmp("io_pread.txt") - @contents = "1234567890" - touch(@fname) { |f| f.write @contents } - @file = File.open(@fname, "r+") - end +describe "IO#pread" do + before :each do + @fname = tmp("io_pread.txt") + @contents = "1234567890" + touch(@fname) { |f| f.write @contents } + @file = File.open(@fname, "r+") + end - after :each do - @file.close - rm_r @fname - end + after :each do + @file.close + rm_r @fname + end - it "accepts a length, and an offset" do - @file.pread(4, 0).should == "1234" - @file.pread(3, 4).should == "567" - end + it "accepts a length, and an offset" do + @file.pread(4, 0).should == "1234" + @file.pread(3, 4).should == "567" + end - it "accepts a length, an offset, and an output buffer" do - buffer = "foo" - @file.pread(3, 4, buffer) - buffer.should == "567" - end + it "accepts a length, an offset, and an output buffer" do + buffer = "foo" + @file.pread(3, 4, buffer) + buffer.should == "567" + end - it "does not advance the file pointer" do - @file.pread(4, 0).should == "1234" - @file.read.should == "1234567890" - end + it "does not advance the file pointer" do + @file.pread(4, 0).should == "1234" + @file.read.should == "1234567890" + end - it "raises EOFError if end-of-file is reached" do - -> { @file.pread(1, 10) }.should raise_error(EOFError) - end + it "raises EOFError if end-of-file is reached" do + -> { @file.pread(1, 10) }.should raise_error(EOFError) + end - it "raises IOError when file is not open in read mode" do - File.open(@fname, "w") do |file| - -> { file.pread(1, 1) }.should raise_error(IOError) - end - end - - it "raises IOError when file is closed" do - file = File.open(@fname, "r+") - file.close + it "raises IOError when file is not open in read mode" do + File.open(@fname, "w") do |file| -> { file.pread(1, 1) }.should raise_error(IOError) end end + + it "raises IOError when file is closed" do + file = File.open(@fname, "r+") + file.close + -> { file.pread(1, 1) }.should raise_error(IOError) + end end diff --git a/spec/ruby/core/io/pwrite_spec.rb b/spec/ruby/core/io/pwrite_spec.rb index fe29d1e1f6..da7d910364 100644 --- a/spec/ruby/core/io/pwrite_spec.rb +++ b/spec/ruby/core/io/pwrite_spec.rb @@ -1,43 +1,41 @@ # -*- encoding: utf-8 -*- require_relative '../../spec_helper' -platform_is_not :windows do - describe "IO#pwrite" do - before :each do - @fname = tmp("io_pwrite.txt") - @file = File.open(@fname, "w+") - end +describe "IO#pwrite" do + before :each do + @fname = tmp("io_pwrite.txt") + @file = File.open(@fname, "w+") + end - after :each do - @file.close - rm_r @fname - end + after :each do + @file.close + rm_r @fname + end - it "returns the number of bytes written" do - @file.pwrite("foo", 0).should == 3 - end + it "returns the number of bytes written" do + @file.pwrite("foo", 0).should == 3 + end - it "accepts a string and an offset" do - @file.pwrite("foo", 2) - @file.pread(3, 2).should == "foo" - end + it "accepts a string and an offset" do + @file.pwrite("foo", 2) + @file.pread(3, 2).should == "foo" + end - it "does not advance the pointer in the file" do - @file.pwrite("bar", 3) - @file.write("foo") - @file.pread(6, 0).should == "foobar" - end + it "does not advance the pointer in the file" do + @file.pwrite("bar", 3) + @file.write("foo") + @file.pread(6, 0).should == "foobar" + end - it "raises IOError when file is not open in write mode" do - File.open(@fname, "r") do |file| - -> { file.pwrite("foo", 1) }.should raise_error(IOError) - end - end - - it "raises IOError when file is closed" do - file = File.open(@fname, "w+") - file.close + it "raises IOError when file is not open in write mode" do + File.open(@fname, "r") do |file| -> { file.pwrite("foo", 1) }.should raise_error(IOError) end end + + it "raises IOError when file is closed" do + file = File.open(@fname, "w+") + file.close + -> { file.pwrite("foo", 1) }.should raise_error(IOError) + end end diff --git a/spec/ruby/optional/capi/thread_spec.rb b/spec/ruby/optional/capi/thread_spec.rb index 5cb46bbb7c..953664f291 100644 --- a/spec/ruby/optional/capi/thread_spec.rb +++ b/spec/ruby/optional/capi/thread_spec.rb @@ -165,7 +165,8 @@ describe "C-API Thread function" do end end - platform_is_not :mingw do + # This test is disabled on Windows: https://bugs.ruby-lang.org/issues/16265 + platform_is_not :mingw, :windows do it "runs a C function with the global lock unlocked and unlocks IO with the generic RUBY_UBF_IO" do thr = Thread.new do @t.rb_thread_call_without_gvl_with_ubf_io diff --git a/win32/win32.c b/win32/win32.c index 8c13ba7e07..308867bb07 100644 --- a/win32/win32.c +++ b/win32/win32.c @@ -7181,21 +7181,43 @@ rb_w32_close(int fd) return 0; } -static int -setup_overlapped(OVERLAPPED *ol, int fd, int iswrite) -{ - memset(ol, 0, sizeof(*ol)); - if (!(_osfile(fd) & (FDEV | FPIPE))) { - LONG high = 0; - /* On mode:a, it can write only FILE_END. - * On mode:a+, though it can write only FILE_END, - * it can read from everywhere. - */ - DWORD method = ((_osfile(fd) & FAPPEND) && iswrite) ? FILE_END : FILE_CURRENT; - DWORD low = SetFilePointer((HANDLE)_osfhnd(fd), 0, &high, method); #ifndef INVALID_SET_FILE_POINTER #define INVALID_SET_FILE_POINTER ((DWORD)-1) #endif + +static int +setup_overlapped(OVERLAPPED *ol, int fd, int iswrite, rb_off_t *_offset) +{ + memset(ol, 0, sizeof(*ol)); + + // On mode:a, it can write only FILE_END. + // On mode:a+, though it can write only FILE_END, + // it can read from everywhere. + DWORD seek_method = ((_osfile(fd) & FAPPEND) && iswrite) ? FILE_END : FILE_CURRENT; + + if (_offset) { + // Explicit offset was provided (pread/pwrite) - use it: + uint64_t offset = *_offset; + ol->Offset = (uint32_t)(offset & 0xFFFFFFFFLL); + ol->OffsetHigh = (uint32_t)((offset & 0xFFFFFFFF00000000LL) >> 32); + + // Update _offset with the current offset: + LARGE_INTEGER seek_offset = {0}, current_offset = {0}; + if (!SetFilePointerEx((HANDLE)_osfhnd(fd), seek_offset, ¤t_offset, seek_method)) { + DWORD last_error = GetLastError(); + if (last_error != NO_ERROR) { + errno = map_errno(last_error); + return -1; + } + } + + // As we need to restore the current offset later, we save it here: + *_offset = current_offset.QuadPart; + } + else if (!(_osfile(fd) & (FDEV | FPIPE))) { + LONG high = 0; + DWORD low = SetFilePointer((HANDLE)_osfhnd(fd), 0, &high, seek_method); + if (low == INVALID_SET_FILE_POINTER) { DWORD err = GetLastError(); if (err != NO_ERROR) { @@ -7203,9 +7225,11 @@ setup_overlapped(OVERLAPPED *ol, int fd, int iswrite) return -1; } } + ol->Offset = low; ol->OffsetHigh = high; } + ol->hEvent = CreateEvent(NULL, TRUE, TRUE, NULL); if (!ol->hEvent) { errno = map_errno(GetLastError()); @@ -7215,11 +7239,22 @@ setup_overlapped(OVERLAPPED *ol, int fd, int iswrite) } static void -finish_overlapped(OVERLAPPED *ol, int fd, DWORD size) +finish_overlapped(OVERLAPPED *ol, int fd, DWORD size, rb_off_t *_offset) { CloseHandle(ol->hEvent); - if (!(_osfile(fd) & (FDEV | FPIPE))) { + if (_offset) { + // If we were doing a `pread`/`pwrite`, we need to restore the current that was saved in setup_overlapped: + DWORD seek_method = (_osfile(fd) & FAPPEND) ? FILE_END : FILE_BEGIN; + + LARGE_INTEGER seek_offset = {0}; + if (seek_method == FILE_BEGIN) { + seek_offset.QuadPart = *_offset; + } + + SetFilePointerEx((HANDLE)_osfhnd(fd), seek_offset, NULL, seek_method); + } + else if (!(_osfile(fd) & (FDEV | FPIPE))) { LONG high = ol->OffsetHigh; DWORD low = ol->Offset + size; if (low < ol->Offset) @@ -7231,7 +7266,7 @@ finish_overlapped(OVERLAPPED *ol, int fd, DWORD size) #undef read /* License: Ruby's */ ssize_t -rb_w32_read(int fd, void *buf, size_t size) +rb_w32_read(int fd, void *buf, size_t size, rb_off_t *offset) { SOCKET sock = TO_SOCKET(fd); DWORD read; @@ -7252,7 +7287,7 @@ rb_w32_read(int fd, void *buf, size_t size) return -1; } - if (_osfile(fd) & FTEXT) { + if (!offset && _osfile(fd) & FTEXT) { return _read(fd, buf, size); } @@ -7286,7 +7321,7 @@ rb_w32_read(int fd, void *buf, size_t size) len = size; size -= len; - if (setup_overlapped(&ol, fd, FALSE)) { + if (setup_overlapped(&ol, fd, FALSE, offset)) { rb_acrt_lowio_unlock_fh(fd); return -1; } @@ -7349,7 +7384,7 @@ rb_w32_read(int fd, void *buf, size_t size) errno = map_errno(err); } - finish_overlapped(&ol, fd, read); + finish_overlapped(&ol, fd, read, offset); ret += read; if (read >= len) { @@ -7370,7 +7405,7 @@ rb_w32_read(int fd, void *buf, size_t size) #undef write /* License: Ruby's */ ssize_t -rb_w32_write(int fd, const void *buf, size_t size) +rb_w32_write(int fd, const void *buf, size_t size, rb_off_t *offset) { SOCKET sock = TO_SOCKET(fd); DWORD written; @@ -7388,7 +7423,8 @@ rb_w32_write(int fd, const void *buf, size_t size) return -1; } - if ((_osfile(fd) & FTEXT) && + // If an offset is given, we can't use `_write`. + if (!offset && (_osfile(fd) & FTEXT) && (!(_osfile(fd) & FPIPE) || fd == fileno(stdout) || fd == fileno(stderr))) { ssize_t w = _write(fd, buf, size); if (w == (ssize_t)-1 && errno == EINVAL) { @@ -7410,7 +7446,8 @@ rb_w32_write(int fd, const void *buf, size_t size) size -= len; retry2: - if (setup_overlapped(&ol, fd, TRUE)) { + // Provide the requested offset. + if (setup_overlapped(&ol, fd, TRUE, offset)) { rb_acrt_lowio_unlock_fh(fd); return -1; } @@ -7449,7 +7486,7 @@ rb_w32_write(int fd, const void *buf, size_t size) } } - finish_overlapped(&ol, fd, written); + finish_overlapped(&ol, fd, written, offset); ret += written; if (written == len) { @@ -7473,6 +7510,16 @@ rb_w32_write(int fd, const void *buf, size_t size) return ret; } +ssize_t rb_w32_pread(int descriptor, void *base, size_t size, rb_off_t offset) +{ + return rb_w32_read(descriptor, base, size, &offset); +} + +ssize_t rb_w32_pwrite(int descriptor, const void *base, size_t size, rb_off_t offset) +{ + return rb_w32_write(descriptor, base, size, &offset); +} + /* License: Ruby's */ long rb_w32_write_console(uintptr_t strarg, int fd)