-
Notifications
You must be signed in to change notification settings - Fork 17.7k
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, runtime: should not have to wait for 10 EPIPE failures before raising SIGPIPE #11845
Comments
I think this proposal is a bad idea. Any Go program whose output might go into a pipe will now need a signal handler. The flag in 11389 is panicking on error, not EPIPE per se. |
This proposal would make Go programs act like C programs by default: writing to a closed pipe would cause the program to exit due to a SIGPIPE signal. To avoid that, a Go program would have to call signal.Ignore(syscall.SIGPIPE), which I agree is not ideal because it's Unix-specific. I also agree that this might be a bad default for Go. In that case, I think we need a simple way for Go programs to say that they should exit when writing to a closed pipe, to avoid the contortions required to check for EPIPE. It would be appropriate for any Go program that acts as a command line client to make that call. Right now, a Go program can do it by calling syscall.Reset(syscall.SIGPIPE), and perhaps the go tool should do that at least for the help command. But, again, it's Unix-specific. Maybe we need something like runtime.ExitOnBrokenPipe(bool). |
FWIW I disagree that Go programs should act like C programs by default, especially because you get SIGPIPE for writes to closed network connections too. If we could somehow make SIGPIPE only come in for writes to standard output or standard error, maybe that would be okay. I think the real problem is that the template package does not distinguish template errors from write errors in any useful way. |
There may be a problem with the template package, but I think there is also a different problem here. If I have a command line tool that generates output, I want that tool to receive a SIGPIPE signal, and exit because of it, if it is writing to a closed pipe. Having that only happen after 10 writes fail is arbitrary and fails if I do fewer than 10 writes. |
one might also have a long-running go-program which is supervised by systemd. and it is "logging" to stdout/stderr. and systemd will write that log to journald. and someone is restarting journald (because that someone is changing, lets say, the compression of the .journal files). and that someone is running systemd-218 or below (so systemd won't save the given filedescriptors such as implemented in systemd-219+). upon journald-restart the program will get SIGPIPE and and it is very reasonable to shut down the program as well (since someone removed the "logging" infrastructure underneath). |
See #11898 |
Proposal after talking to Russ:
With these rules:
Of course, this should all be documented somewhere. |
"Proposal" accepted. |
Sorry to be pedantic, but what exactly does it mean to get an EPIPE "on os.Stdout or os.Stderr"? E.g.,
I'm thinking only case 3 should raise SIGPIPE (i.e., the behavior is tied to the |
I was thinking it meant fd 1 or fd 2. |
That seems justifiable to me too. |
CL https://golang.org/cl/18151 mentions this issue. |
CL https://golang.org/cl/18226 mentions this issue. |
Update #11845. Change-Id: I1c248dc48abc62e51836b9ba50d6deb89706c730 Reviewed-on: https://go-review.googlesource.com/18226 Reviewed-by: Russ Cox <rsc@golang.org>
Back in https://go.googlesource.com/go/+/f7d3eb9db97a65a43b8d6b8bf42b8698fe4468ee we added code so that if we saw 10 EPIPE failures in a row on the same file descriptor, we would exit the program. Today we exit the program by raising a SIGPIPE signal, but we still wait for 10 failures. That leads to issues like https://golang.org/issue/11839 and workarounds like https://golang.org/cl/12571.
Now that we have the os/signal package, Go programs have the mechanisms they need to control this. I propose that we change the Go runtime to not handle SIGPIPE by default. If os/signal.Notify is used for SIGPIPE, that only at that point should we add a handler for SIGPIPE. Then we should remove the EPIPE handling from the os package, and treat it like any other error.
The text was updated successfully, but these errors were encountered: