From 51001cf7efdf1b35ddb21ccb7f7659fefc1e5f74 Mon Sep 17 00:00:00 2001 From: xmakro Date: Tue, 2 Jun 2026 20:19:03 -0700 Subject: [PATCH 1/2] Make the retained dep graph deterministic under the parallel frontend The in-memory dep graph kept for `-Zquery-dep-graph` is built by `GraphEncoder::record`, which pushed each node using `try_lock` and silently dropped the node when the lock was contended. Single-threaded the only contention is a query forced re-entrantly from within `with_retained_dep_graph`, so dropping was harmless. Under the parallel frontend several encoder threads record nodes at the same time, so a contended `try_lock` dropped nodes and edges at random, leaving the retained graph nondeterministic and making the dep-graph ui tests, which assert on its contents, fail intermittently. Remove that reentrancy in `check_paths`: read the graph into owned results while the lock is held, then emit the diagnostics afterwards. The only query forced inside the old closure was `def_path_str` (for the error messages), which can create dep nodes and re-enter the encoder; doing the emission after the lock is released means `record` never re-enters while the lock is held. `record` can then block on the lock unconditionally instead of using `try_lock`, so concurrent encoders never drop a node or edge. This only affects compilation with `-Zquery-dep-graph`; with the flag off the retained graph is absent and nothing changes. --- .../src/graph/linked_graph/mod.rs | 4 +- .../rustc_incremental/src/assert_dep_graph.rs | 45 +++++++++++-------- compiler/rustc_middle/src/dep_graph/graph.rs | 10 +++-- .../rustc_middle/src/dep_graph/retained.rs | 1 + .../rustc_middle/src/dep_graph/serialized.rs | 16 +++---- 5 files changed, 45 insertions(+), 31 deletions(-) diff --git a/compiler/rustc_data_structures/src/graph/linked_graph/mod.rs b/compiler/rustc_data_structures/src/graph/linked_graph/mod.rs index 2223e85a24957..cecb051172598 100644 --- a/compiler/rustc_data_structures/src/graph/linked_graph/mod.rs +++ b/compiler/rustc_data_structures/src/graph/linked_graph/mod.rs @@ -45,17 +45,19 @@ mod tests; /// This graph implementation predates the later [graph traits](crate::graph), /// and does not implement those traits, so it has its own implementations of a /// few basic graph algorithms. +#[derive(Clone)] pub struct LinkedGraph { nodes: IndexVec>, edges: Vec>, } +#[derive(Clone)] pub struct Node { first_edge: [EdgeIndex; 2], // see module comment pub data: Option, } -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct Edge { next_edge: [EdgeIndex; 2], // see module comment source: NodeIndex, diff --git a/compiler/rustc_incremental/src/assert_dep_graph.rs b/compiler/rustc_incremental/src/assert_dep_graph.rs index 36439160b7d20..b3f9a137817a7 100644 --- a/compiler/rustc_incremental/src/assert_dep_graph.rs +++ b/compiler/rustc_incremental/src/assert_dep_graph.rs @@ -57,8 +57,14 @@ use crate::errors; #[allow(missing_docs)] pub(crate) fn assert_dep_graph(tcx: TyCtxt<'_>) { tcx.dep_graph.with_ignore(|| { + // Clone the retained dep graph once and share it between the graph dump and the path + // checks below, rather than locking and cloning it separately for each. + let retained_dep_graph = tcx.dep_graph.retained_dep_graph(); + if tcx.sess.opts.unstable_opts.dump_dep_graph { - tcx.dep_graph.with_retained_dep_graph(dump_graph); + if let Some(graph) = &retained_dep_graph { + dump_graph(graph); + } } if !tcx.sess.opts.unstable_opts.query_dep_graph { @@ -92,7 +98,7 @@ pub(crate) fn assert_dep_graph(tcx: TyCtxt<'_>) { } // Check paths. - check_paths(tcx, &if_this_changed, &then_this_would_need); + check_paths(tcx, retained_dep_graph.as_ref(), &if_this_changed, &then_this_would_need); }) } @@ -172,30 +178,33 @@ impl<'tcx> Visitor<'tcx> for IfThisChanged<'tcx> { } } -fn check_paths<'tcx>(tcx: TyCtxt<'tcx>, if_this_changed: &Sources, then_this_would_need: &Targets) { - // Return early here so as not to construct the query, which is not cheap. +fn check_paths<'tcx>( + tcx: TyCtxt<'tcx>, + retained_dep_graph: Option<&RetainedDepGraph>, + if_this_changed: &Sources, + then_this_would_need: &Targets, +) { if if_this_changed.is_empty() { for &(target_span, _, _, _) in then_this_would_need { tcx.dcx().emit_err(errors::MissingIfThisChanged { span: target_span }); } return; } - tcx.dep_graph.with_retained_dep_graph(|query| { - for &(_, source_def_id, ref source_dep_node) in if_this_changed { - let dependents = query.transitive_predecessors(source_dep_node); - for &(target_span, ref target_pass, _, ref target_dep_node) in then_this_would_need { - if !dependents.contains(&target_dep_node) { - tcx.dcx().emit_err(errors::NoPath { - span: target_span, - source: tcx.def_path_str(source_def_id), - target: *target_pass, - }); - } else { - tcx.dcx().emit_err(errors::Ok { span: target_span }); - } + let Some(query) = retained_dep_graph else { return }; + for &(_, source_def_id, ref source_dep_node) in if_this_changed { + let dependents = query.transitive_predecessors(source_dep_node); + for &(target_span, ref target_pass, _, ref target_dep_node) in then_this_would_need { + if !dependents.contains(&target_dep_node) { + tcx.dcx().emit_err(errors::NoPath { + span: target_span, + source: tcx.def_path_str(source_def_id), + target: *target_pass, + }); + } else { + tcx.dcx().emit_err(errors::Ok { span: target_span }); } } - }); + } } fn dump_graph(graph: &RetainedDepGraph) { diff --git a/compiler/rustc_middle/src/dep_graph/graph.rs b/compiler/rustc_middle/src/dep_graph/graph.rs index b5c32a1848aeb..fa408fd3f832b 100644 --- a/compiler/rustc_middle/src/dep_graph/graph.rs +++ b/compiler/rustc_middle/src/dep_graph/graph.rs @@ -195,10 +195,12 @@ impl DepGraph { self.data.is_some() } - pub fn with_retained_dep_graph(&self, f: impl Fn(&RetainedDepGraph)) { - if let Some(data) = &self.data { - data.current.encoder.with_retained_dep_graph(f) - } + /// Returns a clone of the in-memory retained dep graph, if it is being built + /// (i.e. `-Zquery-dep-graph` is set). Cloning rather than exposing the lock keeps + /// callers from holding it while forcing queries, which would deadlock against a + /// reentrant `record` under the parallel frontend. + pub fn retained_dep_graph(&self) -> Option { + self.data.as_ref().and_then(|data| data.current.encoder.retained_dep_graph()) } pub fn assert_ignored(&self) { diff --git a/compiler/rustc_middle/src/dep_graph/retained.rs b/compiler/rustc_middle/src/dep_graph/retained.rs index 626b3b7821794..7949a47346845 100644 --- a/compiler/rustc_middle/src/dep_graph/retained.rs +++ b/compiler/rustc_middle/src/dep_graph/retained.rs @@ -9,6 +9,7 @@ use super::{DepNode, DepNodeIndex}; /// Normally, dependencies recorded during the current session are written to /// disk and then forgotten, to avoid wasting memory on information that is /// not needed when the compiler is working correctly. +#[derive(Clone)] pub struct RetainedDepGraph { pub inner: LinkedGraph, pub indices: FxHashMap, diff --git a/compiler/rustc_middle/src/dep_graph/serialized.rs b/compiler/rustc_middle/src/dep_graph/serialized.rs index ef5e3d9268ad7..7db646e95bf19 100644 --- a/compiler/rustc_middle/src/dep_graph/serialized.rs +++ b/compiler/rustc_middle/src/dep_graph/serialized.rs @@ -636,10 +636,12 @@ impl EncoderState { // Outline the build of the full dep graph as it's typically disabled and cold. outline(move || { - // Do not ICE when a query is called from within `with_query`. - if let Some(retained_graph) = &mut retained_graph.try_lock() { - retained_graph.push(index, *node, &edges); - } + // Block on the lock rather than using `try_lock`: under the parallel frontend + // several threads record nodes concurrently, and dropping a node on lock + // contention would make the retained graph nondeterministic. Readers take a + // clone of the graph (`retained_dep_graph`) rather than holding the lock, so + // this never deadlocks against a reentrant `record`. + retained_graph.lock().push(index, *node, &edges); }); } @@ -874,10 +876,8 @@ impl GraphEncoder { GraphEncoder { status, retained_graph, profiler: sess.prof.clone() } } - pub(crate) fn with_retained_dep_graph(&self, f: impl Fn(&RetainedDepGraph)) { - if let Some(retained_graph) = &self.retained_graph { - f(&retained_graph.lock()) - } + pub(crate) fn retained_dep_graph(&self) -> Option { + self.retained_graph.as_ref().map(|retained_graph| retained_graph.lock().clone()) } /// Encodes a node that does not exists in the previous graph. From d00722c8507330d0a6633a57aef511dab4f24dc6 Mon Sep 17 00:00:00 2001 From: xmakro Date: Wed, 3 Jun 2026 08:50:15 -0700 Subject: [PATCH 2/2] Re-enable dep-graph ui tests under the parallel frontend These tests assert on the contents of the retained dep graph, which is now built deterministically under the parallel frontend, so they no longer need to be ignored there. Replace the ignore-parallel-frontend directives with blank lines rather than deleting them, so the following line numbers stay the same and the expected output is unchanged. --- tests/ui/dep-graph/dep-graph-assoc-type-codegen.rs | 2 +- tests/ui/dep-graph/dep-graph-caller-callee.rs | 2 +- tests/ui/dep-graph/dep-graph-struct-signature.rs | 2 +- .../ui/dep-graph/dep-graph-trait-impl-two-traits-same-method.rs | 2 +- tests/ui/dep-graph/dep-graph-trait-impl.rs | 2 +- tests/ui/dep-graph/dep-graph-type-alias.rs | 2 +- tests/ui/dep-graph/dep-graph-variance-alias.rs | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/ui/dep-graph/dep-graph-assoc-type-codegen.rs b/tests/ui/dep-graph/dep-graph-assoc-type-codegen.rs index 8b87499966de5..3c228d81e0e56 100644 --- a/tests/ui/dep-graph/dep-graph-assoc-type-codegen.rs +++ b/tests/ui/dep-graph/dep-graph-assoc-type-codegen.rs @@ -1,6 +1,6 @@ // Test that when a trait impl changes, fns whose body uses that trait // must also be recompiled. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph diff --git a/tests/ui/dep-graph/dep-graph-caller-callee.rs b/tests/ui/dep-graph/dep-graph-caller-callee.rs index 43d10cd57cdbb..c4bba8bb2d0b5 100644 --- a/tests/ui/dep-graph/dep-graph-caller-callee.rs +++ b/tests/ui/dep-graph/dep-graph-caller-callee.rs @@ -3,7 +3,7 @@ //@ incremental //@ compile-flags: -Z query-dep-graph -//@ ignore-parallel-frontend dep graph + #![feature(rustc_attrs)] #![allow(dead_code)] diff --git a/tests/ui/dep-graph/dep-graph-struct-signature.rs b/tests/ui/dep-graph/dep-graph-struct-signature.rs index abef7d2e98845..f00a22cf67cf0 100644 --- a/tests/ui/dep-graph/dep-graph-struct-signature.rs +++ b/tests/ui/dep-graph/dep-graph-struct-signature.rs @@ -1,6 +1,6 @@ // Test cases where a changing struct appears in the signature of fns // and methods. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph diff --git a/tests/ui/dep-graph/dep-graph-trait-impl-two-traits-same-method.rs b/tests/ui/dep-graph/dep-graph-trait-impl-two-traits-same-method.rs index ccdd2ff570f61..cdb74b9c97119 100644 --- a/tests/ui/dep-graph/dep-graph-trait-impl-two-traits-same-method.rs +++ b/tests/ui/dep-graph/dep-graph-trait-impl-two-traits-same-method.rs @@ -1,6 +1,6 @@ // Test that adding an impl to a trait `Foo` DOES affect functions // that only use `Bar` if they have methods in common. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph diff --git a/tests/ui/dep-graph/dep-graph-trait-impl.rs b/tests/ui/dep-graph/dep-graph-trait-impl.rs index 952ccf61c7d5e..a0aefba6e4658 100644 --- a/tests/ui/dep-graph/dep-graph-trait-impl.rs +++ b/tests/ui/dep-graph/dep-graph-trait-impl.rs @@ -1,6 +1,6 @@ // Test that when a trait impl changes, fns whose body uses that trait // must also be recompiled. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph diff --git a/tests/ui/dep-graph/dep-graph-type-alias.rs b/tests/ui/dep-graph/dep-graph-type-alias.rs index 8d0407a74ca86..a083f905b8ff9 100644 --- a/tests/ui/dep-graph/dep-graph-type-alias.rs +++ b/tests/ui/dep-graph/dep-graph-type-alias.rs @@ -1,5 +1,5 @@ // Test that changing what a `type` points to does not go unnoticed. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph diff --git a/tests/ui/dep-graph/dep-graph-variance-alias.rs b/tests/ui/dep-graph/dep-graph-variance-alias.rs index 5e86e77b24a27..8a67fe6d7271d 100644 --- a/tests/ui/dep-graph/dep-graph-variance-alias.rs +++ b/tests/ui/dep-graph/dep-graph-variance-alias.rs @@ -1,6 +1,6 @@ // Test that changing what a `type` points to does not go unnoticed // by the variance analysis. -//@ ignore-parallel-frontend dep graph + //@ incremental //@ compile-flags: -Z query-dep-graph