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

Request for creating pipes with fd other than 0,1,2 #2939

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

Srinivasa314
Copy link

No description provided.

@joshtriplett joshtriplett added the T-libs-api Relevant to the library API team, which will review and decide on the RFC. label Jun 4, 2020
@joshtriplett
Copy link
Member

File descriptors are not necessarily clustered near 0, and a slice doesn't seem like the right way to provide such a mapping. I personally think it would make more sense to call one function for each file descriptor (e.g. .fd(42, foo)).

Also, it may make sense to make the caller responsible for creating the pipes themselves (via some separate object), and supply them to the function along with the desired file descriptor number, rather than having the Command create the pipe and return the other end.

@retep998
Copy link
Member

retep998 commented Jun 4, 2020

While the Windows CRT does technically have the concept of fds, we should not be using them. Rust on Windows tries to be independent of the CRT, and we should not be exposing an API that ties us to the CRT.

@Srinivasa314
Copy link
Author

Srinivasa314 commented Jun 4, 2020

File descriptors are not necessarily clustered near 0, and a slice doesn't seem like the right way to provide such a mapping. I personally think it would make more sense to call one function for each file descriptor (e.g. .fd(42, foo)).

As far as I know, in windows once a pipe is created you can make the child process inherit the handle and pass the handle as a command line argument to the child. Or if you want to expose the
pipe as a specific fd then you have to create a struct containing a array of the HANDLEs. Then the
windows C runtime assigns file descriptors 0,1,... to the handles in the array.
So a specific file descriptor can only be assigned in *nix.

Also, it may make sense to make the caller responsible for creating the pipes themselves (via some separate object), and supply them to the function along with the desired file descriptor number, rather than having the Command create the pipe and return the other end

122b1a3

@Srinivasa314
Copy link
Author

While the Windows CRT does technically have the concept of fds, we should not be using them. Rust on Windows tries to be independent of the CRT, and we should not be exposing an API that ties us to the CRT.

The child process has to use some id to connect to the pipe. Fd is a cross platform id that can be used. Otherwise some abstraction has to be created. I think that rather than creating a cross platform pipe id type, fds can be used instead. Tell me if I should created a new pipe id type.

@MikailBag
Copy link

@Srinivasa314 AFAIU, winapi handles are direct equivalent for posix FDs.
To achieve handle inheritance one can use DuplicateHandle winapi function:

  1. Child process is created, e.g. via CreateProcess with CREATE_SUSPENDED value specified in dwCreationFlags.
  2. DuplicateHandle function is used to copy handle to child process (we can do it because CreateProcess returns child process handle).
  3. ResumeThread function is called, and now child process can run and use that handle.

This way arbitrary handle mapping can be achieved.

@Srinivasa314
Copy link
Author

Srinivasa314 commented Jun 4, 2020

@Srinivasa314 AFAIU, winapi handles are direct equivalent for posix FDs.
To achieve handle inheritance one can use DuplicateHandle winapi function:

  1. Child process is created, e.g. via CreateProcess with CREATE_SUSPENDED value specified in dwCreationFlags.
  2. DuplicateHandle function is used to copy handle to child process (we can do it because CreateProcess returns child process handle).
  3. ResumeThread function is called, and now child process can run and use that handle.

This way arbitrary handle mapping can be achieved.

Using DuplicateHandle , you cannot set whatever handle you want unlike dup2. You have to pass a HANDLE pointer and another duplicate HANDLE is created. In that case you have to pass the HANDLE as a command line argument (or any other IPC method) to the child process

@MikailBag
Copy link

Ah, thanks, I see now. DuplicateHandle function does not support specyfing target handle.

However, I am also unsure if open_osfhandle will work with soft that uses winapi directly. For example, it looks like rust's libstd uses winapi and handles to perform IO, so rust programs will not "observe" duplicated handle.

@Srinivasa314
Copy link
Author

Ah, thanks, I see now. DuplicateHandle function does not support specyfing target handle.

However, I am also unsure if open_osfhandle will work with soft that uses winapi directly. For example, it looks like rust's libstd uses winapi and handles to perform IO, so rust programs will not "observe" duplicated handle.

_get_osfhandle can be used to get HANDLE from file descriptor.

@retep998
Copy link
Member

retep998 commented Jun 4, 2020

