Skip to content

Conversation

@RalfJung
Copy link
Member

@RalfJung RalfJung commented Nov 30, 2025

Fixes #116237 by documenting the current behavior regarding thread-local destructors as intended. (I'm not stoked about this, but documenting it is better than leaving it unclear.)

This also adds documentation for explicit join calls (both for scoped and regular threads), saying that those will wait for TLS destructors. That reflects my understanding of the current implementation, which calls join on the native thread handle. Are we okay with guaranteeing that? I think we should, so people have at least some chance of implementing "wait for all destructors" manually. This fixes #127571.

Cc @rust-lang/libs-api

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Nov 30, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 30, 2025

r? @ChrisDenton

rustbot has assigned @ChrisDenton.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

Copy link
Member

@joboet joboet left a comment

Choose a reason for hiding this comment

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

As far as I know, this truly matches current behaviour. I really like that this is phrased in a way that still leaves the possibility of fixing this in the future.

View changes since this review

@ChrisDenton
Copy link
Member

This looks correct to me based on the current implementation and previous discussion. But it does probably warrant a libs-api sign-off so I'll nominate it.

It is not great that implicit joins can't currently offer the same guarantees as explicit ones so we do want to keep the door open to future improvements. On the flip side, this does close off the possibility of providing weaker guarantees for explicit joins in the future but I'd consider that a very good thing. I'd always want some way to wait until a thread is fully finished running.

@ChrisDenton ChrisDenton added the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Nov 30, 2025
@Amanieu
Copy link
Member

Amanieu commented Dec 2, 2025

@rfcbot merge libs-api

@Amanieu Amanieu removed the I-libs-api-nominated Nominated for discussion during a libs-api team meeting. label Dec 2, 2025
@rust-rfcbot
Copy link
Collaborator

rust-rfcbot commented Dec 2, 2025

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

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.

@rust-rfcbot rust-rfcbot added proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. labels Dec 2, 2025
@ChrisDenton
Copy link
Member

Just to be clear the libs-api FCP is specifically for guaranteeing that an explicit join will always fully wait for the thread to finish (including TLS destructors and whatever else).

@RalfJung
Copy link
Member Author

@BurntSushi @dtolnay @joshtriplett @the8472 friendly checkbox reminder. :)

@dtolnay dtolnay added T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. and removed T-libs Relevant to the library team, which will review and decide on the PR/issue. labels Dec 13, 2025
@RalfJung
Copy link
Member Author

@BurntSushi @joshtriplett @the8472 it's been 6 weeks since FCP was proposed -- are there any concerns that need resolving here?

@rust-rfcbot rust-rfcbot added final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. and removed proposed-final-comment-period Proposed to merge/close by relevant subteam, see T-<team> label. Will enter FCP once signed off. labels Jan 19, 2026
@rust-rfcbot
Copy link
Collaborator

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

@rust-rfcbot rust-rfcbot added finished-final-comment-period The final comment period is finished for this PR / Issue. to-announce Announce this issue on triage meeting and removed final-comment-period In the final comment period and will be merged soon unless new substantive objections are raised. labels Jan 29, 2026
@rust-rfcbot
Copy link
Collaborator

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.

@RalfJung
Copy link
Member Author

@bors r=joboet rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Jan 29, 2026

📌 Commit 1deb487 has been approved by joboet

It is now in the queue for this repository.

@rust-bors rust-bors bot added the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Jan 29, 2026
@rust-bors rust-bors bot removed the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jan 29, 2026
rust-bors bot pushed a commit that referenced this pull request Jan 29, 2026
…uwer

Rollup of 8 pull requests

Successful merges:

 - #147387 (hir_owner_parent optimized to inlined call for non-incremental build)
 - #150271 (Move struct placeholder pt2)
 - #151283 (Suggest ignore returning value inside macro for unused_must_use lint)
 - #151565 (Rename, clarify, and document code for "erasing" query values)
 - #149482 (thread::scope: document how join interacts with TLS destructors)
 - #151827 (Use `Rustc` prefix for `rustc` attrs in `AttributeKind`)
 - #151833 (Treat unions as 'data types' in attr parsing diagnostics)
 - #151834 (Update `askama` version to `0.15.4`)
@rust-bors rust-bors bot merged commit faa0d67 into rust-lang:main Jan 29, 2026
11 checks passed
@rustbot rustbot added this to the 1.95.0 milestone Jan 29, 2026
rust-timer added a commit that referenced this pull request Jan 29, 2026
Rollup merge of #149482 - RalfJung:scope-tls-dtors, r=joboet

thread::scope: document how join interacts with TLS destructors

Fixes #116237 by documenting the current behavior regarding thread-local destructors as intended. (I'm not stoked about this, but documenting it is better than leaving it unclear.)

This also adds documentation for explicit `join` calls (both for scoped and regular threads), saying that those *will* wait for TLS destructors. That reflects my understanding of the current implementation, which calls `join` on the native thread handle. Are we okay with guaranteeing that? I think we should, so people have at least some chance of implementing "wait for all destructors" manually. This fixes #127571.

Cc @rust-lang/libs-api
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

disposition-merge This issue / PR is in PFCP or FCP with a disposition to merge it. finished-final-comment-period The final comment period is finished for this PR / Issue. S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. T-libs-api Relevant to the library API team, which will review and decide on the PR/issue. to-announce Announce this issue on triage meeting

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Guarantee that thread-local storage destructors run before join() returns Scoped thread implicit join doesn't wait for thread locals to be dropped

7 participants