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: Stdin.SetDeadline() fails on Linux even when stdin is a pipe #24842

Closed
ibukanov opened this issue Apr 13, 2018 · 12 comments
Closed

os: Stdin.SetDeadline() fails on Linux even when stdin is a pipe #24842

ibukanov opened this issue Apr 13, 2018 · 12 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Linux

Comments

@ibukanov
Copy link

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

go version go1.10 linux/amd64

Does this issue reproduce with the latest release?

yes

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

GOARCH="amd64"
GOBIN=""
GOCACHE="/vol/var/root/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/vol/var/root/go"
GORACE=""
GOROOT="/vol/var/build/go-1.10"
GOTMPDIR=""
GOTOOLDIR="/vol/var/build/go-1.10/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
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"
GOGCCFLAGS="-fPIC -m64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build675138706=/tmp/go-build -gno-record-gcc-switches"

What did you do?

On Linux os.Stdin.SetDeadline() fails with file type does not support deadline even when the standard input is a pipe. The following code demonstrates this:

package main

import (
        "fmt"
        "log"
        "os"
        "os/exec"
        "time"
)

func main() {
        if len(os.Args) == 2 && os.Args[1] == "do_test" {
                err := os.Stdin.SetDeadline(time.Now().Add(time.Minute))
                fmt.Println(err)
                f, err := os.Open("/proc/self/fd/0")
                if err != nil {
                        log.Fatal(err)
                }
                err = f.SetDeadline(time.Now().Add(time.Minute))
                fmt.Println(err)
                os.Exit(0)
        }

        r, w, err := os.Pipe()
        if err != nil {
                log.Fatal(err)
        }
        cmd := exec.Command(os.Args[0], "do_test")
        cmd.Stdin = r
        cmd.Stdout = os.Stdout
        cmd.Stderr = os.Stderr
        err = cmd.Run()
        if err != nil {
                log.Fatal(err)
        }
        r.Close()
        w.Close()
}

The program creates a pipe and then run itself the second time with the stdin of the second invocation connected to the pipe. Then the second copy calls os.Stdin.SetDeadline(). That fails with the above mentioned error that the program prints.

Then, as a workaround, the code opens the stdin again via Linux-specific /proc/self/fd/0 and calls SetDeadline on that. This time the SetDedaline reports a success. Note that this is not a universal workaround as /proc may not be available when, for example, the code runs in a restricted Linux container.

On play.golang.org the first SetDedaline call works as expected while the attempt to open /proc/self/fd/0 fails.

What did you expect to see?

The program should print:

<nil>
<nil>

to indicate that both SetDedaline calls returned nil as error.

What did you see instead?

file type does not support deadline
<nil>
@tklauser tklauser changed the title os.Stdin.SetDeadline() fails on Linux even when stdin is a pipe os: Stdin.SetDeadline() fails on Linux even when stdin is a pipe Apr 13, 2018
@tklauser tklauser added OS-Linux NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Apr 13, 2018
@pam4
Copy link

pam4 commented Apr 13, 2018

It happens in my system too.
It seems that deadlines never work with files obtained with os.NewFile (like os.Stdin is).
Take a look at this code (assume that stdin is a pipe):

func main() {
    fopen, err := os.Open("/proc/self/fd/0")
    if err != nil {
        log.Fatal(err)
    }
    err = fopen.SetDeadline(time.Now().Add(time.Minute))
    fmt.Println(err)  // prints "<nil>"
    fnew := os.NewFile(fopen.Fd(), fopen.Name())
    err = fnew.SetDeadline(time.Now().Add(time.Minute))
    fmt.Println(err)  // prints "file type does not support deadline"
}

os.NewFile calls newFile with kind == kindNewFile, therefore according to the comment "the file is not added to the runtime poller".

const (
	kindNewFile newFileKind = iota
	kindOpenFile
	kindPipe
)

// newFile is like NewFile, but if called from OpenFile or Pipe
// (as passed in the kind parameter) it tries to add the file to
// the runtime poller.
func newFile(fd uintptr, name string, kind newFileKind) *File {

@ianlancetaylor
Copy link
Contributor

There is unfortunately no good way to support this. The SetDeadline support requires that the file be in non-blocking mode. When you call os.Pipe the pipes are created in non-blocking mode. Standard input is normally in blocking mode. We can't reasonably change the blocking mode of standard input, as that would affect any other programs that are using the same descriptor for standard input, including, in a common case, the shell after our program exits.

In the future 1.11 release, if you put the standard input pipe into nonblocking mode before starting the Go program, then SetDeadline will work. That's the best we can do.

Closing this issue because I don't see any way to fix it.

@ibukanov
Copy link
Author

@ianlancetaylor I am surprised that SetDeadline has not changed on its own file from blocking to non-blocking. Was was the reason for that? As the new API it could be a reasonable opt-in.

In any case, why not provide an explicit API for 1.11 to make a descriptor non-blocking or have a version of SetDeadline that makes so on supported file descriptors? Without that it seems it will be still impossible to write a command line go application like a trivial cat-like program without using system-specific hacks like opening /proc/self/fd/0. Upon receiving an exit signal such application should close stdin and write all data that it read to stdout and exits.

@ianlancetaylor
Copy link
Contributor

Changing an existing file descriptor to non-blocking mode is a significant change that can affect other programs using the same descriptor (due to pipes passed through exec). It is not a change that can be made as a side effect of some other call.

We could consider an explicit API to change a descriptor to non-blocking and add it to the poller. But it seems like a very rare use case.

I don't understand why you say that you have can't write a cat-like program. Clearly you can. If you are worried about signals, catch them with the os/signal package. If you want a deadline, do your reads in a goroutine and let your main program use a timer. Using SetDeadline is an optimization. Go provides several other mechanisms for reading from a file with a deadline.

@ibukanov
Copy link
Author

@ianlancetaylor In my case the code is a log transformer that adds a prefix/cleanup to each line it receives on stdin connected to a pipe from another program and writes that to stdout. I want to ensure that I can restart the transformer without loosing any log messages when the transformer receives a termination signal.

Without working SetDeadline one cannot do that in Go AFAICS without platform-specific code. The problem is how to stop reading from stdin without loosing any data. I thought SetDeadline in 1.10 finally provided a way. Upon the termination condition the code just sets the deadline on the stdin to a moment in the past. That makes the go routine that reads it to quit allowing to perform the final write and exit. But then I discovered this issue.

Now, if 1.11 makes SetDeadline to work with nonblocking stdin, then at least I can do much more portable hack than opening /dev/stdin (which may not work even with mounted /proc if stdin comes from a pipe from a process with different uid) . The code can use a syscall to check if stdin is blocking, make it non-blocking and then re-exec itself. But it would be nice to be able to avoid that...

@ianlancetaylor
Copy link
Contributor

I see how SetDeadline could help your case. It's not that you want to set a deadline, it's that you want to interrupt the Read. In this case, just call syscall.Close(0) and give the Read a few milliseconds. It should have the same effect.

@pam4
Copy link

pam4 commented Apr 17, 2018

@ianlancetaylor In my Linux system (go1.10.1) syscall.Close(0) will not unblock an os.Stdin.Read.
Same effect with os.Stdin.Close, though calling that from a different goroutine doesn't seem to be explicitly supported.
Both do close os.Stdin in the sense that the next read will return an error immediately, but the current read will not unblock unless some data arrives.
You can always leave it blocked and call os.Exit (or return from main) at will, but there is a race because the Read could unblock just after your program has decided to give up on it, and you would lose one Read's data.
Not so common need, but I can imagine other scenarios, for example to implement something like a read -t timeout similar to the shell builtin.

EDIT: maybe you meant that it will work if the fd is set to non-blocking mode, which I haven't tried.

@ianlancetaylor
Copy link
Contributor

I agree that it won't unblock a read. That's why I suggested waiting a few milliseconds to see if the read will complete. It's not going to complete after the descriptor is closed.

Anyhow for your specific use case it seems to me that you can do this:

package main

import (
	"fmt"
	"os"
	"syscall"
	"time"
)

func main() {
	if err := syscall.SetNonblock(0, true); err != nil {
		panic(err)
	}
	f := os.NewFile(0, "stdin")
	go func() {
		time.Sleep(time.Millisecond)
		fmt.Println("setting deadline")
		if err := f.SetDeadline(time.Now().Add(-time.Millisecond)); err != nil {
			panic(err)
		}
	}()
	var buf [1]byte
	_, err := f.Read(buf[:])
	if err == nil {
		panic("Read succeeded")
	}
	fmt.Println("read failed as expected:", err)
	if err := syscall.SetNonblock(0, false); err != nil {
		panic(err)
	}
}

@pam4
Copy link

pam4 commented Apr 17, 2018

@ianlancetaylor

I agree that it won't unblock a read. That's why I suggested waiting a few milliseconds to see if the read will complete. It's not going to complete after the descriptor is closed.

My tests seem to suggest that it will complete if some data arrives:

$ { sleep 30; echo test; } | mygoprogram
test
copy err: read /dev/stdin: bad file descriptor

mygoprogram:

func main() {
    go func() {
        time.Sleep(time.Second)
        syscall.Close(0)
    }()
    _, err := io.Copy(os.Stdout, os.Stdin)
    fmt.Println("copy err:", err)
}

Thanks for your useful code sample; syscall.SetNonblock works for me and Read doesn't block.
If I run it like this: sleep 30 | mygoprogram I get:

read failed as expected: read stdin: resource temporarily unavailable

If I run it like this: echo test | mygoprogram I get:

panic: Read succeeded
[...]

The spawned goroutine doesn't seem to have a chance to start in either cases (even if I print something at the top I don't see it). You probably intended for it to be run with go1.11 that will have this feature.

@ianlancetaylor
Copy link
Contributor

You're right, I was assuming 1.11. Any change in this area would only be in 1.11 anyhow, it would not be backported to 1.10.

@ibukanov
Copy link
Author

I was hopping to have a solution in 1.11 without syscall.*, but rather via some os.* portable API. But single syscall.SetNonblock() followed by os.NewFile() does not look bad.

@ianlancetaylor
Copy link
Contributor

Changing standard input to be nonblocking is both unusual and risky, in that if it is left in nonblocking mode it can easily break the shell after the program exits. I don't think we need support for this in the os package.

@golang golang locked and limited conversation to collaborators Apr 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. OS-Linux
Projects
None yet
Development

No branches or pull requests

5 participants