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

Open
zombiezen opened this Issue Jul 19, 2017 · 14 comments

Comments

Projects
None yet
7 participants
@zombiezen
Member

zombiezen commented Jul 19, 2017

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

go version go1.8.3 windows/amd64

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

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=C:\Users\light\go
set GORACE=
set GOROOT=C:\Go
set GOTOOLDIR=C:\Go\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

https://play.golang.org/p/r5V40AgLht

What did you expect to see?

Parent process
Child process
Hello, World!

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?

Parent process
fork/exec C:\Users\light\AppData\Local\Temp\2\go-build579847834\command-line-arguments\_obj\exe\foo.exe: not supported by windows
exit status 1
@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jul 19, 2017

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 ExtraFiles field. That is how ExtraFiles is designed to work, but while that works on Unix, it does not work on Windows. To implement this we would have to somehow tell the child process that file descriptor 3 should map to some handle, but that seems difficult to do in general.

@zombiezen

This comment has been minimized.

Member

zombiezen commented Jul 20, 2017

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 Cmd.ExtraFiles doc:

ExtraFiles specifies additional open files to be inherited by the new process. It does not include standard input, standard output, or standard error. If non-nil, entry i becomes file descriptor 3+i.

It seems unfortunate to be unable to send pipes to subprocesses on Windows. I think we should address that somehow.

@ianlancetaylor

This comment has been minimized.

Contributor

ianlancetaylor commented Jul 20, 2017

I believe you can do it today by calling the Fd method to get the handle, calling SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 1) to let it stay open in the child, and then passing the handle to the child as an argument. For the os/exec package, it's the last step that is the tricky part. We could invent some way to pass the files to a Go program, but I don't know of any general purpose mechanism that would work for a non-Go program. As far as I know the program basically has to receive the handle number in a command line argument or environment variable.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Aug 8, 2017

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.

I believe you can do it today by calling the Fd method to get the handle, calling SetHandleInformation(handle, HANDLE_FLAG_INHERIT, 1) to let it stay open in the child, and then passing the handle to the child as an argument.

Normally you would use DuplicateHandle API to do that. It provides everything you might need just for that job.

Alex

@zombiezen

This comment has been minimized.

Member

zombiezen commented Aug 8, 2017

Makes sense. I'd like to send a doc fix to Cmd.ExtraFiles to summarize this discussion.

One more question though: if SetHandleInformation enables inheriting handles, would it make sense to amend the behavior of ExtraFiles on Windows to set this option on all the file handles in the ExtraFiles slice? I understand that there's still an out-of-band mechanism that's required to pass the individual handles, but in some ways, ExtraFiles already requires some arrangement with the child process on Unix systems. Clearly, this would need to be communicated in the doc comment how this works, but there's already difference between Windows and Unixy behavior for this field.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Aug 9, 2017

if SetHandleInformation enables inheriting handles, would it make sense to amend the behavior of ExtraFiles on Windows to set this option on all the file handles in the ExtraFiles slice?

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?

ExtraFiles already requires some arrangement with the child process on Unix systems

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

@mattn

This comment has been minimized.

Member

mattn commented Aug 9, 2017

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.

gospel echo-server

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.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Aug 11, 2017

https://github.com/mattn/gospel

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

@ccbrown

This comment has been minimized.

ccbrown commented Aug 12, 2017

@alexbrainman

What you are proposing - calling SetHandleInformation - will make that handle inheritable, and it can possibly escape into wrong process.

Seems to me like if you use SetHandleInformation to make the handle inheritable, create the process, then use SetHandleInformation to make the handle uninheritable again, all while holding ForkLock, you wouldn't have this risk. Am I missing something?

Then you could just pass the same handles that you gave to ExtraFiles to the child process via arguments, environment variables, etc.

ExtraFiles already requires some arrangement with the child process on Unix systems

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.

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.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Aug 14, 2017

Seems to me like if you use SetHandleInformation to make the handle inheritable, create the process, then use SetHandleInformation to make the handle uninheritable again, all while holding ForkLock, you wouldn't have this risk. Am I missing something?

I think what you are suggesting should work.

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.

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

@glasser

This comment has been minimized.

Contributor

glasser commented Feb 26, 2018

I am by no means an expert on Windows, but it looks like an answer involves the _get_osfhandle system call.

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 _get_osfhandle syscall in the child (passing, say, 3 as an argument) to translate into something that works with underlying calls. Turned out doing the same in Go works OK too:

	// Strangely, it always returns non-nil error, so we ignore it. If we got an
	// error, the next step (writing to the fd) will fail anyway.
	value, _, _ := syscall.NewLazyDLL("msvcrt").NewProc("_get_osfhandle").Call(fd)
	return uintptr(value)

Calling this with fd = 3 returns something that can be passed to os.NewFile and seems to work properly, at least when the pipe is set up by Node.

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!

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Feb 27, 2018

I found that Node supports this (parent and child) and that it called the _get_osfhandle syscall in the child (passing, say, 3 as an argument) to translate into something that works with underlying calls.

Yes, like I said before, if parent and child agree on how to pass file handles, that should work.

syscall.NewLazyDLL("msvcrt").NewProc("_get_osfhandle").Call(fd)

Yes that should work. Unless some computers do not have msvcrt.dll.

Alex

@zhangyoufu

This comment has been minimized.

zhangyoufu commented Apr 9, 2018

It is possible to pass file/pipe handles and some internal flags to child process using STARTUPINFO.lpReserved2. This is an undocumented feature inside MSVC C runtime. I have an unbuffer.py to demonstrate how to unbuffer stdout of child process using this idea. Maybe we can mimic this undocumented feature and implement ExtraFiles on Windows.

@alexbrainman

This comment has been minimized.

Member

alexbrainman commented Apr 9, 2018

Maybe we can mimic this undocumented feature and implement ExtraFiles on Windows.

I don't see how that is possible. How would that work?

Alex

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment