os: reading on nonblocking stdin causes "waiting for unsupported file type" error

[tw4452852]

Please answer these questions before submitting your issue. Thanks!

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

go version devel +c6e7cb4 Wed May 31 00:50:43 2017 +0000 linux/amd64

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

GOARCH="amd64"
GOBIN=""
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/tw/golib"
GORACE=""
GOROOT="/home/tw/goroot"
GOTOOLDIR="/home/tw/goroot/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build557202845=/tmp/go-build -gno-record-gcc-switches"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"

What did you do?

package main

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

func main() {
        fd := int(os.Stdin.Fd())
        syscall.SetNonblock(fd, true)
        defer syscall.SetNonblock(fd, false)

        bi := bufio.NewReaderSize(os.Stdin, 0)

        for {
                r, _, err := bi.ReadRune()
                if err != nil {
                        log.Printf("read error: %v\n", err)
                        return
                }
                log.Printf("read a rune: %q\n", r)
        }
}

What did you expect to see?

Get a EAGAIN error.

What did you see instead?

Get "waiting for unsupported file type" error.

[davecheney]

@tw4452852 this is not a complete bug report.

Please update your report to include all the requested information.

[bradfitz]

We can reopen this when there are any details.

[tw4452852]

@davecheney @bradfitz Sorry for clicking for sudden. Already update the necessary information.

[tw4452852]

I knew this is a expected result after we use poll for file (#18507), But I think we could still keep compatibility with such patch:

diff --git a/src/internal/poll/fd_unix.go b/src/internal/poll/fd_unix.go
index 3ca6e15..b2c570f 100644
--- a/src/internal/poll/fd_unix.go
+++ b/src/internal/poll/fd_unix.go
@@ -121,7 +121,7 @@ func (fd *FD) Read(p []byte) (int, error) {
                n, err := syscall.Read(fd.Sysfd, p)
                if err != nil {
                        n = 0
-                       if err == syscall.EAGAIN {
+                       if err == syscall.EAGAIN && fd.pd.runtimeCtx != 0 {
                                if err = fd.pd.waitRead(fd.isFile); err == nil {
                                        continue
                                }

[bradfitz]

You've described a solution. @davecheney and I are waiting to hear what the problem is.

[tw4452852]

@bradfitz
Before #18507 , If we try to read on a nonblocked file, we will get a EAGAIN error, but now we get a internal error waiting for unsupported file type instead. I think this will lead user confused. So I just propose a patch to try to keep the compatibility.

[davecheney]

/cc @ianlancetaylor

IMO I think there is probably a better way to do this, and changing the blocking state of an fd is going to cause breakage down the line anyway. However if this worked in 1.8.3, then I think it should at least be investigated as a regression.

[bradfitz]

Ian is on vacation.

What "worked" before? Is there a valid use case or just a contrived scenario?

How would this impact a real user? Because it's not this:

fd := int(os.Stdin.Fd())
syscall.SetNonblock(fd, true)

/cc @rsc

[rsc]

@bradfitz, @tw4452852's one-line patch SGTM. The error this program exposes is clearly an internal error and is less useful than returning EAGAIN as in Go 1.8.

[tw4452852]

What "worked" before? Is there a valid use case or just a contrived scenario?

One of the use cases is that users may poll nonblocking fds on their own.

[tw4452852]

@davecheney I have already encountered this "regression" in elves/elvish#384 and junegunn/fzf#910 which work in Go 1.8.

[gopherbot]

CL https://golang.org/cl/47555 mentions this issue.

[davecheney]

Thank you for sending your change. There is an error in gerrit which means I cannot leave my comment there, but what I wanted to say is this change needs a test so that it does not regres in the future.

…

On Thu, Jul 6, 2017 at 5:44 PM, GopherBot ***@***.***> wrote: CL https://golang.org/cl/47555 mentions this issue. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#20915 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAAcAwFGOb0d9WRpi-pzcgo_oVbBgoaDks5sLJBLgaJpZM4OPEK3> .

[tw4452852]

@davecheney I have added a test for this. Take a look at? Thanks.

[gopherbot]

CL https://golang.org/cl/48490 mentions this issue.