Go: os: don't waste extra threads for opening files

[Al2Klimov]

What version of Go are you using (go version)?

$ go version
go version go1.12.6 darwin/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output

$ go env

GOARCH="amd64"

GOBIN=""

GOCACHE="/Users/aklimov/Library/Caches/go-build"

GOEXE=""

GOFLAGS=""

GOHOSTARCH="amd64"

GOHOSTOS="darwin"

GOOS="darwin"

GOPATH="/Users/aklimov/Git/golang"

GOPROXY=""

GORACE=""

GOROOT="/Users/aklimov/Git/brew/Cellar/go/1.12.6/libexec"

GOTMPDIR=""

GOTOOLDIR="/Users/aklimov/Git/brew/Cellar/go/1.12.6/libexec/pkg/tool/darwin_amd64"

GCCGO="gccgo"

CC="clang"

CXX="clang++"

CGO_ENABLED="1"

GOMOD=""

CGO_CFLAGS="-g -O2"

CGO_CPPFLAGS=""

CGO_CXXFLAGS="-g -O2"

CGO_FFLAGS="-g -O2"

CGO_LDFLAGS="-g -O2"

PKG_CONFIG="pkg-config"

GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/3h/0nn80rp973b08bd0r56g5pd80000gn/T/go-build873614728=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

1. I've built the program shown below,
2. ran mkfifo pipe and
3. ran GOMAXPROCS=1 lldb theprogramshownbelow.

package main

import (
    "fmt"
    "os"
    "runtime"
    "time"
)

func main() {
    for i := runtime.NumCPU() * 2; i > 0; i-- {
        go openPipe()
    }

    for {
        time.Sleep(time.Second)
    }
}

func openPipe() {
    p, errOpen := os.Open("./pipe")
    fmt.Printf("os.Open(\"./pipe\"): %#v\n", errOpen)
    p.Close()

    for {
        time.Sleep(time.Second)
    }
}

What did you expect to see?

Not much more threads than one.

What did you see instead?

11 threads including a lot of hanging in the open(2) syscall.

[gopherbot]

Change https://golang.org/cl/182397 mentions this issue: os: open(2) files w/o blocking

[FiloSottile]

/cc @rsc @ianlancetaylor

[tv42]

After the suggested change, os.Open("/dev/stdout") and such will likely cause explosions in other processes. Unfortunately nonblocking on Unix is a property of the file, not the file descriptor (or a separate syscall), and making unsuspecting processes suddenly have their shared files be nonblocking is not a good idea.

[Al2Klimov]

Are you sure? fcntl(2) being effectively used in #32619 "provides for control over descriptors" according to the man page.

[tv42]

@Al2Klimov The per-FD flags are accessed with F_GETFD/F_SETFD, the per-file flags are accessed with F_GETFL/F_SETFL. O_NONBLOCK is defined as a "file status flag" (not file descriptor flag), and used with F_SETFL.

[ianlancetaylor]

If /dev/stdout is connected to a pipe, then I suspect that we may already cause this kind of confusion, since in that case we will set the O_NONBLOCK flag on the file descriptor via syscall.SetNonblock. That is, I'm not sure that the suggested change introduces a problem for /dev/stdout that does not already exist.

[tv42]

@ianlancetaylor Unf actually that is pretty nasty. I mean, one could hope that in 2019 every process was already prepared to handle EAGAIN, but it really wouldn't surprise me if doing that still either made a bunch of things error out, or sent them into busy loops.

[tv42]

Fun fact: a Go process reading stdin is the only process on my machine with a non-blocking stdin/stdout.

$ grep -h ^flags: /proc/*/fdinfo/{0,1,2}|sort|uniq -c|sort -n
      2 flags:  02000000                   <----------- this is non-blocking
     11 flags:  00
     11 flags:  0100002
     15 flags:  01
    117 flags:  0100000
    153 flags:  02
    177 flags:  0100001

[tv42]

So my earlier argument was "some things will break", with a vague promise of breaking. Here's a concrete breakage if you automatically add O_NONBLOCK to all os.OpenFile calls:

man 7 fifo

  A process can open a FIFO in nonblocking mode.  In this  case,  opening
  for  read-only succeeds even if no one has opened on the write side yet
  and opening for write-only fails with ENXIO (no such device or address)
  unless the other end has already been opened.

So, automatic O_NONBLOCK at open(2) time will break all fifo uses where the write side is opened first.

[Al2Klimov]

Strange. With Create() instead of Open() I also got error=nil.

[dominikh]

I'd like to point out that O_NONBLOCK is an attribute of the file _description_ (where description != descriptor), not the file. That is, two calls to _open_ will produce two different file descriptions. A file description can have many file descriptors. For example, calling dup on a file descriptor will yield a new file descriptor that shares the first file descriptor's file description. Same goes for fork.

The core issues explained by @tv42 still apply, but two processes each opening the same file cannot screw each other up, as they will not share the same file description.

Of course, the file descriptors 0–2 still commonly share a single file description, but opening /dev/stdout in non-blocking mode _should_ be safe.

Refer to the following excerpt of open(2)

   A call to open() creates a new open file description, an entry in the
   system-wide table of open files.  The open file description records
   the file offset and the file status flags (see below). 

as well as the entire subsection Open file descriptions of section NOTES.

[tv42]

@dominikh is correct, I misremembered the details. The traps only activate if you share an open fd (inherit it or pass it over unix domain socket). So by that logic, just os.OpenFile("/dev/stdout", os.O_WRONLY|syscall.O_NONBLOCK, 0) shouldn't hurt others.

Adding O_NONBLOCK to all opens will still break fifo write-side opening with no readers:

package main

import (
    "log"
    "os"
    "syscall"
)

func main() {
    if err := os.Remove("fifo"); err != nil && !os.IsNotExist(err) {
        log.Fatalf("remove: %v", err)
    }
    if err := syscall.Mkfifo("fifo", 0644); err != nil {
        log.Fatalf("mkfifo: %v", err)
    }

    f, err := os.OpenFile("fifo", os.O_WRONLY|syscall.O_NONBLOCK, 0644)
    if err != nil {
        log.Fatalf("open: %v", err)
    }
    defer f.Close()
}

$ go run .
2019/06/22 12:00:39 open: open fifo: no such device or address
exit status 1

Also, I'd like to clarify the issue subject: it should be "os: don't waste extra threads when opening fifos". This bug is only relevant if you're opening lots of FIFOs for reading concurrently. That's a really rare edge case scenario. The long wait in the syscall doesn't trigger on most kinds of files.

FIFOs are a rare special case, and 1) leaving O_NONBLOCK out wastes system call threads 2) adding O_NONBLOCK breaks some use cases.

POSIX doesn't define what O_NONBLOCK means for FIFOs, any improvement here is Linux-only.

You could perhaps argue for automatic O_NONBLOCK as long as it's the file flag is O_RDONLY. But to deal with fifos, you're making raw syscalls anyway to create one, etc. I'd be ready to call this an obscure enough an edge case that making the caller use syscalls and os.NewFile is not too much of a burden.