From: qmuntal Date: Wed, 26 Nov 2025 09:25:16 +0000 (+0100) Subject: [release-branch.go1.25] os,internal/poll: don't call IsNonblock for consoles and... X-Git-Tag: go1.25.6~12 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=93f5d1c27e6942dc582da8165400bdeff62e5877;p=gostls13.git [release-branch.go1.25] 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 #76392 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 (cherry picked from commit CL 724640) Reviewed-on: https://go-review.googlesource.com/c/go/+/725580 Reviewed-by: Dmitri Shuralyov --- diff --git a/src/internal/poll/fd_windows.go b/src/internal/poll/fd_windows.go index dd833afd24..243aa4c7a5 100644 --- a/src/internal/poll/fd_windows.go +++ b/src/internal/poll/fd_windows.go @@ -402,6 +402,10 @@ func (fd *FD) Init(net string, pollable bool) error { fd.rop.fd = fd fd.wop.fd = fd + 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 7e94328710..ad172b5048 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} } // syscall.Open always returns a non-blocking handle. - return newFile(r, name, "file", false), nil + return newFile(r, name, kindOpenFile, false), 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 6f091c3818..c408372ad7 100644 --- a/src/os/os_windows_test.go +++ b/src/os/os_windows_test.go @@ -2209,3 +2209,42 @@ func TestSplitPath(t *testing.T) { } } } + +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 2ec09cf2d3..72fe170500 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 int, perm FileMode) (syscall.Handle, error) {