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 +} 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 }{