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>

6 files changed, 80 insertions, 68 deletions

diff --git a/src/internal/poll/fd_plan9.go b/src/internal/poll/fd_plan9.go
index 6db1a9ebb1..b65485200a 100644
--- a/src/internal/poll/fd_plan9.go
+++ b/src/internal/poll/fd_plan9.go
@@ -36,10 +36,6 @@ type FD struct {
 	isFile bool
 }
 
-func (fd *FD) initIO() error {
-	return nil
-}
-
 // We need this to close out a file descriptor when it is unlocked,
 // but the real implementation has to live in the net package because
 // it uses os.File's.
diff --git a/src/internal/poll/fd_poll_runtime.go b/src/internal/poll/fd_poll_runtime.go
index 2dd95a8bba..2aef11243a 100644
--- a/src/internal/poll/fd_poll_runtime.go
+++ b/src/internal/poll/fd_poll_runtime.go
@@ -156,7 +156,6 @@ func setDeadlineImpl(fd *FD, t time.Time, mode int) error {
 	}
 	defer fd.decref()
 
-	fd.initIO()
 	if fd.pd.runtimeCtx == 0 {
 		return ErrNoDeadline
 	}
diff --git a/src/internal/poll/fd_unix.go b/src/internal/poll/fd_unix.go
index 0888632d80..31e6e21120 100644
--- a/src/internal/poll/fd_unix.go
+++ b/src/internal/poll/fd_unix.go
@@ -47,10 +47,6 @@ type FD struct {
 	isFile bool
 }
 
-func (fd *FD) initIO() error {
-	return nil
-}
-
 // Init initializes the FD. The Sysfd field should already be set.
 // This can be called multiple times on a single FD.
 // The net argument is a network name from the net package (e.g., "tcp"),
diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go
index 469d078fa3..e846c2cd52 100644
--- a/src/internal/poll/fd_windows.go
+++ b/src/internal/poll/fd_windows.go
@@ -10,6 +10,7 @@ import (
 	"internal/syscall/windows"
 	"io"
 	"sync"
+	"sync/atomic"
 	"syscall"
 	"unicode/utf16"
 	"unicode/utf8"
@@ -98,6 +99,12 @@ func (o *operation) setEvent() {
 	o.o.HEvent = h | 1
 }
 
+func (o *operation) close() {
+	if o.o.HEvent != 0 {
+		syscall.CloseHandle(o.o.HEvent)
+	}
+}
+
 func (o *operation) overlapped() *syscall.Overlapped {
 	if o.fd.isBlocking {
 		// Don't return the overlapped object if the file handle
@@ -169,7 +176,7 @@ func waitIO(o *operation) error {
 		panic("can't wait on blocking operations")
 	}
 	fd := o.fd
-	if !fd.pd.pollable() {
+	if !fd.pollable() {
 		// The overlapped handle is not added to the runtime poller,
 		// the only way to wait for the IO to complete is block until
 		// the overlapped event is signaled.
@@ -190,7 +197,7 @@ func waitIO(o *operation) error {
 // cancelIO cancels the IO operation o and waits for it to complete.
 func cancelIO(o *operation) {
 	fd := o.fd
-	if !fd.pd.pollable() {
+	if !fd.pollable() {
 		return
 	}
 	// Cancel our request.
@@ -209,14 +216,13 @@ func cancelIO(o *operation) {
 // to avoid reusing the values from a previous call.
 func execIO(o *operation, submit func(o *operation) error) (int, error) {
 	fd := o.fd
-	fd.initIO()
 	// Notify runtime netpoll about starting IO.
 	err := fd.pd.prepare(int(o.mode), fd.isFile)
 	if err != nil {
 		return 0, err
 	}
 	// Start IO.
-	if !fd.isBlocking && o.o.HEvent == 0 && !fd.pd.pollable() {
+	if !fd.isBlocking && o.o.HEvent == 0 && !fd.pollable() {
 		// If the handle is opened for overlapped IO but we can't
 		// use the runtime poller, then we need to use an
 		// event to wait for the IO to complete.
@@ -244,10 +250,11 @@ func execIO(o *operation, submit func(o *operation) error) (int, error) {
 			err = windows.WSAGetOverlappedResult(fd.Sysfd, &o.o, &o.qty, false, &o.flags)
 		}
 	}
-	// ERROR_OPERATION_ABORTED may have been caused by us. In that case,
-	// map it to our own error. Don't do more than that, each submitted
-	// function may have its own meaning for each error.
-	if err == syscall.ERROR_OPERATION_ABORTED {
+	switch err {
+	case syscall.ERROR_OPERATION_ABORTED:
+		// ERROR_OPERATION_ABORTED may have been caused by us. In that case,
+		// map it to our own error. Don't do more than that, each submitted
+		// function may have its own meaning for each error.
 		if waitErr != nil {
 			// IO canceled by the poller while waiting for completion.
 			err = waitErr
@@ -257,6 +264,12 @@ func execIO(o *operation, submit func(o *operation) error) (int, error) {
 			// we assume it is interrupted by Close.
 			err = errClosing(fd.isFile)
 		}
+	case windows.ERROR_IO_INCOMPLETE:
+		// waitIO couldn't wait for the IO to complete.
+		if waitErr != nil {
+			// The wait error will be more informative.
+			err = waitErr
+		}
 	}
 	return int(o.qty), err
 }
@@ -314,9 +327,7 @@ type FD struct {
 	// Whether FILE_FLAG_OVERLAPPED was not set when opening the file.
 	isBlocking bool
 
-	// Initialization parameters.
-	initIOOnce sync.Once
-	initIOErr  error // only used in the net package
+	disassociated atomic.Bool
 }
 
 // setOffset sets the offset fields of the overlapped object
@@ -343,6 +354,12 @@ func (fd *FD) addOffset(off int) {
 	fd.setOffset(fd.offset + int64(off))
 }
 
+// pollable should be used instead of fd.pd.pollable(),
+// as it is aware of the disassociated state.
+func (fd *FD) pollable() bool {
+	return fd.pd.pollable() && !fd.disassociated.Load()
+}
+
 // fileKind describes the kind of file.
 type fileKind byte
 
@@ -353,35 +370,6 @@ const (
 	kindPipe
 )
 
-func (fd *FD) initIO() error {
-	if fd.isBlocking {
-		return nil
-	}
-	fd.initIOOnce.Do(func() {
-		if fd.closing() {
-			// Closing, nothing to do.
-			return
-		}
-		// The runtime poller will ignore I/O completion
-		// notifications not initiated by this package,
-		// so it is safe to add handles owned by the caller.
-		fd.initIOErr = fd.pd.init(fd)
-		if fd.initIOErr != nil {
-			return
-		}
-		fd.rop.runtimeCtx = fd.pd.runtimeCtx
-		fd.wop.runtimeCtx = fd.pd.runtimeCtx
-		if fd.kind != kindNet || socketCanUseSetFileCompletionNotificationModes {
-			// Non-socket handles can use SetFileCompletionNotificationModes without problems.
-			err := syscall.SetFileCompletionNotificationModes(fd.Sysfd,
-				syscall.FILE_SKIP_SET_EVENT_ON_HANDLE|syscall.FILE_SKIP_COMPLETION_PORT_ON_SUCCESS,
-			)
-			fd.skipSyncNotif = err == nil
-		}
-	})
-	return fd.initIOErr
-}
-
 // Init initializes the FD. The Sysfd field should already be set.
 // This can be called multiple times on a single FD.
 // The net argument is a network name from the net package (e.g., "tcp"),
@@ -411,20 +399,46 @@ func (fd *FD) Init(net string, pollable bool) error {
 	fd.rop.fd = fd
 	fd.wop.fd = fd
 
-	// A file handle (and its duplicated handles) can only be associated
-	// with one IOCP. A new association will fail if the handle is already
-	// associated. Defer the association until the first I/O operation so that
-	// overlapped handles passed in os.NewFile have a chance to be used
-	// with an external IOCP. This is common case, for example, when calling
-	// os.NewFile on a handle just to pass it to a exec.Command standard
-	// input/output/error. If the association fails, the I/O operations
-	// will be performed synchronously.
-	if fd.kind == kindNet {
-		// The net package is the only consumer that requires overlapped
-		// handles and that cares about handle IOCP association errors.
-		// We can should do the IOCP association here.
-		return fd.initIO()
+	// It is safe to add overlapped handles that also perform I/O
+	// outside of the runtime poller. The runtime poller will ignore
+	// I/O completion notifications not initiated by us.
+	err := fd.pd.init(fd)
+	if err != nil {
+		return err
+	}
+	fd.rop.runtimeCtx = fd.pd.runtimeCtx
+	fd.wop.runtimeCtx = fd.pd.runtimeCtx
+	if fd.kind != kindNet || socketCanUseSetFileCompletionNotificationModes {
+		// Non-socket handles can use SetFileCompletionNotificationModes without problems.
+		err := syscall.SetFileCompletionNotificationModes(fd.Sysfd,
+			syscall.FILE_SKIP_SET_EVENT_ON_HANDLE|syscall.FILE_SKIP_COMPLETION_PORT_ON_SUCCESS,
+		)
+		fd.skipSyncNotif = err == nil
+	}
+	return nil
+}
+
+// DisassociateIOCP disassociates the file handle from the IOCP.
+// The disassociate operation will not succeed if there is any
+// in-progress IO operation on the file handle.
+func (fd *FD) DisassociateIOCP() error {
+	if err := fd.incref(); err != nil {
+		return err
+	}
+	defer fd.decref()
+
+	if fd.isBlocking || !fd.pollable() {
+		// Nothing to disassociate.
+		return nil
 	}
+
+	info := windows.FILE_COMPLETION_INFORMATION{}
+	if err := windows.NtSetInformationFile(fd.Sysfd, &windows.IO_STATUS_BLOCK{}, unsafe.Pointer(&info), uint32(unsafe.Sizeof(info)), windows.FileReplaceCompletionInformation); err != nil {
+		return err
+	}
+	fd.disassociated.Store(true)
+	// Don't call fd.pd.close(), it would be too racy.
+	// There is no harm on leaving fd.pd open until Close is called.
 	return nil
 }
 
@@ -432,6 +446,8 @@ func (fd *FD) destroy() error {
 	if fd.Sysfd == syscall.InvalidHandle {
 		return syscall.EINVAL
 	}
+	fd.rop.close()
+	fd.wop.close()
 	// Poller may want to unregister fd in readiness notification mechanism,
 	// so this must be executed before fd.CloseFunc.
 	fd.pd.close()
@@ -454,12 +470,7 @@ func (fd *FD) Close() error {
 	if !fd.fdmu.increfAndClose() {
 		return errClosing(fd.isFile)
 	}
-	// There is a potential race between a concurrent call to fd.initIO,
-	// which calls fd.pd.init, and the call to fd.pd.evict below.
-	// This is solved by calling fd.initIO ourselves, which will
-	// block until the concurrent fd.initIO has completed. Note
-	// that fd.initIO is no-op if first called from here.
-	fd.initIO()
+
 	if fd.kind == kindPipe {
 		syscall.CancelIoEx(fd.Sysfd, nil)
 	}
diff --git a/src/internal/syscall/windows/syscall_windows.go b/src/internal/syscall/windows/syscall_windows.go
index b6859a5432..b6692166cc 100644
--- a/src/internal/syscall/windows/syscall_windows.go
+++ b/src/internal/syscall/windows/syscall_windows.go
@@ -32,6 +32,7 @@ func UTF16PtrToString(p *uint16) string {
 }
 
 const (
+	ERROR_INVALID_HANDLE         syscall.Errno = 6
 	ERROR_BAD_LENGTH             syscall.Errno = 24
 	ERROR_SHARING_VIOLATION      syscall.Errno = 32
 	ERROR_LOCK_VIOLATION         syscall.Errno = 33
@@ -39,6 +40,7 @@ const (
 	ERROR_CALL_NOT_IMPLEMENTED   syscall.Errno = 120
 	ERROR_INVALID_NAME           syscall.Errno = 123
 	ERROR_LOCK_FAILED            syscall.Errno = 167
+	ERROR_IO_INCOMPLETE          syscall.Errno = 996
 	ERROR_NO_TOKEN               syscall.Errno = 1008
 	ERROR_NO_UNICODE_TRANSLATION syscall.Errno = 1113
 	ERROR_CANT_ACCESS_FILE       syscall.Errno = 1920
diff --git a/src/internal/syscall/windows/types_windows.go b/src/internal/syscall/windows/types_windows.go
index 6c81754e1a..9f8f61f6d9 100644
--- a/src/internal/syscall/windows/types_windows.go
+++ b/src/internal/syscall/windows/types_windows.go
@@ -248,3 +248,11 @@ type FILE_LINK_INFORMATION struct {
 	FileNameLength  uint32
 	FileName        [syscall.MAX_PATH]uint16
 }
+
+const FileReplaceCompletionInformation = 61
+
+// https://learn.microsoft.com/en-us/windows-hardware/drivers/ddi/ntifs/ns-ntifs-_file_completion_information
+type FILE_COMPLETION_INFORMATION struct {
+	Port syscall.Handle
+	Key  uintptr
+}