Skip to content

Conversation

@joboet
Copy link
Member

@joboet joboet commented Nov 9, 2025

Almost all functionality in std::thread is currently implemented in thread/mod.rs, resulting in a huge file with more than 2000 lines and multiple, interoperating unsafe sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining unsafe interplay is that of the lifecycle and scope modules, the spawn_scoped implementation relies on the live thread count being updated correctly by the lifecycle module.

This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions:

  • with_current_name is moved to the already existing current module and now uses the name method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway.
  • The private JoinInner type used to implement both join handles now has some more methods (is_finished, thread and the AsInner/IntoInner implementations) to avoid having to expose private fields and their invariants.
  • The private spawn_unchecked_ (note the underscore) method of Builder is now a freestanding function in lifecycle.

The rest of the changes are just visibility annotations.

I realise this PR ended up quite large – let me know if there is anyway I can aid the review process.

Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating mod.rs. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.

@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 9, 2025
@rustbot
Copy link
Collaborator

rustbot commented Nov 9, 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

@rust-log-analyzer

This comment has been minimized.

@rustbot
Copy link
Collaborator

rustbot commented Nov 9, 2025

The Miri subtree was changed

cc @rust-lang/miri

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

@rust-log-analyzer

This comment has been minimized.

Copy link
Member

Choose a reason for hiding this comment

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

Would be good to have some module-level docs for this, the name isn't exactly self-explaining.

@ChrisDenton
Copy link
Member

ChrisDenton commented Nov 10, 2025

Hm, I'm going to nominate this for @rust-lang/libs. Not because it particularly needs discussion but just for awareness. If anyone has anything they'd like to change/add about this PR I'd prefer they speak up now then have more churn after this is merged.

I've not finished considering it yet but my initial thought is that it seems to have swung a bit too heavily from (almost) everything in one file to (almost) everything in separate files. For example, JoinHandle is essentially a small shim that simply forwards to the underlying implementation and the only unsafe is the Send and Sync impls. Builder is somewhat similarly trivial.

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

Amanieu commented Nov 19, 2025

This was discussed in @rust-lang/libs meeting last week. We're OK with this change, though some people had concerns that some of the logic was split across too many files.

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

I was convinced that this is ok. Well, I'm still a bit nervous about some of the smaller files but I'm willing to see how it goes.

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 26, 2025

📌 Commit 693055f has been approved by ChrisDenton

It is now in the queue for this repository.

@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 Nov 26, 2025
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 26, 2025
std: split up the `thread` module

Almost all functionality in `std::thread` is currently implemented in `thread/mod.rs`, resulting in a *huge* file with more than 2000 lines and multiple, interoperating `unsafe` sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining `unsafe` interplay is that of the `lifecycle` and `scope` modules, the `spawn_scoped` implementation relies on the live thread count being updated correctly by the `lifecycle` module.

This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions:
* `with_current_name` is moved to the already existing `current` module and now uses the `name` method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway.
* The private `JoinInner` type used to implement both join handles now has some more methods (`is_finished`, `thread` and the `AsInner`/`IntoInner` implementations) to avoid having to expose private fields and their invariants.
* The private `spawn_unchecked_` (note the underscore) method of `Builder` is now a freestanding function in `lifecycle`.

The rest of the changes are just visibility annotations.

I realise this PR ended up quite large – let me know if there is anyway I can aid the review process.

Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating `mod.rs`. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 26, 2025
std: split up the `thread` module

Almost all functionality in `std::thread` is currently implemented in `thread/mod.rs`, resulting in a *huge* file with more than 2000 lines and multiple, interoperating `unsafe` sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining `unsafe` interplay is that of the `lifecycle` and `scope` modules, the `spawn_scoped` implementation relies on the live thread count being updated correctly by the `lifecycle` module.

This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions:
* `with_current_name` is moved to the already existing `current` module and now uses the `name` method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway.
* The private `JoinInner` type used to implement both join handles now has some more methods (`is_finished`, `thread` and the `AsInner`/`IntoInner` implementations) to avoid having to expose private fields and their invariants.
* The private `spawn_unchecked_` (note the underscore) method of `Builder` is now a freestanding function in `lifecycle`.

The rest of the changes are just visibility annotations.

I realise this PR ended up quite large – let me know if there is anyway I can aid the review process.

Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating `mod.rs`. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.
Zalathar added a commit to Zalathar/rust that referenced this pull request Nov 26, 2025
std: split up the `thread` module

Almost all functionality in `std::thread` is currently implemented in `thread/mod.rs`, resulting in a *huge* file with more than 2000 lines and multiple, interoperating `unsafe` sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining `unsafe` interplay is that of the `lifecycle` and `scope` modules, the `spawn_scoped` implementation relies on the live thread count being updated correctly by the `lifecycle` module.

This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions:
* `with_current_name` is moved to the already existing `current` module and now uses the `name` method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway.
* The private `JoinInner` type used to implement both join handles now has some more methods (`is_finished`, `thread` and the `AsInner`/`IntoInner` implementations) to avoid having to expose private fields and their invariants.
* The private `spawn_unchecked_` (note the underscore) method of `Builder` is now a freestanding function in `lifecycle`.

The rest of the changes are just visibility annotations.

I realise this PR ended up quite large – let me know if there is anyway I can aid the review process.

Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating `mod.rs`. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.
bors added a commit that referenced this pull request Nov 26, 2025
Rollup of 19 pull requests

Successful merges:

 - #148048 (Stabilize `maybe_uninit_write_slice`)
 - #148641 (Add a diagnostic attribute for special casing const bound errors for non-const impls)
 - #148765 (std: split up the `thread` module)
 - #149074 (Add Command::get_env_clear)
 - #149097 (num: Implement `uint_gather_scatter_bits` feature for unsigned integers)
 - #149131 (optimize `slice::Iter::next_chunk`)
 - #149190 (Forbid `CHECK: br` and `CHECK-NOT: br` in codegen tests (suggest `br {{.*}}` instead))
 - #149239 (clarify float min/max behavios for NaNs and signed zeros)
 - #149243 (Fix typo and clarify bootstrap change tracker entry)
 - #149270 (implement `Iterator::{exactly_one, collect_array}`)
 - #149295 (Suggest _bytes versions of endian-converting methods)
 - #149301 (Motor OS: make decode_error_kind more comprehensive)
 - #149306 (bootstrap: Miri now handles jemalloc like everything else)
 - #149325 (rustdoc: add regression test for #140968)
 - #149332 (fix rustdoc search says “Consider searching for "null" instead.” #149324)
 - #149349 (Fix typo in comment.)
 - #149353 (Tidying up UI tests [3/N])
 - #149355 (Document that `build.description` affects symbol mangling and crate IDs)
 - #149360 (Enable CI download for windows-gnullvm)

r? `@ghost`
`@rustbot` modify labels: rollup
@Zalathar
Copy link
Member

Failed in rollup in aarch64-msvc-1: #149361 (comment)

Unfortunately, some of these internal module paths show up in debuginfo tests that need adjustment.

@bors r-

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 27, 2025
@joboet
Copy link
Member Author

joboet commented Nov 28, 2025

@bors r=@ChrisDenton

@bors
Copy link
Collaborator

bors commented Nov 28, 2025

📌 Commit 17bca49 has been approved by ChrisDenton

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 28, 2025
@joboet
Copy link
Member Author

joboet commented Nov 29, 2025

@bors r-
I need to rebase first...

@bors bors 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-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. labels Nov 29, 2025
@rustbot

This comment was marked as outdated.

@joboet
Copy link
Member Author

joboet commented Nov 29, 2025

Rebase on top of #144465.
I made a mistake while rebasing, here is the correct diff.

@rustbot ready

@rustbot rustbot 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 Nov 29, 2025
@ChrisDenton
Copy link
Member

@bors r+

@bors
Copy link
Collaborator

bors commented Nov 29, 2025

📌 Commit c22b819 has been approved by ChrisDenton

It is now in the queue for this repository.

@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 Nov 29, 2025
bors added a commit that referenced this pull request Nov 29, 2025
Rollup of 3 pull requests

Successful merges:

 - #148746 (const validation: remove check for mutable refs in final value of const)
 - #148765 (std: split up the `thread` module)
 - #149454 (resolve: Identifier resolution refactorings)

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit c93801c into rust-lang:main Nov 29, 2025
11 checks passed
@rustbot rustbot added this to the 1.93.0 milestone Nov 29, 2025
rust-timer added a commit that referenced this pull request Nov 29, 2025
Rollup merge of #148765 - joboet:split-up-thread, r=ChrisDenton

std: split up the `thread` module

Almost all functionality in `std::thread` is currently implemented in `thread/mod.rs`, resulting in a *huge* file with more than 2000 lines and multiple, interoperating `unsafe` sections. This PR splits the file up into multiple different private modules, each implementing mostly independent parts of the functionality. The only remaining `unsafe` interplay is that of the `lifecycle` and `scope` modules, the `spawn_scoped` implementation relies on the live thread count being updated correctly by the `lifecycle` module.

This PR contains no functional changes and only moves code around for the most part, with a few notable exceptions:
* `with_current_name` is moved to the already existing `current` module and now uses the `name` method instead of calculating the name from private fields. The old code was just a reimplementation of that method anyway.
* The private `JoinInner` type used to implement both join handles now has some more methods (`is_finished`, `thread` and the `AsInner`/`IntoInner` implementations) to avoid having to expose private fields and their invariants.
* The private `spawn_unchecked_` (note the underscore) method of `Builder` is now a freestanding function in `lifecycle`.

The rest of the changes are just visibility annotations.

I realise this PR ended up quite large – let me know if there is anyway I can aid the review process.

Edit: I've simplified the diff by adding an intermediate commit that creates all the new files by duplicating `mod.rs`. The actual changes in the second commit thus appear to delete the non-relevant parts from the respective file.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

8 participants