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

Jump to bottom

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

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

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

tw4452852 opened this issue Jul 6, 2017 · 15 comments
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone
Go1.9

Comments

@tw4452852
Copy link
Contributor

tw4452852 commented Jul 6, 2017 •
edited

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

@tw4452852 this is not a complete bug report.

Please update your report to include all the requested information.

@bradfitz
Copy link
Contributor

bradfitz commented Jul 6, 2017

We can reopen this when there are any details.

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

tw4452852 commented Jul 6, 2017 •
edited

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

@tw4452852
Copy link
Contributor Author

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

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

@tw4452852 tw4452852 mentioned this issue Jul 6, 2017
Closed
@davecheney
Copy link
Contributor

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

@bradfitz bradfitz added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Jul 6, 2017
@rsc
Copy link
Contributor

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

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

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

@gopherbot
Copy link

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

@davecheney
Copy link
Contributor

davecheney commented Jul 6, 2017 via email

@tw4452852
Copy link
Contributor Author

tw4452852 commented Jul 7, 2017 •
edited

@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

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

@ianlancetaylor ianlancetaylor mentioned this issue Mar 21, 2018
Closed
@golang golang locked and limited conversation to collaborators Jul 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Milestone
Go1.9
Development

No branches or pull requests
6 participants
@bradfitz @davecheney @rsc @tw4452852 @ianlancetaylor @gopherbot