From 437d2362ce8ad3e10631aaf90cb4d8c8126d1bac Mon Sep 17 00:00:00 2001 From: qmuntal Date: Wed, 26 Nov 2025 10:25:16 +0100 Subject: [PATCH] os,internal/poll: don't call IsNonblock for consoles and Stdin windows.IsNonblock can block for synchronous handles that have an outstanding I/O operation. Console handles are always synchronous, so we should not call IsNonblock for them. Stdin is often a pipe, and almost always a synchronous handle, so we should not call IsNonblock for it either. This avoids potential deadlocks during os package initialization, which calls NewFile(syscall.Stdin). Fixes #75949 Updates #76391 Cq-Include-Trybots: luci.golang.try:gotip-windows-amd64-longtest,gotip-windows-amd64-race,gotip-windows-arm64 Change-Id: I1603932b0a99823019aa0cad960f94cee9996505 Reviewed-on: https://go-review.googlesource.com/c/go/+/724640 LUCI-TryBot-Result: Go LUCI Reviewed-by: Damien Neil Auto-Submit: Damien Neil Reviewed-by: Cherry Mui --- src/internal/poll/fd_windows.go | 4 +++ src/os/file_windows.go | 61 +++++++++++++++++++++++++++------ src/os/os_windows_test.go | 39 +++++++++++++++++++++ src/os/removeall_windows.go | 2 +- src/os/root_windows.go | 2 +- 5 files changed, 95 insertions(+), 13 deletions(-) diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go index 6443f6eb30..edad656350 100644 --- a/src/internal/poll/fd_windows.go +++ b/src/internal/poll/fd_windows.go @@ -451,6 +451,10 @@ func (fd *FD) Init(net string, pollable bool) error { fd.isFile = fd.kind != kindNet fd.isBlocking = !pollable + if !pollable { + return nil + } + // 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. diff --git a/src/os/file_windows.go b/src/os/file_windows.go index 83f5fde7f9..8f0827a23d 100644 --- a/src/os/file_windows.go +++ b/src/os/file_windows.go @@ -44,20 +44,60 @@ func (file *File) fd() uintptr { return uintptr(file.pfd.Sysfd) } +// newFileKind describes the kind of file to newFile. +type newFileKind int + +const ( + // kindNewFile means that the descriptor was passed to us via NewFile. + kindNewFile newFileKind = iota + // kindOpenFile means that the descriptor was opened using + // Open, Create, or OpenFile. + kindOpenFile + // kindPipe means that the descriptor was opened using Pipe. + kindPipe + // kindSock means that the descriptor is a network file descriptor + // that was created from net package and was opened using net_newUnixFile. + kindSock + // kindConsole means that the descriptor is a console handle. + kindConsole +) + // newFile returns a new File with the given file handle and name. // Unlike NewFile, it does not check that h is syscall.InvalidHandle. // If nonBlocking is true, it tries to add the file to the runtime poller. -func newFile(h syscall.Handle, name string, kind string, nonBlocking bool) *File { - if kind == "file" { +func newFile(h syscall.Handle, name string, kind newFileKind, nonBlocking bool) *File { + typ := "file" + switch kind { + case kindNewFile, kindOpenFile: t, err := syscall.GetFileType(h) if err != nil || t == syscall.FILE_TYPE_CHAR { var m uint32 if syscall.GetConsoleMode(h, &m) == nil { - kind = "console" + typ = "console" + // Console handles are always blocking. + break } } else if t == syscall.FILE_TYPE_PIPE { - kind = "pipe" + typ = "pipe" } + // NewFile doesn't know if a handle is blocking or non-blocking, + // so we try to detect that here. This call may block/ if the handle + // is blocking and there is an outstanding I/O operation. + // + // Avoid doing this for Stdin, which is almost always blocking and might + // be in use by other process when the "os" package is initializing. + // See go.dev/issue/75949 and go.dev/issue/76391. + if kind == kindNewFile && h != syscall.Stdin { + nonBlocking, _ = windows.IsNonblock(h) + } + case kindPipe: + typ = "pipe" + case kindSock: + typ = "file+net" + case kindConsole: + typ = "console" + default: + panic("newFile with unknown kind") } f := &File{&file{ @@ -72,13 +112,13 @@ func newFile(h syscall.Handle, name string, kind string, nonBlocking bool) *File // Ignore initialization errors. // Assume any problems will show up in later I/O. - f.pfd.Init(kind, nonBlocking) + f.pfd.Init(typ, nonBlocking) return f } // newConsoleFile creates new File that will be used as console. func newConsoleFile(h syscall.Handle, name string) *File { - return newFile(h, name, "console", false) + return newFile(h, name, kindConsole, false) } // newFileFromNewFile is called by [NewFile]. @@ -87,8 +127,7 @@ func newFileFromNewFile(fd uintptr, name string) *File { if h == syscall.InvalidHandle { return nil } - nonBlocking, _ := windows.IsNonblock(syscall.Handle(fd)) - return newFile(h, name, "file", nonBlocking) + return newFile(h, name, kindNewFile, false) } // net_newWindowsFile is a hidden entry point called by net.conn.File. @@ -100,7 +139,7 @@ func net_newWindowsFile(h syscall.Handle, name string) *File { if h == syscall.InvalidHandle { panic("invalid FD") } - return newFile(h, name, "file+net", true) + return newFile(h, name, kindSock, true) } func epipecheck(file *File, e error) { @@ -121,7 +160,7 @@ func openFileNolog(name string, flag int, perm FileMode) (*File, error) { return nil, &PathError{Op: "open", Path: name, Err: err} } nonblocking := flag&windows.O_FILE_FLAG_OVERLAPPED != 0 - return newFile(r, name, "file", nonblocking), nil + return newFile(r, name, kindOpenFile, nonblocking), nil } func openDirNolog(name string) (*File, error) { @@ -235,7 +274,7 @@ func Pipe() (r *File, w *File, err error) { return nil, nil, NewSyscallError("pipe", e) } // syscall.Pipe always returns a non-blocking handle. - return newFile(p[0], "|0", "pipe", false), newFile(p[1], "|1", "pipe", false), nil + return newFile(p[0], "|0", kindPipe, false), newFile(p[1], "|1", kindPipe, false), nil } var useGetTempPath2 = sync.OnceValue(func() bool { diff --git a/src/os/os_windows_test.go b/src/os/os_windows_test.go index 3e7bddc791..d549608b48 100644 --- a/src/os/os_windows_test.go +++ b/src/os/os_windows_test.go @@ -2288,3 +2288,42 @@ func TestOpenFileTruncateNamedPipe(t *testing.T) { } f.Close() } + +func TestNewFileStdinBlocked(t *testing.T) { + // See https://go.dev/issue/75949. + t.Parallel() + + // Use a subprocess to test that os.NewFile on a blocked stdin works. + // Can't do it in the same process because os.NewFile would close + // stdin for the whole test process once the test ends. + if os.Getenv("GO_WANT_HELPER_PROCESS") == "1" { + // In the child process, just exit. + // If we get here, the os package successfully initialized. + os.Exit(0) + } + name := pipeName() + stdin := newBytePipe(t, name, false) + file := newFileOverlapped(t, name, false) + + var wg sync.WaitGroup + wg.Go(func() { + // Block stdin on a read. + if _, err := stdin.Read(make([]byte, 1)); err != nil { + t.Error(err) + } + }) + + time.Sleep(100 * time.Millisecond) // Give time for the read to start. + cmd := testenv.CommandContext(t, t.Context(), testenv.Executable(t), fmt.Sprintf("-test.run=^%s$", t.Name())) + cmd.Env = cmd.Environ() + cmd.Env = append(cmd.Env, "GO_WANT_HELPER_PROCESS=1") + cmd.Stdin = stdin + if err := cmd.Run(); err != nil { + t.Fatal(err) + } + // Unblock the read to let the goroutine exit. + if _, err := file.Write(make([]byte, 1)); err != nil { + t.Fatal(err) + } + wg.Wait() // Don't leave goroutines behind. +} diff --git a/src/os/removeall_windows.go b/src/os/removeall_windows.go index 5cbc5fb036..6a1be1cca7 100644 --- a/src/os/removeall_windows.go +++ b/src/os/removeall_windows.go @@ -13,5 +13,5 @@ func isErrNoFollow(err error) bool { } func newDirFile(fd syscall.Handle, name string) (*File, error) { - return newFile(fd, name, "file", false), nil + return newFile(fd, name, kindOpenFile, false), nil } diff --git a/src/os/root_windows.go b/src/os/root_windows.go index 033a119862..381e33fc0e 100644 --- a/src/os/root_windows.go +++ b/src/os/root_windows.go @@ -135,7 +135,7 @@ func rootOpenFileNolog(root *Root, name string, flag int, perm FileMode) (*File, return nil, &PathError{Op: "openat", Path: name, Err: err} } // openat always returns a non-blocking handle. - return newFile(fd, joinPath(root.Name(), name), "file", false), nil + return newFile(fd, joinPath(root.Name(), name), kindOpenFile, false), nil } func openat(dirfd syscall.Handle, name string, flag uint64, perm FileMode) (syscall.Handle, error) { -- 2.52.0