Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
tw4452852 opened this issue Jul 6, 2017 · 15 comments
Closed

Comments

@tw4452852
Copy link
Contributor

@tw4452852 tw4452852 commented Jul 6, 2017

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.

@tw4452852 tw4452852 changed the title os: reading on nonblocking stdin causes "waiting for unsupported file type os: reading on nonblocking stdin causes "waiting for unsupported file type" error Jul 6, 2017
@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 6, 2017

@tw4452852 this is not a complete bug report.

Please update your report to include all the requested information.

@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 6, 2017

We can reopen this when there are any details.

@bradfitz bradfitz closed this Jul 6, 2017
@tw4452852
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 6, 2017

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

@tw4452852
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 6, 2017

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
Copy link
Contributor

@bradfitz bradfitz commented Jul 6, 2017

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

@tw4452852
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 6, 2017

@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
Copy link
Contributor

@davecheney davecheney commented Jul 6, 2017

/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.

@davecheney davecheney reopened this Jul 6, 2017
@bradfitz
Copy link
Contributor

@bradfitz bradfitz commented Jul 6, 2017

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
Copy link
Contributor

@rsc rsc commented Jul 6, 2017

@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
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 6, 2017

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
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 6, 2017

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

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 6, 2017

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

@davecheney
Copy link
Contributor

@davecheney davecheney commented Jul 6, 2017

@tw4452852
Copy link
Contributor Author

@tw4452852 tw4452852 commented Jul 7, 2017

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

@ianlancetaylor ianlancetaylor added this to the Go1.9 milestone Jul 13, 2017
@gopherbot
Copy link

@gopherbot gopherbot commented Jul 13, 2017

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

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
6 participants
You can’t perform that action at this time.