-
Notifications
You must be signed in to change notification settings - Fork 13k
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
Add JoinHandle::is_running. #90439
Add JoinHandle::is_running. #90439
Conversation
(rust-highfive has picked a reviewer for you, use r? to override) |
45e5c0f
to
c5f9193
Compare
It feels like instead of a is_running() if, it might be better to add a try_join, like we have https://doc.rust-lang.org/nightly/std/process/struct.Child.html#method.try_wait? That would avoid both essentially checking twice and potentially "missing" the result in some sense since it combines the check with actually getting the results. The described use case at least sounds like it would benefit from try_join rather than is_running, though it's likely to be a small difference in practice. |
c5f9193
to
67362b3
Compare
.join() doesn't do any checks that .is_running() does (it doesn't even check the refcount), so there's no duplicate check that can be avoided. And there's no way to "miss" the result, since the only way to get the result is to .join() which consumes the (only, non-clonable) JoinHandle. I could add a try_join() that just does It can still be useful to have |
The signature of try_join() would also be an interesting question, since it has to consume Self when it joins, but leave it alone when it doesn't join. |
Wouldn't a |
Then as a user I'd need to provide a waker/context to be able to poll the thread.
You're referring to the code that runs on the thread after its main function returns? There's no portable way to check if a thread is really 'done'*. E.g. pthread doesn't provide anything for that. The only way would be to detach the thread and never join it at all. (*For various definitions of 'done'.) |
Hm. OK. I think we could do something like a try_take with basically https://doc.rust-lang.org/nightly/std/sync/struct.Arc.html#method.get_mut, which would let us try_get_result or something. I think this implementation is reasonable enough for now, and we can consider alternatives as part of stabilization; it's not clear to me that there's anything too much better given the limitations of the underlying interfaces. It's definitely the case that we expect a very short wait after is_running is false in general -- it should just be the system cleanup for the most part. We could in theory pull out the result value of the thread within the is_running if strong_count = 1, but the user (or we) would still need to call the inner join() in order to not leak the thread resources, so it's not trivially done I suspect. Maybe something to consider in the future. r=me with the tracking issue filled in |
@bors r=Mark-Simulacrum |
📌 Commit 978ebd9 has been approved by |
Wouldn't detaching the thread after pulling out the result work on platforms that don't have a non-blocking join? |
…imulacrum Add JoinHandle::is_running. This adds: ```rust impl<T> JoinHandle<T> { /// Checks if the the associated thread is still running its main function. /// /// This might return `false` for a brief moment after the thread's main /// function has returned, but before the thread itself has stopped running. pub fn is_running(&self) -> bool; } ``` The usual way to check if a background thread is still running is to set some atomic flag at the end of its main function. We already do that, in the form of dropping an Arc which will reduce the reference counter. So we might as well expose that information. This is useful in applications with a main loop (e.g. a game, gui, control system, ..) where you spawn some background task, and check every frame/iteration whether the background task is finished to .join() it in that frame/iteration while keeping the program responsive.
…imulacrum Add JoinHandle::is_running. This adds: ```rust impl<T> JoinHandle<T> { /// Checks if the the associated thread is still running its main function. /// /// This might return `false` for a brief moment after the thread's main /// function has returned, but before the thread itself has stopped running. pub fn is_running(&self) -> bool; } ``` The usual way to check if a background thread is still running is to set some atomic flag at the end of its main function. We already do that, in the form of dropping an Arc which will reduce the reference counter. So we might as well expose that information. This is useful in applications with a main loop (e.g. a game, gui, control system, ..) where you spawn some background task, and check every frame/iteration whether the background task is finished to .join() it in that frame/iteration while keeping the program responsive.
…imulacrum Add JoinHandle::is_running. This adds: ```rust impl<T> JoinHandle<T> { /// Checks if the the associated thread is still running its main function. /// /// This might return `false` for a brief moment after the thread's main /// function has returned, but before the thread itself has stopped running. pub fn is_running(&self) -> bool; } ``` The usual way to check if a background thread is still running is to set some atomic flag at the end of its main function. We already do that, in the form of dropping an Arc which will reduce the reference counter. So we might as well expose that information. This is useful in applications with a main loop (e.g. a game, gui, control system, ..) where you spawn some background task, and check every frame/iteration whether the background task is finished to .join() it in that frame/iteration while keeping the program responsive.
☀️ Test successful - checks-actions |
Finished benchmarking commit (6384dca): comparison url. Summary: This benchmark run did not return any relevant changes. If you disagree with this performance assessment, please file an issue in rust-lang/rustc-perf. @rustbot label: -perf-regression |
// Joining the thread should not block for a significant time now. | ||
let join_time = Instant::now(); | ||
assert_eq!(t.join().unwrap(), 1234); | ||
assert!(join_time.elapsed() < Duration::from_secs(2)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this relaible? Maybe Duration::from_secs(2 + 1.0) better?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would three seconds be more reliable than two?
This adds:
The usual way to check if a background thread is still running is to set some atomic flag at the end of its main function. We already do that, in the form of dropping an Arc which will reduce the reference counter. So we might as well expose that information.
This is useful in applications with a main loop (e.g. a game, gui, control system, ..) where you spawn some background task, and check every frame/iteration whether the background task is finished to .join() it in that frame/iteration while keeping the program responsive.