Allow process::Child to be used with external calls to waitpid
The wait and wait_with_output methods of process::Child expect to do the waiting for the child to exit themselves. If you are running many processes at once, and you don't know which one will finish first, this is inconvenient, because the system calls wait and waitpid consume each exit notification as they report it. (waitid can be told not to do that, but it is not fully supported in the libc crate yet.) That places the Child object in an inconsistent state, in which it is not safe to call wait or wait_with_output.
If Child had a set_exit_status method, an application in this situation could use it to keep the objects consistent: something like
fn wait_all(children: &[&mut Child]) -> nix::Result<()> {
use nix::sys::wait::{wait, WaitStatus}
let mut pending : HashMap<u32, &mut Child> =
children.iter().map(|c| (c.id(), c)).collect();
while pending.len() {
let status = try!(wait());
match status {
Exited(pid, ..) | Signaled(pid, ..) => {
let mut child = try!(pending.remove(pid).ok_or(UnexpectedChild(pid)));
child.set_exit_status(status);
},
_ => unreachable!();
}
}
Ok(())
}
(In this hypothetical, ExitStatus impls From<nix::WaitStatus>, which I believe nix can make happen without assistance from std.)
It's not clear to me that std itself can do much here. Ideally, we'd probably document this somewhere, though I'm not sure where we could do that either. I'm inclined to close because of this, but asking @rust-lang/libs for opinions on the matter.
Mark Rousskov at 2017-05-20 01:50:52
I know I've personally run into this a bunch of times, and @zackw's suggestion of adding a method to libstd seems plausible to me!
Alex Crichton at 2017-05-20 02:07:58
@Mark-Simulacrum I don't understand why you say "it's not clear to me that std itself can do much"? I'm only asking for the addition of
Child.set_exit_status. Only the stdlib can do that, and that is enough for the stdlib to do. It is somewhat awkward, admittedly, but it is enough for the likes of tokio to build a clean API around.Zack Weinberg at 2017-05-22 23:57:31
I have no idea really what I was saying there... yeah, this seems quite reasonable. I think the best approach here would be to put up a PR implementing this and then we can discuss there and hopefully merge. I don't see much that is controversial here (though not personally hugely familiar with the topic).
Mark Rousskov at 2017-05-23 00:02:04
I agree, I think we would be prepared to consider a PR.
David Tolnay at 2017-11-18 20:20:30
Triage: i don't believe a PR was ever sent.
Steve Klabnik at 2019-12-10 14:34:34
Yeah, this fell off the bottom of my todo list. I can try to find time in the next couple weeks to send a PR if there’s still interest.
On Tue, Dec 10, 2019 at 9:34 AM Steve Klabnik notifications@github.com wrote:
Triage: i don't believe a PR was ever sent.
— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/rust-lang/rust/issues/39188?email_source=notifications&email_token=AACPSC7J37EJ6QL5CCLVSCLQX6SHXA5CNFSM4C5BP6T2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGPOA2A#issuecomment-564060264, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACPSC5PXUYGZTYEIWHQS5DQX6SHXANCNFSM4C5BP6TQ .
Zack Weinberg at 2019-12-10 16:41:31