Skip to content

Commit

Permalink
os: if descriptor is non-blocking, retain that in Fd method
Browse files Browse the repository at this point in the history
For golang#58408
Fixes golang#60211

Change-Id: I30f5678b46e15121865b19d1c0f82698493fad4e
Reviewed-on: https://go-review.googlesource.com/c/go/+/495079
Run-TryBot: Ian Lance Taylor <iant@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Bryan Mills <bcmills@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
  • Loading branch information
ianlancetaylor authored and gopherbot committed May 17, 2023
1 parent f0de4b4 commit f777726
Show file tree
Hide file tree
Showing 6 changed files with 78 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/internal/syscall/unix/nonblocking.go
Expand Up @@ -19,3 +19,7 @@ func IsNonblock(fd int) (nonblocking bool, err error) {
}
return flag&syscall.O_NONBLOCK != 0, nil
}

func HasNonblockFlag(flag int) bool {
return flag&syscall.O_NONBLOCK != 0
}
4 changes: 4 additions & 0 deletions src/internal/syscall/unix/nonblocking_js.go
Expand Up @@ -9,3 +9,7 @@ package unix
func IsNonblock(fd int) (nonblocking bool, err error) {
return false, nil
}

func HasNonblockFlag(flag int) bool {
return false
}
4 changes: 4 additions & 0 deletions src/internal/syscall/unix/nonblocking_libc.go
Expand Up @@ -19,6 +19,10 @@ func IsNonblock(fd int) (nonblocking bool, err error) {
return flag&syscall.O_NONBLOCK != 0, nil
}

func HasNonblockFlag(flag int) bool {
return flag&syscall.O_NONBLOCK != 0
}

// Implemented in the syscall package.
//
//go:linkname fcntl syscall.fcntl
Expand Down
4 changes: 4 additions & 0 deletions src/internal/syscall/unix/nonblocking_wasip1.go
Expand Up @@ -19,6 +19,10 @@ func IsNonblock(fd int) (nonblocking bool, err error) {
return flags&syscall.FDFLAG_NONBLOCK != 0, nil
}

func HasNonblockFlag(flag int) bool {
return flag&syscall.FDFLAG_NONBLOCK != 0
}

// This helper is implemented in the syscall package. It means we don't have
// to redefine the fd_fdstat_get host import or the fdstat struct it
// populates.
Expand Down
50 changes: 50 additions & 0 deletions src/os/fifo_test.go
Expand Up @@ -8,6 +8,7 @@ package os_test

import (
"errors"
"internal/syscall/unix"
"internal/testenv"
"io/fs"
"os"
Expand Down Expand Up @@ -155,3 +156,52 @@ func TestNonPollable(t *testing.T) {
}
}
}

// Issue 60211.
func TestOpenFileNonBlocking(t *testing.T) {
exe, err := os.Executable()
if err != nil {
t.Skipf("can't find executable: %v", err)
}
f, err := os.OpenFile(exe, os.O_RDONLY|syscall.O_NONBLOCK, 0666)
if err != nil {
t.Fatal(err)
}
defer f.Close()
nonblock, err := unix.IsNonblock(int(f.Fd()))
if err != nil {
t.Fatal(err)
}
if !nonblock {
t.Errorf("file opened with O_NONBLOCK but in blocking mode")
}
}

func TestNewFileNonBlocking(t *testing.T) {
var p [2]int
if err := syscall.Pipe(p[:]); err != nil {
t.Fatal(err)
}
if err := syscall.SetNonblock(p[0], true); err != nil {
t.Fatal(err)
}
f := os.NewFile(uintptr(p[0]), "pipe")
nonblock, err := unix.IsNonblock(p[0])
if err != nil {
t.Fatal(err)
}
if !nonblock {
t.Error("pipe blocking after NewFile")
}
fd := f.Fd()
if fd != uintptr(p[0]) {
t.Errorf("Fd returned %d, want %d", fd, p[0])
}
nonblock, err = unix.IsNonblock(p[0])
if err != nil {
t.Fatal(err)
}
if !nonblock {
t.Error("pipe blocking after Fd")
}
}
17 changes: 12 additions & 5 deletions src/os/file_unix.go
Expand Up @@ -116,12 +116,12 @@ 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.
// Open, Create, or OpenFile (without O_NONBLOCK).
kindOpenFile
// kindPipe means that the descriptor was opened using Pipe.
kindPipe
// kindNonBlock means that the descriptor was passed to us via NewFile,
// and the descriptor is already in non-blocking mode.
// kindNonBlock means that the descriptor is already in
// non-blocking mode.
kindNonBlock
// kindNoPoll means that we should not put the descriptor into
// non-blocking mode, because we know it is not a pipe or FIFO.
Expand Down Expand Up @@ -184,7 +184,9 @@ func newFile(fd uintptr, name string, kind newFileKind) *File {
clearNonBlock := false
if pollable {
if kind == kindNonBlock {
f.nonblock = true
// The descriptor is already in non-blocking mode.
// We only set f.nonblock if we put the file into
// non-blocking mode.
} else if err := syscall.SetNonblock(fdi, true); err == nil {
f.nonblock = true
clearNonBlock = true
Expand Down Expand Up @@ -263,7 +265,12 @@ func openFileNolog(name string, flag int, perm FileMode) (*File, error) {
syscall.CloseOnExec(r)
}

f := newFile(uintptr(r), name, kindOpenFile)
kind := kindOpenFile
if unix.HasNonblockFlag(flag) {
kind = kindNonBlock
}

f := newFile(uintptr(r), name, kind)
f.pfd.SysFile = s
return f, nil
}
Expand Down

0 comments on commit f777726

Please sign in to comment.