Please consider that in the future Rust on Windows may have targets that do not have the CRT (rust-lang/rust#58713). Therefore the implementation for this on Windows should not rely on the CRT and functions like _get_osfhandle existing.

@retep998
Copy link
Member

retep998 commented Jun 4, 2020

Here is an article from 2004 that describes how this technique works. What worries me is this line in particular:

Note: Apparently this method does not work under 64bit Windows Vista.

Is it only Vista? Is it Vista and newer? How exactly does it "not work"?

@Srinivasa314
Copy link
Author

Please consider that in the future Rust on Windows may have targets that do not have the CRT (rust-lang/rust#58713). Therefore the implementation for this on Windows should not rely on the CRT and functions like _get_osfhandle existing.

The child can get a HANDLE from fd without using get_osfhandle from CRT.The child can call GetStartUpInfo and get the HANDLEs from the lpreserved2 field.

@Srinivasa314
Copy link
Author

Here is an article from 2004 that describes how this technique works. What worries me is this line in particular:

Note: Apparently this method does not work under 64bit Windows Vista.

Is it only Vista? Is it Vista and newer? How exactly does it "not work"?

  • In the article, they send messages like "Hello" using lpreserved2, but the actual purpose of lpreserved2 is to send a struct containing an array of HANDLEs.
  • Windows Vista was released in 2006 so they may have used a beta version since the article was last updated in 2004.
  • I did the exact procedure in a library that I created and it worked in windows 10
  • Nodejs is supported for all versions of windows and they use that method (I saw libuv source code)

@the8472
Copy link
Member

the8472 commented Jun 5, 2020

Since the descriptors usually come from different abstractions such as File, Pipe or TcpStream what would be the signature of stdio()? (fds: &[Raw{Fd, Handle}]) or &[&dyn AsRaw{Fd, Handle}]?

@Srinivasa314
Copy link
Author

Since the descriptors usually come from different abstractions such as File, Pipe or TcpStream what would be the signature of stdio()? (fds: &[Raw{Fd, Handle}]) or &[&dyn AsRaw{Fd, Handle}]?

I modified the example and described the signature of the function in https://github.com/rust-lang/rfcs/pull/2939/files#diff-4066f02fd5a441062768cca1581d30b9

@the8472
Copy link
Member

the8472 commented Jun 6, 2020

I don't think it makes sense to center this around pipes. Yes, pipes are an important use-case of passing additional descriptors, but there are uses for passing other things to child processes, including regular files, memfds, handles to directories etc.

So neither the Pipe type nor the function name pipes() makes sense as it is overly specific.

@Srinivasa314
Copy link
Author

I don't think it makes sense to center this around pipes. Yes, pipes are an important use-case of passing additional descriptors, but there are uses for passing other things to child processes, including regular files, memfds, handles to directories etc.

So neither the Pipe type nor the function name pipes() makes sense as it is overly specific.

I didnt think about it. Now I changed it again. If you want to know why the flags are needed that is because you have to specify them in the StartupInfo struct in windows.

@retep998
Copy link
Member

retep998 commented Jun 6, 2020

As there is nothing essential to having this code in std, this should probably be implemented in a third party crate first. It would be easier to decide on this RFC when there is a working implementation available to be tested and used.

@Srinivasa314
Copy link
Author

Srinivasa314 commented Jun 6, 2020

As there is nothing essential to having this code in std, this should probably be implemented in a third party crate first. It would be easier to decide on this RFC when there is a working implementation available to be tested and used.

The reason I created this RFC is that a third party crate will have to duplicate a lot of code of std::process::Command. I will try to create a third party crate for testing purposes.

EDIT: I realised that I need to duplicate more code from rust std than I thought, so if you want to see a basic implementation of the procedure then you can check this file which I made for one of my crates. (The implementation present there is specific to my crate so for it to be in rust std it should be more generalized.)

@kennytm
Copy link
Member

kennytm commented Jun 6, 2020

we could restrict these fd-passing methods to std::os::unix::process::CommandExt if it is not clear whether it is applicable on Windows.

a third-party crate could wrap around pre_exec to perform the dup2.

@Srinivasa314
Copy link
Author

we could restrict these fd-passing methods to std::os::unix::process::CommandExt if it is not clear whether it is applicable on Windows.

a third-party crate could wrap around pre_exec to perform the dup2.

You can use fds on windows using the CRT or simulate their behaviour using StartupInfo.
A third party crate could use pre_exec in std::os::unix::process::CommandExt in unix but unfortunately there is no way to set startup info for std::os::windows::process::CommandExt.

@Srinivasa314
Copy link
Author

@retep998 It would be feasible to create a third party crate if std::os::windows::process::CommandExt supported modifying startupinfo or otherwise I have to use a patched version of this file in the third party crate.

For std::os::unix::process::CommandExt there is a pre_exec method in which the third party crate can dup2.

@KodrAus KodrAus added the Libs-Tracked Libs issues that are tracked on the team's project board. label Jul 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Libs-Tracked Libs issues that are tracked on the team's project board. T-libs-api Relevant to the library API team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants