-
Notifications
You must be signed in to change notification settings - Fork 18k
os/exec: program hangs if it reads from pipe on Plan 9 (9front) #43524
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
Comments
Is the |
I don't think so. The OCEXEC change happened on December 13th and that issue was opened in October. |
Change https://golang.org/cl/281833 mentions this issue: |
Pinged on the patch, not sure if pinging here is better. Are there any open questions, changes, or updates that I need to do to get the fix landed? |
Change Plan 9 fork/exec to use the O_CLOEXEC file descriptor, instead of relying on spooky at a distance. Historically, Plan 9 has set the O_CLOEXEC flag on the underlying channels in the kernel, rather than the file descriptors -- if two fds pointed at a single channel, as with dup, changing the flags on one of them would be observable on the other. The per-Chan semantics are ok, if unexpected, when a chan is only handled within a single process, but this isn't always the case. Forked processes share Chans, but even more of a problem is the interaction between /srv and OCEXEC, which can lead to unexectedly closed file descriptors in completely unrelated proceses. For example: func exists() bool { // If some other thread execs here, // we don't want to leak the fd, so // open it O_CLOEXEC fd := Open("/srv/foo", O_CLOEXEC) if fd != -1 { Close(fd) return true } return false } would close the connection to any file descriptor (maybe even for the root fs) in ALL other processes that have it open if an exec were to happen(!), which is quite undesriable. As a result, 9front will be changing this behavior for the next release. Go is the only code observed so far that relies on this behavior on purpose, and It's easy to make the code work with both semantics: simply using the file descriptor that was opened with O_CEXEC instead of throwing it away. So we do that here. Fixes golang#43524 Change-Id: I4887f5c934a5e63e5e6c1bb59878a325abc928d3 GitHub-Last-Rev: 96bb21b GitHub-Pull-Request: golang#43533 Reviewed-on: https://go-review.googlesource.com/c/go/+/281833 Reviewed-by: David du Colombier <0intro@gmail.com> Reviewed-by: Richard Miller <millerresearch@gmail.com> Reviewed-by: Jacob Moody <j4kem00dy@gmail.com> Run-TryBot: David du Colombier <0intro@gmail.com> Trust: Ian Lance Taylor <iant@golang.org>
Fix is committed, so this should be closed. Thanks to the people that reviewed the code. |
Fixed in CL 281833. |
What version of Go are you using (
go version
)?Does this issue reproduce with the latest release?
Yes
What operating system and processor architecture are you using (
go env
)?amd64/plan9 (only 9front)
What did you do?
Run this program:
What did you expect to see?
No hang
What did you see instead?
Program hangs trying to read from stdin, and also the parent process hangs trying to read error string from the child. Speaking to 9front devs, this seems to be due to a recent change in how OCEXEC flag works. The OCEXEC flag semantics have been changed to refer to the file descriptor instead of the channel.
Note: you can only reproduce this when stdin is a pipe. It doesn't hang if the stdin is a disk file. So, there are two pipes involved here. One is created by
os/exec
to write to program stdin, and the other pipe is used by thesyscall
package to read error string from the child. The write end of the pipe created by syscall is marked closed-on-exec. Since the close-on-exec is no longer set correctly on 9front, we hang trying to read the error string from the child. Write to stdin pipe also likely hangs due to this.It's possible to add a workaround which works on both 9front and 9legacy.
@gopherbot Add labels OS-Plan9, NeedsFix
The text was updated successfully, but these errors were encountered: