internal/poll: don't call Seek for overlapped Windows handles

Overlapped handles don't have the file pointer updated when performing
I/O operations, so there is no need to call FD.Seek to reset the file
pointer.

Also, some overlapped file handles don't support seeking. See #74951.

Fixes #74951.

Change-Id: I0edd53beed7d3862730f3b2ed5fe9ba490e66c06
Reviewed-on: https://go-review.googlesource.com/c/go/+/697295
Reviewed-by: Damien Neil <dneil@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Dmitri Shuralyov <dmitshur@google.com>

diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go
index 52571d5fe0e1bee8e00602dd48a50719922abdd4..ebb2abcbf81a88779c2b264310f0e00b2d058ee5 100644 (file)
--- a/src/internal/poll/fd_windows.go
+++ b/src/internal/poll/fd_windows.go
@@ -622,12 +622,22 @@ func (fd *FD) Pread(b []byte, off int64) (int, error) {
 
        fd.l.Lock()
        defer fd.l.Unlock()
-       curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent)
-       if err != nil {
-               return 0, err
+       if fd.isBlocking {
+               curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent)
+               if err != nil {
+                       return 0, err
+               }
+               defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart)
+               defer fd.setOffset(curoffset)
+       } else {
+               // Overlapped handles don't have the file pointer updated
+               // when performing I/O operations, so there is no need to
+               // call Seek to reset the file pointer.
+               // Also, some overlapped file handles don't support seeking.
+               // See https://go.dev/issues/74951.
+               curoffset := fd.offset
+               defer fd.setOffset(curoffset)
        }
-       defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart)
-       defer fd.setOffset(curoffset)
        o := &fd.rop
        o.InitBuf(b)
        fd.setOffset(off)
@@ -847,12 +857,22 @@ func (fd *FD) Pwrite(buf []byte, off int64) (int, error) {
 
        fd.l.Lock()
        defer fd.l.Unlock()
-       curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent)
-       if err != nil {
-               return 0, err
+       if fd.isBlocking {
+               curoffset, err := syscall.Seek(fd.Sysfd, 0, io.SeekCurrent)
+               if err != nil {
+                       return 0, err
+               }
+               defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart)
+               defer fd.setOffset(curoffset)
+       } else {
+               // Overlapped handles don't have the file pointer updated
+               // when performing I/O operations, so there is no need to
+               // call Seek to reset the file pointer.
+               // Also, some overlapped file handles don't support seeking.
+               // See https://go.dev/issues/74951.
+               curoffset := fd.offset
+               defer fd.setOffset(curoffset)
        }
-       defer syscall.Seek(fd.Sysfd, curoffset, io.SeekStart)
-       defer fd.setOffset(curoffset)
 
        var ntotal int
        for {

diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go
index 3cdaad32c7f30a8fda5026e1ed64cbb00eb77341..7c5e8ac4a410d877b912dcb9146c275ce113f738 100644 (file)
--- a/src/os/os_windows_test.go
+++ b/src/os/os_windows_test.go
@@ -1883,6 +1883,34 @@ func TestFileOverlappedSeek(t *testing.T) {
        }
 }
 
+func TestFileOverlappedReadAtVolume(t *testing.T) {
+       // Test that we can use File.ReadAt with an overlapped volume handle.
+       // See https://go.dev/issues/74951.
+       t.Parallel()
+       name := `\\.\` + filepath.VolumeName(t.TempDir())
+       namep, err := syscall.UTF16PtrFromString(name)
+       if err != nil {
+               t.Fatal(err)
+       }
+       h, err := syscall.CreateFile(namep,
+               syscall.GENERIC_READ|syscall.GENERIC_WRITE,
+               syscall.FILE_SHARE_WRITE|syscall.FILE_SHARE_READ,
+               nil, syscall.OPEN_ALWAYS, syscall.FILE_FLAG_OVERLAPPED, 0)
+       if err != nil {
+               if errors.Is(err, syscall.ERROR_ACCESS_DENIED) {
+                       t.Skip("skipping test: access denied")
+               }
+               t.Fatal(err)
+       }
+       f := os.NewFile(uintptr(h), name)
+       defer f.Close()
+
+       var buf [0]byte
+       if _, err := f.ReadAt(buf[:], 0); err != nil {
+               t.Fatal(err)
+       }
+}
+
 func TestPipe(t *testing.T) {
        t.Parallel()
        r, w, err := os.Pipe()