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

Implement inheriting more than 3 file descriptors for UNIX Commands #17605

Closed
wants to merge 1 commit into from
Closed

Conversation

ipetkov
Copy link
Contributor

@ipetkov ipetkov commented Sep 28, 2014

  • This allows creating sub-processes through std::io::process::Command with file descriptors beyond stdio
  • Support is still missing under Windows

* This allows creating sub-processes through the
  std::io::process::Command struct with file descriptors beyond
  stdin/stdout/stderr
* Support is still missing under Windows
@rust-highfive
Copy link
Contributor

warning Warning warning

  • These commits modify unsafe code. Please review it carefully!

@thestinger
Copy link
Contributor

We really need to switch to posix_spawn rather than digging deeper into this hole. Along with performance simplicity, there are issues like #14232. It should likely all be thrown out and rewritten as part of the runtime rewrite.

@alexcrichton
Copy link
Member

I have investigated posix_spawn in the past because I agree I would love to use it! I could not, however, figure out how to change the working directory of the child process, which is a fairly critical aspect, so I was unable to make progress on that front. Do you know if it's possible to change the current directory of the child with posix_spawn?

@alexcrichton
Copy link
Member

@ipetkov thanks for the patch! I'll try to take a closer look tomorrow. I'm pretty wary of implementing support for a feature such as this while ignoring windows, especially without an expected frame of time to have it implemented. Would you be willing to implement the windows side of the API as well?

@ipetkov
Copy link
Contributor Author

ipetkov commented Sep 28, 2014

@alexcrichton I'd be willing to take a stab at implementing the Windows side of the API, though I don't have much experience with the WINAPI itself, and I'm not sure what is the correct way to go about implementing this.

After a few Google searches it looks like UpdateProcThreadAttribute function might be able to accomplish setting the child's file descriptors in the right order (though I'm not sure how it would work for leaving holes without opening handles to NUL), however, WinXP does not support this call...

Alternatively the files to inherit could be shuffled around via _dup/_dup2 etc., but without a direct call to fork() the file descriptors will be modified in the parent. Resetting them back to their original state before returning to the caller seems like a difficult problem to handle.

@alexcrichton
Copy link
Member

Most of my inspiration for these apis came from libuv's implementation, and you can also take a look at their windows implementation of more than 3 stdio file descriptors.

That being said, now that I look at it, it looks significantly complicated... We may be able to implement a somewhat stripped down version to get the tests passing and then we can add more features later on.

@alexcrichton
Copy link
Member

Closing out of inactivity, and I think that with the runtime removal this functionality is likely to just disappear as it's unimplemented entirely right now. It would of course be nice to add support for this, but we'll need to figure out a story for windows at the same time as well.

@ipetkov ipetkov deleted the extra-io branch December 26, 2014 21:38
lnicola pushed a commit to lnicola/rust that referenced this pull request Jul 28, 2024
Set `RUSTC_TOOLCHAIN` for runnables

With this the client doesn't necessarily need to guess the sysroot anymore
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants