Skip to content

Avoid leaking the query-job collection warning into the panic query stack#157351

Open
xmakro wants to merge 2 commits into
rust-lang:mainfrom
xmakro:fix/parallel-collect-jobs-warn
Open

Avoid leaking the query-job collection warning into the panic query stack#157351
xmakro wants to merge 2 commits into
rust-lang:mainfrom
xmakro:fix/parallel-collect-jobs-warn

Conversation

@xmakro
Copy link
Copy Markdown

@xmakro xmakro commented Jun 3, 2026

Part of #154314.

When the compiler panics it prints the active query stack. That collection runs with CollectActiveJobsKind::PartialAllowed, which deliberately skips any query state shard whose lock it cannot take without waiting, since a complete job map is not needed just to print a stack.

Under the parallel front-end another thread can still hold a shard lock while the panic is being reported, so a shard is skipped nondeterministically and a warn!("Failed to collect active jobs ...") was printed. Because warnings are shown by default (the default RUSTC_LOG filter is WARN), this leaked an extra line into the diagnostics of panicking compilations and made their output differ run to run. The panicking thread's own query chain is always collectible (a query does not hold its shard lock while it runs), so the printed stack itself is unaffected; only the spurious warning varied.

A skipped shard is expected and tolerated on this path, so warn! was the wrong level for it to begin with. This lowers the message to debug! so it stays available with RUSTC_LOG but no longer pollutes the default output, and re-enables the one ui test that was disabled because of it.

`collect_active_query_jobs` with `CollectActiveJobsKind::PartialAllowed`
is only used to print the query stack when the compiler panics. It
intentionally skips any query state shard whose lock it cannot take
without waiting, since a complete job map is not needed for that.

Under the parallel front-end another thread can still hold a shard lock
while the panic is being reported, so the skip happens nondeterministically
and the `warn!` was printed into the panic output. Because warnings are
shown by default, this leaked a "Failed to collect active jobs" line into
the diagnostics of panicking compilations and made their output unstable.

Lower the message to `debug!` so it stays available with `RUSTC_LOG` but
no longer pollutes the default output.
@rustbot rustbot added A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. labels Jun 3, 2026
@xmakro xmakro marked this pull request as ready for review June 3, 2026 02:52
@rustbot rustbot added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 3, 2026
@rustbot rustbot removed the S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. label Jun 3, 2026
@rustbot
Copy link
Copy Markdown
Collaborator

rustbot commented Jun 3, 2026

Thanks for the pull request, and welcome! The Rust team is excited to review your changes, and you should hear from @JohnTitor (or someone else) some time within the next two weeks.

Please see the contribution instructions for more information. Namely, in order to ensure the minimum review times lag, PR authors and assigned reviewers should ensure that the review label (S-waiting-on-review and S-waiting-on-author) stays updated, invoking these commands when appropriate:

  • @rustbot author: the review is finished, PR author should check the comments and take action accordingly
  • @rustbot review: the author is ready for a review, this PR will be queued again in the reviewer's queue
Why was this reviewer chosen?

The reviewer was selected based on:

  • Owners of files modified in this PR: compiler, query-system
  • compiler, query-system expanded to 73 candidates
  • Random selection from 17 candidates

@petrochenkov petrochenkov self-assigned this Jun 3, 2026
…nt-end

This test was marked ignore-parallel-frontend because the panic-time query
stack collection could nondeterministically print a "Failed to collect active
jobs" warning. With that warning lowered to debug! the ICE output is stable
across runs, so replace the directive with a blank line rather than deleting
it. The expected stderr is unchanged because the line numbers stay the same.
@xmakro xmakro force-pushed the fix/parallel-collect-jobs-warn branch from f2efaa9 to b6a2d84 Compare June 3, 2026 15:54
@petrochenkov
Copy link
Copy Markdown
Contributor

@bors r+ rollup

@rust-bors
Copy link
Copy Markdown
Contributor

rust-bors Bot commented Jun 3, 2026

📌 Commit b6a2d84 has been approved by petrochenkov

It is now in the queue for this repository.

🌲 The tree is currently closed for pull requests below priority 5. This pull request will be tested once the tree is reopened.

@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 Jun 3, 2026
JonathanBrouwer added a commit to JonathanBrouwer/rust that referenced this pull request Jun 3, 2026
…arn, r=petrochenkov

Avoid leaking the query-job collection warning into the panic query stack

Part of rust-lang#154314.

When the compiler panics it prints the active query stack. That collection runs with `CollectActiveJobsKind::PartialAllowed`, which deliberately skips any query state shard whose lock it cannot take without waiting, since a complete job map is not needed just to print a stack.

Under the parallel front-end another thread can still hold a shard lock while the panic is being reported, so a shard is skipped nondeterministically and a `warn!("Failed to collect active jobs ...")` was printed. Because warnings are shown by default (the default `RUSTC_LOG` filter is `WARN`), this leaked an extra line into the diagnostics of panicking compilations and made their output differ run to run. The panicking thread's own query chain is always collectible (a query does not hold its shard lock while it runs), so the printed stack itself is unaffected; only the spurious warning varied.

A skipped shard is expected and tolerated on this path, so `warn!` was the wrong level for it to begin with. This lowers the message to `debug!` so it stays available with `RUSTC_LOG` but no longer pollutes the default output, and re-enables the one ui test that was disabled because of it.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-query-system Area: The rustc query system (https://rustc-dev-guide.rust-lang.org/query.html) 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