Add support for pread/pwrite on windows. (#7827)

Merged-By: ioquatix <samuel@codeotaku.com>

.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:

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_read(int, void *, size_t, rb_off_t *offset);
-ssize_t rb_w32_write(int, const void *, size_t);
+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 *);

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
-  describe "IO#pread" do
+  before :each do
-    before :each do
+    @fname = tmp("io_pread.txt")
-      @fname = tmp("io_pread.txt")
+    @contents = "1234567890"
-      @contents = "1234567890"
+    touch(@fname) { |f| f.write @contents }
-      touch(@fname) { |f| f.write @contents }
+    @file = File.open(@fname, "r+")
-      @file = File.open(@fname, "r+")
+  end
-    end
 
-    after :each do
+  after :each do
-      @file.close
+    @file.close
-      rm_r @fname
+    rm_r @fname
-    end
+  end
 
-    it "accepts a length, and an offset" do
+  it "accepts a length, and an offset" do
-      @file.pread(4, 0).should == "1234"
+    @file.pread(4, 0).should == "1234"
-      @file.pread(3, 4).should == "567"
+    @file.pread(3, 4).should == "567"
-    end
+  end
 
-    it "accepts a length, an offset, and an output buffer" do
+  it "accepts a length, an offset, and an output buffer" do
-      buffer = "foo"
+    buffer = "foo"
-      @file.pread(3, 4, buffer)
+    @file.pread(3, 4, buffer)
-      buffer.should == "567"
+    buffer.should == "567"
-    end
+  end
 
-    it "does not advance the file pointer" do
+  it "does not advance the file pointer" do
-      @file.pread(4, 0).should == "1234"
+    @file.pread(4, 0).should == "1234"
-      @file.read.should == "1234567890"
+    @file.read.should == "1234567890"
-    end
+  end
 
-    it "raises EOFError if end-of-file is reached" do
+  it "raises EOFError if end-of-file is reached" do
-      -> { @file.pread(1, 10) }.should raise_error(EOFError)
+    -> { @file.pread(1, 10) }.should raise_error(EOFError)
-    end
+  end
 
-    it "raises IOError when file is not open in read mode" do
+  it "raises IOError when file is not open in read mode" do
-      File.open(@fname, "w") do |file|
+    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
+
+  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

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
-  describe "IO#pwrite" do
+  before :each do
-    before :each do
+    @fname = tmp("io_pwrite.txt")
-      @fname = tmp("io_pwrite.txt")
+    @file = File.open(@fname, "w+")
-      @file = File.open(@fname, "w+")
+  end
-    end
 
-    after :each do
+  after :each do
-      @file.close
+    @file.close
-      rm_r @fname
+    rm_r @fname
-    end
+  end
 
-    it "returns the number of bytes written" do
+  it "returns the number of bytes written" do
-      @file.pwrite("foo", 0).should == 3
+    @file.pwrite("foo", 0).should == 3
-    end
+  end
 
-    it "accepts a string and an offset"  do
+  it "accepts a string and an offset"  do
-      @file.pwrite("foo", 2)
+    @file.pwrite("foo", 2)
-      @file.pread(3, 2).should == "foo"
+    @file.pread(3, 2).should == "foo"
-    end
+  end
 
-    it "does not advance the pointer in the file" do
+  it "does not advance the pointer in the file" do
-      @file.pwrite("bar", 3)
+    @file.pwrite("bar", 3)
-      @file.write("foo")
+    @file.write("foo")
-      @file.pread(6, 0).should == "foobar"
+    @file.pread(6, 0).should == "foobar"
-    end
+  end
 
-    it "raises IOError when file is not open in write mode" do
+  it "raises IOError when file is not open in write mode" do
-      File.open(@fname, "r") do |file|
+    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
+
+  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

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

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, &current_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)