-
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: Pipe can't be inherited on windows #21085
Comments
I don't see how to implement this using the os/exec API, because I don't see how the child process can discover the handle. Your program is assuming that file descriptor 3 corresponds to the file passed in the |
I'm not exactly sure about discovery either. I'm more just observing that this is how os/exec says that things will occur, without much of an indication of how it works on Windows. In particular, from
It seems unfortunate to be unable to send pipes to subprocesses on Windows. I think we should address that somehow. |
I believe you can do it today by calling the |
Unfortunately what Ian said is correct. os/exec.Cmd.ExtraFiles does not work on Windows. We use Windows CreateProcess API to start new process. CreateProcess accepts STARTUPINFO struct that has only hStdInput, hStdOutput and hStdError fields. You can pass any file handle from one process to the other (just like on Unix), but you need some way for processes to exchange file handle numbers (stdio or TCP or ...). We could try and do something with os/exec.Cmd.ExtraFiles on Windows, but the problem is the child process - there is no general mechanism to pass that information between processes. Windows file handle do not have specific numbers, for example stdin is not 0, stdout is not 1 and so on.
Normally you would use DuplicateHandle API to do that. It provides everything you might need just for that job. Alex |
Makes sense. I'd like to send a doc fix to Cmd.ExtraFiles to summarize this discussion. One more question though: if |
I don't think it is that simple. We use CreateProcess Windows API to start new process. CreateProcess has bInheritHandles parameter that controls which handles are inherited by the child process. Go syscall package sets bInheritHandles to true because it needs to pass stdin, stdout and stderr. But this makes all inheritable handles escaped into child process (see the doco https://msdn.microsoft.com/en-us/library/windows/desktop/ms682425(v=vs.85).aspx : "... If this parameter TRUE, each inheritable handle in the calling process is inherited by the new process. ..."). We cannot allow that, so none of file handles inside Go are inheritable. We only make stdin, stdout and stderr for new child inheritable, just before we launch new process. We also have a lock that prevents us from running CreateProcess simultaneously, otherwise this would allow these 3 handles (potentially) end up in the wrong process. What you are proposing - calling SetHandleInformation - will make that handle inheritable, and it can possibly escape into wrong process. We could use DuplicateHandle API instead (just before we launch the process), and then close new handle immediately. But DuplicateHandle will create new file handle with new handle number. How are you going to pass that number to the user of Cmd.ExtraFiles?
Not true. I could be wrong because I do not know much about Unix, but whatever file is in ExtraFiles[0] will have file handle of 3 in the child process, ExtraFiles[1] will be 4, and so on. So everyone knows how it works, and you don't need to pass these numbers around. Alex |
This is simple implementation that passing socket into external process which works on Windows. https://github.com/mattn/gospel You can test this like below.
This pass the socket handle via environment variable GOSPEL_FD (in really, it's not handle, but file). To accept using the handle for the target process (limited), you should use DuplicateHandle. |
This actually uses WSADuplicateSocket and WSAStartup to pass network socket between processes. These functions use WSAPROTOCOL_INFO struct (instead of file handles) to do the job. Alex |
Seems to me like if you use Then you could just pass the same handles that you gave to
I would say there's still an out-of-band arrangement between the processes, it can just usually be made at compile-time for Unix since the file descriptors are predictable. But that's neither here nor there. This field already acts in non-portable ways, so surely allowing the behavior to be "on Windows, file descriptors are unchanged, and on Unix, entry i becomes file descriptor 3+i" would be reasonable. |
I think what you are suggesting should work.
Sure we could do that. Perhaps we should say more than "file descriptors are unchanged". It is very important that child deals with these file handles - at least close them, if it won't use them. Otherwise unpredictable things will happen. Alex |
I am by no means an expert on Windows, but it looks like an answer involves the I'm trying to spawn a Go binary from a Node program with an extra file. I found that Node supports this (parent and child) and that it called the
Calling this with fd = 3 returns something that can be passed to I don't know if this will lead to a more generic solution for os/exec, but maybe this will help others dealing with this issue! |
Yes, like I said before, if parent and child agree on how to pass file handles, that should work.
Yes that should work. Unless some computers do not have msvcrt.dll. Alex |
It is possible to pass file/pipe handles and some internal flags to child process using |
I don't see how that is possible. How would that work? Alex |
What's I haven't even attempted to read a significant fraction of Chrome's source code, and I'm definitely not a Windows developer, but.. Chrome's |
Here's evidence of a node.js program happily passing fds 3 & 4 to a |
I do not know. Alex |
Came across this when tackling a similar issue. In short, starting Windows Vista, |
Change https://golang.org/cl/288272 mentions this issue: |
Change https://golang.org/cl/288298 mentions this issue: |
Change https://golang.org/cl/288299 mentions this issue: |
This allows users to specify handles that they explicitly want to be inherited by the new process. These handles must already be marked as inheritable. Updates #44011. Updates #21085. Change-Id: Ib18322e7dc2909e68c4209e80385492804fa15d3 Reviewed-on: https://go-review.googlesource.com/c/go/+/288298 Trust: Jason A. Donenfeld <Jason@zx2c4.com> Reviewed-by: Alex Brainman <alex.brainman@gmail.com>
@zx2c4 This issue was closed by https://golang.org/cl/288299, I think these lines should be removed too: Lines 121 to 122 in 770f1de
go/src/syscall/exec_windows.go Lines 265 to 267 in 770f1de
But it seems that the CL only takes care of the parent process. It's still not clear how to make it work for the child process. I have tried the demo in the OP (https://play.golang.org/p/r5V40AgLht) with these changes, and the output is:
Then I modified the demo as below, and it works. func child() error {
fmt.Fprintln(os.Stderr, "Child process")
- r := os.NewFile(3, "<pipe>")
+ r := os.NewFile(os.Stderr.Fd()+4, "<pipe>")
_, err := io.Copy(os.Stdout, r)
return err
} I understand that this is not a reliable way to get the fd in the child process, I just want to prove that the CL works for the parent process somehow. Then I tried to use the FWIW, some of the tests in |
Note that pipe connection is just supported on non-windows as of now. see golang/go#21085.
Note that pipe connection is just supported on non-windows as of now. see golang/go#21085.
Note that pipe connection is just supported on non-windows as of now. see golang/go#21085.
What version of Go are you using (
go version
)?What operating system and processor architecture are you using (
go env
)?What did you do?
https://play.golang.org/p/r5V40AgLht
What did you expect to see?
I would like to be able to pass an unnamed piped into a subprocess. According to MSDN docs, this seems like something that Windows is capable of, but it is done using handles, not file descriptors IIUC.
What did you see instead?
The text was updated successfully, but these errors were encountered: