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

std: Add a nonblocking Child::try_wait method #38866

Merged
merged 1 commit into from
Jan 9, 2017

Conversation

alexcrichton
Copy link
Member

This commit adds a new method to the Child type in the std::process module
called try_wait. This method is the same as wait except that it will not
block the calling thread and instead only attempt to collect the exit status. On
Unix this means that we call waitpid with the WNOHANG flag and on Windows it
just means that we pass a 0 timeout to WaitForSingleObject.

Currently it's possible to build this method out of tree, but it's unfortunately
tricky to do so. Specifically on Unix you essentially lose ownership of the pid
for the process once a call to waitpid has succeeded. Although Child tracks
this state internally to be resilient to multiple calls to wait or a kill
after a successful wait, if the child is waited on externally then the state
inside of Child is not updated. This means that external implementations of
this method must be extra careful to essentially not use a Child's methods
after a call to waitpid has succeeded (even in a nonblocking fashion).

By adding this functionality to the standard library it should help canonicalize
these external implementations and ensure they can continue to robustly reuse
the Child type from the standard library without worrying about pid ownership.

@rust-highfive
Copy link
Collaborator

r? @aturon

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member Author

API-wise I was torn between returning io::Result<ExitStatus> and io::Result<Option<ExitStatus>>. I opted for the former as it's consistent with our "nonblocking" interfaces elsewhere, namely Read and Write for TCP streams and such.

@aturon
Copy link
Member

aturon commented Jan 6, 2017

@bors: r+

@bors
Copy link
Contributor

bors commented Jan 6, 2017

📌 Commit 0048bf3 has been approved by aturon

@aturon
Copy link
Member

aturon commented Jan 6, 2017

@bors: r-

Sorry, needs to have a tracking issue first. But this LGTM.

This commit adds a new method to the `Child` type in the `std::process` module
called `try_wait`. This method is the same as `wait` except that it will not
block the calling thread and instead only attempt to collect the exit status. On
Unix this means that we call `waitpid` with the `WNOHANG` flag and on Windows it
just means that we pass a 0 timeout to `WaitForSingleObject`.

Currently it's possible to build this method out of tree, but it's unfortunately
tricky to do so. Specifically on Unix you essentially lose ownership of the pid
for the process once a call to `waitpid` has succeeded. Although `Child` tracks
this state internally to be resilient to multiple calls to `wait` or a `kill`
after a successful wait, if the child is waited on externally then the state
inside of `Child` is not updated. This means that external implementations of
this method must be extra careful to essentially not use a `Child`'s methods
after a call to `waitpid` has succeeded (even in a nonblocking fashion).

By adding this functionality to the standard library it should help canonicalize
these external implementations and ensure they can continue to robustly reuse
the `Child` type from the standard library without worrying about pid ownership.
@alexcrichton
Copy link
Member Author

@bors: r=aturon

@bors
Copy link
Contributor

bors commented Jan 7, 2017

📌 Commit abb9189 has been approved by aturon

@bors
Copy link
Contributor

bors commented Jan 8, 2017

⌛ Testing commit abb9189 with merge 7e7a00c...

@bors
Copy link
Contributor

bors commented Jan 8, 2017

💔 Test failed - status-travis

@alexcrichton
Copy link
Member Author

alexcrichton commented Jan 9, 2017 via email

@bors
Copy link
Contributor

bors commented Jan 9, 2017

⌛ Testing commit abb9189 with merge 7aab3d3...

bors added a commit that referenced this pull request Jan 9, 2017
std: Add a nonblocking `Child::try_wait` method

This commit adds a new method to the `Child` type in the `std::process` module
called `try_wait`. This method is the same as `wait` except that it will not
block the calling thread and instead only attempt to collect the exit status. On
Unix this means that we call `waitpid` with the `WNOHANG` flag and on Windows it
just means that we pass a 0 timeout to `WaitForSingleObject`.

Currently it's possible to build this method out of tree, but it's unfortunately
tricky to do so. Specifically on Unix you essentially lose ownership of the pid
for the process once a call to `waitpid` has succeeded. Although `Child` tracks
this state internally to be resilient to multiple calls to `wait` or a `kill`
after a successful wait, if the child is waited on externally then the state
inside of `Child` is not updated. This means that external implementations of
this method must be extra careful to essentially not use a `Child`'s methods
after a call to `waitpid` has succeeded (even in a nonblocking fashion).

By adding this functionality to the standard library it should help canonicalize
these external implementations and ensure they can continue to robustly reuse
the `Child` type from the standard library without worrying about pid ownership.
@bors
Copy link
Contributor

bors commented Jan 9, 2017

☀️ Test successful - status-appveyor, status-travis
Approved by: aturon
Pushing 7aab3d3 to master...

@bors bors merged commit abb9189 into rust-lang:master Jan 9, 2017
@alexcrichton alexcrichton deleted the try-wait branch January 13, 2017 23:27
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