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.

Sign up for GitHub

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

Jump to bottom

Improve docs for is_running to explain use case #94033

Merged
merged 1 commit into from
Jun 21, 2022
Merged

Improve docs for is_running to explain use case #94033

merged 1 commit into from
Jun 21, 2022

Conversation

joshtriplett
Copy link
Member

No description provided.

@rust-highfive
Copy link
Collaborator

r? @m-ou-se

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Feb 15, 2022
@joshtriplett joshtriplett mentioned this pull request Feb 15, 2022
Closed
8 tasks
m-ou-se
m-ou-se reviewed Feb 16, 2022
View reviewed changes
library/std/src/thread/mod.rs Outdated
Comment on lines 1455 to 1456
/// However, if this returns `false`, `join` can be expected to return
/// relatively quickly, and not block for any significant amount of time.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a thought: we could 'fix' this by just always pthread_detaching every spawned thread directly, and only using our own return value Arc for 'join'ing threads. (This is also how I implemented scoped threads.) Then we can make sure that join will instantly return with no possibility of blocking as soon as the Arc is ready.

@m-ou-se m-ou-se mentioned this pull request Mar 3, 2022
Merged
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
db22fbb 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
24cedc2 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
962f4aa 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
c6f5e74 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
418a386 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
6dbeb02 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2022
@matthiaskrgr
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
5c0a1b6 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 4, 2022
@matthiaskrgr
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
ee3a2c7 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
Dylan-DPC added a commit to Dylan-DPC/rust that referenced this pull request Mar 4, 2022
@Dylan-DPC
Rollup merge of rust-lang#94549 - m-ou-se:thread-is-finished, r=yaahc
34ae383 
Rename JoinHandle::is_running to is_finished.

This is renaming `is_running` to `is_finished` as discussed on the tracking issue here: rust-lang#90470 (comment)

Taking some of the docs suggestions from rust-lang#94033
@bors
Copy link
Contributor

bors commented Mar 4, 2022

☔ The latest upstream changes (presumably #94612) made this pull request unmergeable. Please resolve the merge conflicts.

@m-ou-se m-ou-se added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Mar 9, 2022
@dtolnay dtolnay changed the title Improve docs for is_running to explain use case and discourage busy-pollling Improve docs for is_running to explain use case and discourage busy-polling Mar 18, 2022
@apiraino apiraino added the T-libs Relevant to the library team, which will review and decide on the PR/issue. label May 19, 2022
@joshtriplett joshtriplett force-pushed the documentation-is-running-better-go-catch-it branch from 06bddb5 to 7098a71 Compare June 20, 2022 20:42
@joshtriplett joshtriplett changed the title Improve docs for is_running to explain use case and discourage busy-polling Improve docs for is_running to explain use case Jun 20, 2022
@joshtriplett
Copy link
Member Author

I've rebased this to eliminate the merge conflicts.

I think there's value in explaining the use case for this, to go along with the now-present explanation of when to use join instead.

@rustbot ready

@joshtriplett joshtriplett added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Jun 20, 2022
@m-ou-se
Copy link
Member

m-ou-se commented Jun 21, 2022

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 21, 2022

📌 Commit 7098a71 has been approved by m-ou-se

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 21, 2022
@JohnTitor JohnTitor mentioned this pull request Jun 21, 2022
Merged
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 21, 2022
@bors
Auto merge of rust-lang#98335 - JohnTitor:rollup-j2zudxv, r=JohnTitor
72fd41a 
Rollup of 11 pull requests

Successful merges:

 - rust-lang#94033 (Improve docs for `is_running` to explain use case)
 - rust-lang#97269 (adjust transmute const stabilization version)
 - rust-lang#97805 (Add proper tracing spans to rustc_trait_selection::traits::error_reporting)
 - rust-lang#98022 (Fix erroneous span for borrowck error)
 - rust-lang#98124 (Improve loading of crates.js and sidebar-items.js)
 - rust-lang#98278 (Some token stream cleanups)
 - rust-lang#98306 (`try_fold_unevaluated` for infallible folders)
 - rust-lang#98313 (Remove lies in comments.)
 - rust-lang#98323 (:arrow_up: rust-analyzer)
 - rust-lang#98329 (Avoid an ICE and instead let the compiler report a useful error)
 - rust-lang#98330 (update ioslice docs to use shared slices)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 84c17c2 into rust-lang:master Jun 21, 2022
@rustbot rustbot added this to the 1.63.0 milestone Jun 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m-ou-se m-ou-se m-ou-se left review comments

Assignees

@m-ou-se m-ou-se

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs Relevant to the library team, which will review and decide on the PR/issue.
Projects
None yet
Milestone
1.63.0
Development

Successfully merging this pull request may close these issues.
6 participants
@joshtriplett @rust-highfive @bors @m-ou-se @apiraino @rustbot