Skip to content

Disable debug_assert_not_in_new_nodes for multiple threads#152655

Merged
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
zetanumbers:disable_debug_assert_151509
Mar 1, 2026
Merged

Disable debug_assert_not_in_new_nodes for multiple threads#152655
rust-bors[bot] merged 1 commit intorust-lang:mainfrom
zetanumbers:disable_debug_assert_151509

Conversation

@zetanumbers
Copy link
Contributor

@zetanumbers zetanumbers commented Feb 15, 2026 •
edited by rustbot
Loading

Fixes #152654

This debug assertion is no longer intended to succeed for the parallel compiler since #151509 doesn't panic on a red-green query graph coloring race, generating both red and green nodes (#151509 (comment))

@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 Feb 15, 2026
@rustbot
Copy link
Collaborator

rustbot commented Feb 15, 2026

r? @nnethercote

rustbot has assigned @nnethercote.
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

Why was this reviewer chosen?

The reviewer was selected based on:

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

@nnethercote
Copy link
Contributor

nnethercote commented Feb 16, 2026

Please give some explanation of what this change is doing, and why. Even though it's a small change, just writing "Fixes #152654" isn't enough.

@nnethercote nnethercote 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-review Status: Awaiting review from the assignee but also interested parties. labels Feb 16, 2026
@Zoxc
Copy link
Contributor

Zoxc commented Feb 18, 2026

I think assert_dep_node_not_yet_allocated_in_current_session can race too. I'd just disable these by not setting the nodes_in_current_session field on multiple threads.

@Zoxc
Copy link
Contributor

Zoxc commented Feb 18, 2026

We could probably consider removing it too. I don't think it has much value anymore.

@zetanumbers
Copy link
Contributor Author

zetanumbers commented Feb 27, 2026

Please give some explanation of what this change is doing, and why. Even though it's a small change, just writing "Fixes #152654" isn't enough.

Updated the top comment. Is this good?

@nnethercote
Copy link
Contributor

nnethercote commented Mar 1, 2026

Updated the top comment. Is this good?

Yes, thank you.

I think assert_dep_node_not_yet_allocated_in_current_session can race too. I'd just disable these by not setting the nodes_in_current_session field on multiple threads.
We could probably consider removing it too. I don't think it has much value anymore.

This PR is enough to fix a known bug. To keep things moving I will approve it as is. Please open a follow-up PR implementing either of these suggestions if you like. The second option is more expansive -- there are multiple checks involving nodes_in_current_session -- so if you do that please make sure to explain clearly why removing it is valid. Thanks!

@bors r+ rollup

@rust-bors
Copy link
Contributor

rust-bors bot commented Mar 1, 2026

📌 Commit 9fb9225 has been approved by nnethercote

It is now in the queue for this repository.

@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-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Mar 1, 2026
matthiaskrgr added a commit to matthiaskrgr/rust that referenced this pull request Mar 1, 2026
@matthiaskrgr
Rollup merge of rust-lang#152655 - zetanumbers:disable_debug_assert_1…
358ae92 
…51509, r=nnethercote

Disable debug_assert_not_in_new_nodes for multiple threads

Fixes rust-lang#152654

This debug assertion is no longer intended to succeed for the parallel compiler since rust-lang#151509 doesn't panic on a red-green query graph coloring race, generating both red and green nodes  (rust-lang#151509 (comment))
This was referenced Mar 1, 2026
Closed
Merged
rust-bors bot pushed a commit that referenced this pull request Mar 1, 2026
@bors
Auto merge of #153252 - matthiaskrgr:rollup-uYxeyaV, r=matthiaskrgr
9c41836 
Rollup of 5 pull requests

Successful merges:

 - #152042 (Suggest async block instead of async closure when possible)
 - #152949 (Introduce --ci flag in tidy)
 - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads)
 - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`)
 - #153229 (rustfmt: add test for field representing type builtin syntax)
rust-bors bot pushed a commit that referenced this pull request Mar 1, 2026
@bors
Auto merge of #153252 - matthiaskrgr:rollup-uYxeyaV, r=matthiaskrgr
024757f 
Rollup of 5 pull requests

Successful merges:

 - #152042 (Suggest async block instead of async closure when possible)
 - #152949 (Introduce --ci flag in tidy)
 - #152655 (Disable debug_assert_not_in_new_nodes for multiple threads)
 - #153209 (Clean up `QueryVTable::hash_result` into `hash_value_fn`)
 - #153229 (rustfmt: add test for field representing type builtin syntax)
@rust-bors rust-bors bot merged commit fbbb35d into rust-lang:main Mar 1, 2026
11 checks passed
@rustbot rustbot added this to the 1.96.0 milestone Mar 1, 2026
rust-timer added a commit that referenced this pull request Mar 1, 2026
@rust-timer
Unrolled build for #152655
541933d 
Rollup merge of #152655 - zetanumbers:disable_debug_assert_151509, r=nnethercote

Disable debug_assert_not_in_new_nodes for multiple threads

Fixes #152654

This debug assertion is no longer intended to succeed for the parallel compiler since #151509 doesn't panic on a red-green query graph coloring race, generating both red and green nodes  (#151509 (comment))
@zetanumbers zetanumbers deleted the disable_debug_assert_151509 branch March 2, 2026 06:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

@nnethercote nnethercote

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

Milestone

1.96.0

Development

Successfully merging this pull request may close these issues.

[ICE]: node from previous graph present in new node collection

4 participants

@zetanumbers @rustbot @nnethercote @Zoxc