Never panic in thread::park and thread::park_timeout

[joboet]

fixes #102398

@rustbot label +T-libs +T-libs-api

[rustbot]

Hey! It looks like you've submitted a new PR for the library teams!

If this PR contains changes to any rust-lang/rust public library APIs then please comment with @rustbot label +T-libs-api -T-libs to tag it appropriately. If this PR contains changes to any unstable APIs please edit the PR description to add a link to the relevant API Change Proposal or create one if you haven't already. If you're unsure where your change falls no worries, just leave it as is and the reviewer will take a look and make a decision to forward on if necessary.

Examples of T-libs-api changes:

• Stabilizing library features
• Introducing insta-stable changes such as new implementations of existing stable traits on existing stable types
• Introducing new or changing existing unstable library APIs (excluding permanently unstable features / features without a tracking issue)
• Changing public documentation in ways that create new stability guarantees
• Changing observable runtime behavior of library APIs

[rust-highfive]

r? @m-ou-se

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

[the8472]

Wouldn't not parking the thread if some underlying resource can't be acquired be a valid strategy? After all park is allowed to wake up any time, similar to condvars. So instead of panicing it would return immediately.

[m-ou-se]

@the8472 Yes, but if the error keeps occuring, that'd effectively turn a park-loop into a spin-loop, which is pretty bad.

[m-ou-se]

Changing the implementation to never panic looks good to me. Making it a documented guarantee should go through FCP though.

@rfcbot merge

This FCP is for documenting the guarantee that thread::park() (and thread::park_timeout()) will never panic. See #102398 for context.

[rfcbot]

Team member @m-ou-se has proposed to merge this. The next step is review by the rest of the tagged team members:

• @Amanieu
• @BurntSushi
• @cuviper
• @dtolnay
• @joshtriplett
• @m-ou-se
• @yaahc

No concerns currently listed.

Once a majority of reviewers approve (and at most 2 approvals are outstanding), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

[rfcbot]

🔔 This is now entering its final comment period, as per the review above. 🔔

[joboet]

Wouldn't not parking the thread if some underlying resource can't be acquired be a valid strategy? After all park is allowed to wake up any time, similar to condvars. So instead of panicing it would return immediately.

Yes, definitely, and this is still valid behaviour. However, since implementing that would impact so many different areas (TLS for instance), I doubt that it is really worth it, as errors should be extremely rare here.

[DemiMarie]

Wouldn't not parking the thread if some underlying resource can't be acquired be a valid strategy? After all park is allowed to wake up any time, similar to condvars. So instead of panicing it would return immediately.

Yes, definitely, and this is still valid behaviour. However, since implementing that would impact so many different areas (TLS for instance), I doubt that it is really worth it, as errors should be extremely rare here.

What about eagerly allocating the resources when creating the thread?

[rfcbot]

The final comment period, with a disposition to merge, as per the review above, is now complete.

As the automated representative of the governance process, I would like to thank the author for their work and everyone else who contributed.

This will be merged soon.

[m-ou-se]

What about eagerly allocating the resources when creating the thread?

That doesn't prevent all possible panics. We can't guarantee that e.g. the underlying libc calls always succeed. See #102398 (comment)

[m-ou-se]

@bors r+

[bors]

📌 Commit b0b9f5b has been approved by m-ou-se

It is now in the queue for this repository.