os,internal/poll: disassociate handle from IOCP in File.Fd

Go 1.25 will gain support for overlapped IO on handles passed to
os.NewFile thanks to CL 662236. It was previously not possible to add
an overlapped handle to the Go runtime's IO completion port (IOCP),
and now happens on the first call the an IO method.

This means that there is code that relies on the fact that File.Fd
returns a handle that can always be associated with a custom IOCP.
That wouldn't be the case anymore, as a handle can only be associated
with one IOCP at a time and it must be explicitly disassociated.

To fix this breaking change, File.Fd will disassociate the handle
from the Go runtime IOCP before returning it. It is then not necessary
to defer the association until the first IO method is called, which
was recently added in CL 661955 to support this same use case, but
in a more complex and unreliable way.

Updates #19098.

Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-race,gotip-windows-amd64-longtest,gotip-windows-arm64
Change-Id: Id8a7e04d35057047c61d1733bad5bf45494b2c28
Reviewed-on: https://go-review.googlesource.com/c/go/+/664455
Reviewed-by: Damien Neil <dneil@google.com>
Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
Reviewed-by: Michael Pratt <mpratt@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

3 files changed, 60 insertions, 15 deletions

diff --git a/src/os/file.go b/src/os/file.go
index c6e9167d2c..7a7f2c06be 100644
--- a/src/os/file.go
+++ b/src/os/file.go
@@ -714,8 +714,12 @@ func (f *File) SyscallConn() (syscall.RawConn, error) {
 // Do not close the returned descriptor; that could cause a later
 // close of f to close an unrelated descriptor.
 //
-// On Unix systems this will cause the [File.SetDeadline]
-// methods to stop working.
+// Fd's behavior differs on some platforms:
+//
+//   - On Unix and Windows, [File.SetDeadline] methods will stop working.
+//   - On Windows, the file descriptor will be disassociated from the
+//     Go runtime I/O completion port if there are no concurrent I/O
+//     operations on the file.
 //
 // For most uses prefer the f.SyscallConn method.
 func (f *File) Fd() uintptr {
diff --git a/src/os/file_windows.go b/src/os/file_windows.go
index 7b1db188b5..c97307371c 100644
--- a/src/os/file_windows.go
+++ b/src/os/file_windows.go
@@ -37,6 +37,11 @@ func (file *File) fd() uintptr {
 	if file == nil {
 		return uintptr(syscall.InvalidHandle)
 	}
+	// Try to disassociate the file from the runtime poller.
+	// File.Fd doesn't return an error, so we don't have a way to
+	// report it. We just ignore it. It's up to the caller to call
+	// it when there are no concurrent IO operations.
+	_ = file.pfd.DisassociateIOCP()
 	return uintptr(file.pfd.Sysfd)
 }
 
diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go
index d78080ccc4..89a61f0229 100644
--- a/src/os/os_windows_test.go
+++ b/src/os/os_windows_test.go
@@ -1985,19 +1985,7 @@ func TestPipeCanceled(t *testing.T) {
 }
 
 func iocpAssociateFile(f *os.File, iocp syscall.Handle) error {
-	sc, err := f.SyscallConn()
-	if err != nil {
-		return err
-	}
-	var syserr error
-	err = sc.Control(func(fd uintptr) {
-		if _, err = windows.CreateIoCompletionPort(syscall.Handle(fd), iocp, 0, 0); err != nil {
-			syserr = err
-		}
-	})
-	if err == nil {
-		err = syserr
-	}
+	_, err := windows.CreateIoCompletionPort(syscall.Handle(f.Fd()), iocp, 0, 0)
 	return err
 }
 
@@ -2075,6 +2063,54 @@ func TestFileAssociatedWithExternalIOCP(t *testing.T) {
 	}
 }
 
+func TestFileWriteFdRace(t *testing.T) {
+	t.Parallel()
+
+	f := newFileOverlapped(t, filepath.Join(t.TempDir(), "a"), true)
+
+	var wg sync.WaitGroup
+	wg.Add(2)
+
+	go func() {
+		defer wg.Done()
+		n, err := f.Write([]byte("hi"))
+		if err != nil {
+			// We look at error strings as the
+			// expected errors are OS-specific.
+			switch {
+			case errors.Is(err, windows.ERROR_INVALID_HANDLE):
+				// Ignore an expected error.
+			default:
+				// Unexpected error.
+				t.Error(err)
+			}
+			return
+		}
+		if n != 2 {
+			t.Errorf("wrote %d bytes, expected 2", n)
+			return
+		}
+	}()
+	go func() {
+		defer wg.Done()
+		f.Fd()
+	}()
+	wg.Wait()
+
+	iocp, err := windows.CreateIoCompletionPort(syscall.InvalidHandle, 0, 0, 0)
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer syscall.CloseHandle(iocp)
+	if err := iocpAssociateFile(f, iocp); err != nil {
+		t.Fatal(err)
+	}
+
+	if _, err := f.Write([]byte("hi")); err != nil {
+		t.Error(err)
+	}
+}
+
 func TestSplitPath(t *testing.T) {
 	t.Parallel()
 	for _, tt := range []struct{ path, wantDir, wantBase string }{