Skip to content

Explicitly drop job server helper thread - #165854

Merged
rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
aerooneqq:explicitly-drop-helper-thread
Nov 12, 2026
Merged

rust-bors[bot] merged 1 commit into
rust-lang:mainfrom
aerooneqq:explicitly-drop-helper-thread

Conversation

@aerooneqq

@aerooneqq aerooneqq commented Nov 12, 2026 •

Copy link
Copy Markdown
Contributor

An adaptation of an LLM patch that was generated in https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/with/639482582.

The original patch consisted from two fixes: explicit drop of a helper thread and additional logic for job server token request counting. This PR implements only the explicit drop part as I don't see how additional requests counting logic should help the issue.

This reworks #165676, as the rationale behind explicit drop of a helper thread seems reasonable for me:

From my rough understanding, while helper is handling a token, its upgrade() is often the last strong ref I guess. So, when run_in_thread_pool_with_globals returns and rustc exits, the token may still be in helper's hands.

This fix didn't fail after several launches in #165506 and locally.

r? @petrochenkov

@rustbot rustbot added S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Nov 12, 2026
@aerooneqq
aerooneqq force-pushed the explicitly-drop-helper-thread branch from 01a2027 to bb1742b Compare November 12, 2026 11:06
@petrochenkov

Copy link
Copy Markdown
Contributor

@bors r+

@rust-bors

rust-bors Bot commented Nov 12, 2026

Copy link
Copy Markdown
Contributor

📌 Commit bb1742b has been tentatively approved by petrochenkov

It will be put into the queue for this repository once PR CI succeeds.

@rust-bors rust-bors Bot 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 12, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 12, 2026
…hread, r=petrochenkov

Explicitly drop job server helper thread

An adaptation of an LLM patch that was generated in https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/with/639482582.

The original patch consisted from two fixes: explicit drop of a helper thread and additional logic for job server token request counting. This PR implements only the explicit drop part as I don't see how additional requests counting logic should help the issue.

This reworks rust-lang#165676, as the [rationale](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/near/639152114) behind explicit drop of a helper thread seems reasonable for me:

> From my rough understanding, while helper is handling a token, its upgrade() is often the last strong ref I guess. So, when run_in_thread_pool_with_globals returns and rustc exits, the token may still be in helper's hands.

This fix didn't fail after several launches in rust-lang#165506 and locally.

r? @petrochenkov
rust-bors Bot pushed a commit that referenced this pull request Nov 12, 2026
…uwer

Rollup of 5 pull requests

Successful merges:

 - #165854 (Explicitly drop job server helper thread)
 - #164077 (replace `TypingEnv::post_analysis()` with `mir::Body::typing_env()`)
 - #165305 (hexagon: pass small aggregates on the stack when registers run out)
 - #165417 (Mention width limit panic in fmt doc)
 - #165565 (view-types: lower view types from HIR)
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Nov 12, 2026
…hread, r=petrochenkov

Explicitly drop job server helper thread

An adaptation of an LLM patch that was generated in https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/with/639482582.

The original patch consisted from two fixes: explicit drop of a helper thread and additional logic for job server token request counting. This PR implements only the explicit drop part as I don't see how additional requests counting logic should help the issue.

This reworks rust-lang#165676, as the [rationale](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/near/639152114) behind explicit drop of a helper thread seems reasonable for me:

> From my rough understanding, while helper is handling a token, its upgrade() is often the last strong ref I guess. So, when run_in_thread_pool_with_globals returns and rustc exits, the token may still be in helper's hands.

This fix didn't fail after several launches in rust-lang#165506 and locally.

r? @petrochenkov
rust-bors Bot pushed a commit that referenced this pull request Nov 12, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #161922 (Tie extern "llvm-intrinsics" and llvm.* symbol names together)
 - #164864 (Avoid hoisting uninitialized reads in EarlyOtherwiseBranch)
 - #165854 (Explicitly drop job server helper thread)
 - #164077 (replace `TypingEnv::post_analysis()` with `mir::Body::typing_env()`)
 - #165305 (hexagon: pass small aggregates on the stack when registers run out)
 - #165417 (Mention width limit panic in fmt doc)
 - #165860 (Fix inverted wasm cfg in core benchmarks)
rust-bors Bot pushed a commit that referenced this pull request Nov 12, 2026
…uwer

Rollup of 7 pull requests

Successful merges:

 - #161922 (Tie extern "llvm-intrinsics" and llvm.* symbol names together)
 - #165854 (Explicitly drop job server helper thread)
 - #164077 (replace `TypingEnv::post_analysis()` with `mir::Body::typing_env()`)
 - #165305 (hexagon: pass small aggregates on the stack when registers run out)
 - #165417 (Mention width limit panic in fmt doc)
 - #165860 (Fix inverted wasm cfg in core benchmarks)
 - #165871 (Cleanup: Remove `name` from `AttributeExt`)
rust-bors Bot pushed a commit that referenced this pull request Nov 12, 2026
Rollup merge of #165854 - aerooneqq:explicitly-drop-helper-thread, r=petrochenkov

Explicitly drop job server helper thread

An adaptation of an LLM patch that was generated in https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/with/639482582.

The original patch consisted from two fixes: explicit drop of a helper thread and additional logic for job server token request counting. This PR implements only the explicit drop part as I don't see how additional requests counting logic should help the issue.

This reworks #165676, as the [rationale](https://rust-lang.zulipchat.com/#narrow/channel/246057-t-cargo/topic/Interaction.20of.20Cargo.20jobserver.20tests.20and.20parallel.20frontend/near/639152114) behind explicit drop of a helper thread seems reasonable for me:

> From my rough understanding, while helper is handling a token, its upgrade() is often the last strong ref I guess. So, when run_in_thread_pool_with_globals returns and rustc exits, the token may still be in helper's hands.

This fix didn't fail after several launches in #165506 and locally.

r? @petrochenkov
@rust-bors
rust-bors Bot merged commit 438355c into rust-lang:main Nov 12, 2026
14 checks passed
@rustbot rustbot added this to the 1.102.0 milestone Nov 12, 2026
@weihanglo

Copy link
Copy Markdown
Member

Thanks. Just what to call out that the patch was from @rarick after his carefully review :)

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-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants