From d7a23523d6000d6bae00b19b5f2651ec8ce0233a Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 6 Feb 2026 19:26:15 +0300 Subject: [PATCH 1/9] Replace beack_query_cycles implementation with new --- compiler/rustc_middle/src/query/job.rs | 4 +- compiler/rustc_query_impl/src/execution.rs | 5 +- compiler/rustc_query_impl/src/job.rs | 346 +++++---------------- 3 files changed, 87 insertions(+), 268 deletions(-) diff --git a/compiler/rustc_middle/src/query/job.rs b/compiler/rustc_middle/src/query/job.rs index f1a2b3a34d0e8..57eefee64593f 100644 --- a/compiler/rustc_middle/src/query/job.rs +++ b/compiler/rustc_middle/src/query/job.rs @@ -73,7 +73,6 @@ impl<'tcx> QueryJob<'tcx> { pub struct QueryWaiter<'tcx> { pub query: Option, pub condvar: Condvar, - pub span: Span, pub cycle: Mutex>>>, } @@ -100,10 +99,9 @@ impl<'tcx> QueryLatch<'tcx> { &self, tcx: TyCtxt<'tcx>, query: Option, - span: Span, ) -> Result<(), CycleError>> { let waiter = - Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() }); + Arc::new(QueryWaiter { query, cycle: Mutex::new(None), condvar: Condvar::new() }); self.wait_on_inner(tcx, &waiter); // FIXME: Get rid of this lock. We have ownership of the QueryWaiter // although another thread may still have a Arc reference so we cannot diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 53afcacb63a6c..1fb502148d7a3 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -228,7 +228,6 @@ fn cycle_error<'tcx, C: QueryCache>( fn wait_for_query<'tcx, C: QueryCache>( query: &'tcx QueryVTable<'tcx, C>, tcx: TyCtxt<'tcx>, - span: Span, key: C::Key, latch: QueryLatch<'tcx>, current: Option, @@ -240,7 +239,7 @@ fn wait_for_query<'tcx, C: QueryCache>( // With parallel queries we might just have to wait on some other // thread. - let result = latch.wait_on(tcx, current, span); + let result = latch.wait_on(tcx, current); match result { Ok(()) => { @@ -320,7 +319,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>( // Only call `wait_for_query` if we're using a Rayon thread pool // as it will attempt to mark the worker thread as blocked. - return wait_for_query(query, tcx, span, key, latch, current_job_id); + return wait_for_query(query, tcx, key, latch, current_job_id); } let id = job.id; diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 2d9824a783ea5..c054aaf2bc73e 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -1,18 +1,15 @@ use std::io::Write; -use std::iter; -use std::ops::ControlFlow; -use std::sync::Arc; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_data_structures::fx::FxHashMap; +use rustc_data_structures::indexmap::{self, IndexMap}; use rustc_errors::{Diag, DiagCtxtHandle}; use rustc_hir::def::DefKind; use rustc_middle::query::{ - CycleError, QueryInfo, QueryJob, QueryJobId, QueryLatch, QueryStackDeferred, QueryStackFrame, - QueryWaiter, + CycleError, QueryInfo, QueryJob, QueryJobId, QueryStackDeferred, QueryStackFrame, }; use rustc_middle::ty::TyCtxt; use rustc_session::Session; -use rustc_span::{DUMMY_SP, Span}; +use rustc_span::Span; use crate::plumbing::collect_active_jobs_from_all_queries; @@ -34,18 +31,6 @@ impl<'tcx> QueryJobMap<'tcx> { fn frame_of(&self, id: QueryJobId) -> &QueryStackFrame> { &self.map[&id].frame } - - fn span_of(&self, id: QueryJobId) -> Span { - self.map[&id].job.span - } - - fn parent_of(&self, id: QueryJobId) -> Option { - self.map[&id].job.parent - } - - fn latch_of(&self, id: QueryJobId) -> Option<&QueryLatch<'tcx>> { - self.map[&id].job.latch.as_ref() - } } #[derive(Clone, Debug)] @@ -113,262 +98,99 @@ pub(crate) fn find_dep_kind_root<'tcx>( last_layout } -/// A resumable waiter of a query. The usize is the index into waiters in the query's latch -type Waiter = (QueryJobId, usize); - -/// Visits all the non-resumable and resumable waiters of a query. -/// Only waiters in a query are visited. -/// `visit` is called for every waiter and is passed a query waiting on `query` -/// and a span indicating the reason the query waited on `query`. -/// If `visit` returns `Break`, this function also returns `Break`, -/// and if all `visit` calls returns `Continue` it also returns `Continue`. -/// For visits of non-resumable waiters it returns the return value of `visit`. -/// For visits of resumable waiters it returns information required to resume that waiter. -fn visit_waiters<'tcx>( - job_map: &QueryJobMap<'tcx>, - query: QueryJobId, - mut visit: impl FnMut(Span, QueryJobId) -> ControlFlow>, -) -> ControlFlow> { - // Visit the parent query which is a non-resumable waiter since it's on the same stack - if let Some(parent) = job_map.parent_of(query) { - visit(job_map.span_of(query), parent)?; - } - - // Visit the explicit waiters which use condvars and are resumable - if let Some(latch) = job_map.latch_of(query) { - for (i, waiter) in latch.info.lock().waiters.iter().enumerate() { - if let Some(waiter_query) = waiter.query { - // Return a value which indicates that this waiter can be resumed - visit(waiter.span, waiter_query).map_break(|_| Some((query, i)))?; - } +/// Breaks cycle on some query. +/// +/// This function uses ordered depth-first search from a single root query down to the first +/// duplicate query. +/// It doesn't distinguish between a query wait and a query execution, so both are just query calls. +/// As such some queries may have two or more parent query calls too. +#[allow(rustc::potential_query_instability)] +pub fn break_query_cycles<'tcx>( + query_map: QueryJobMap<'tcx>, + registry: &rustc_thread_pool::Registry, +) { + let mut root_query = None; + for (&query, info) in &query_map.map { + if info.job.parent.is_none() { + assert!(root_query.is_none(), "found multiple threads without start"); + root_query = Some(query); } } + let root_query = root_query.expect("no root query was found"); - ControlFlow::Continue(()) -} - -/// Look for query cycles by doing a depth first search starting at `query`. -/// `span` is the reason for the `query` to execute. This is initially DUMMY_SP. -/// If a cycle is detected, this initial value is replaced with the span causing -/// the cycle. -fn cycle_check<'tcx>( - job_map: &QueryJobMap<'tcx>, - query: QueryJobId, - span: Span, - stack: &mut Vec<(Span, QueryJobId)>, - visited: &mut FxHashSet, -) -> ControlFlow> { - if !visited.insert(query) { - return if let Some(p) = stack.iter().position(|q| q.1 == query) { - // We detected a query cycle, fix up the initial span and return Some - - // Remove previous stack entries - stack.drain(0..p); - // Replace the span for the first query with the cycle cause - stack[0].0 = span; - ControlFlow::Break(None) - } else { - ControlFlow::Continue(()) + let mut subqueries = FxHashMap::<_, Vec<_>>::default(); + for query in query_map.map.values() { + let Some(parent) = query.job.parent else { + continue; }; + subqueries.entry(parent).or_default().push((query.job.id, usize::MAX)); } - // Query marked as visited is added it to the stack - stack.push((span, query)); - - // Visit all the waiters - let r = visit_waiters(job_map, query, |span, successor| { - cycle_check(job_map, successor, span, stack, visited) - }); - - // Remove the entry in our stack if we didn't find a cycle - if r.is_continue() { - stack.pop(); - } - - r -} - -/// Finds out if there's a path to the compiler root (aka. code which isn't in a query) -/// from `query` without going through any of the queries in `visited`. -/// This is achieved with a depth first search. -fn connected_to_root<'tcx>( - job_map: &QueryJobMap<'tcx>, - query: QueryJobId, - visited: &mut FxHashSet, -) -> ControlFlow> { - // We already visited this or we're deliberately ignoring it - if !visited.insert(query) { - return ControlFlow::Continue(()); - } - - // This query is connected to the root (it has no query parent), return true - if job_map.parent_of(query).is_none() { - return ControlFlow::Break(None); - } - - visit_waiters(job_map, query, |_, successor| connected_to_root(job_map, successor, visited)) -} - -/// Looks for query cycles starting from the last query in `jobs`. -/// If a cycle is found, all queries in the cycle is removed from `jobs` and -/// the function return true. -/// If a cycle was not found, the starting query is removed from `jobs` and -/// the function returns false. -fn remove_cycle<'tcx>( - job_map: &QueryJobMap<'tcx>, - jobs: &mut Vec, - wakelist: &mut Vec>>, -) -> bool { - let mut visited = FxHashSet::default(); - let mut stack = Vec::new(); - // Look for a cycle starting with the last query in `jobs` - if let ControlFlow::Break(waiter) = - cycle_check(job_map, jobs.pop().unwrap(), DUMMY_SP, &mut stack, &mut visited) - { - // The stack is a vector of pairs of spans and queries; reverse it so that - // the earlier entries require later entries - let (mut spans, queries): (Vec<_>, Vec<_>) = stack.into_iter().rev().unzip(); - - // Shift the spans so that queries are matched with the span for their waitee - spans.rotate_right(1); - - // Zip them back together - let mut stack: Vec<_> = iter::zip(spans, queries).collect(); - - // Remove the queries in our cycle from the list of jobs to look at - for r in &stack { - if let Some(pos) = jobs.iter().position(|j| j == &r.1) { - jobs.remove(pos); - } - } - - struct EntryPoint { - query_in_cycle: QueryJobId, - waiter: Option<(Span, QueryJobId)>, - } - - // Find the queries in the cycle which are - // connected to queries outside the cycle - let entry_points = stack - .iter() - .filter_map(|&(_, query_in_cycle)| { - if job_map.parent_of(query_in_cycle).is_none() { - // This query is connected to the root (it has no query parent) - Some(EntryPoint { query_in_cycle, waiter: None }) - } else { - let mut waiter_on_cycle = None; - // Find a direct waiter who leads to the root - let _ = visit_waiters(job_map, query_in_cycle, |span, waiter| { - // Mark all the other queries in the cycle as already visited - let mut visited = FxHashSet::from_iter(stack.iter().map(|q| q.1)); - - if connected_to_root(job_map, waiter, &mut visited).is_break() { - waiter_on_cycle = Some((span, waiter)); - ControlFlow::Break(None) - } else { - ControlFlow::Continue(()) - } - }); - - waiter_on_cycle.map(|waiter_on_cycle| EntryPoint { - query_in_cycle, - waiter: Some(waiter_on_cycle), - }) - } - }) - .collect::>(); - - // Pick an entry point, preferring ones with waiters - let entry_point = entry_points - .iter() - .find(|entry_point| entry_point.waiter.is_some()) - .unwrap_or(&entry_points[0]); - - // Shift the stack so that our entry point is first - let entry_point_pos = - stack.iter().position(|(_, query)| *query == entry_point.query_in_cycle); - if let Some(pos) = entry_point_pos { - stack.rotate_left(pos); - } - - let usage = entry_point.waiter.map(|(span, job)| (span, job_map.frame_of(job).clone())); - - // Create the cycle error - let error = CycleError { - usage, - cycle: stack - .iter() - .map(|&(span, job)| QueryInfo { span, frame: job_map.frame_of(job).clone() }) - .collect(), + for query in query_map.map.values() { + let Some(latch) = &query.job.latch else { + continue; }; - - // We unwrap `waiter` here since there must always be one - // edge which is resumable / waited using a query latch - let (waitee_query, waiter_idx) = waiter.unwrap(); - - // Extract the waiter we want to resume - let waiter = job_map.latch_of(waitee_query).unwrap().extract_waiter(waiter_idx); - - // Set the cycle error so it will be picked up when resumed - *waiter.cycle.lock() = Some(error); - - // Put the waiter on the list of things to resume - wakelist.push(waiter); - - true - } else { - false - } -} - -/// Detects query cycles by using depth first search over all active query jobs. -/// If a query cycle is found it will break the cycle by finding an edge which -/// uses a query latch and then resuming that waiter. -/// There may be multiple cycles involved in a deadlock, so this searches -/// all active queries for cycles before finally resuming all the waiters at once. -pub fn break_query_cycles<'tcx>( - job_map: QueryJobMap<'tcx>, - registry: &rustc_thread_pool::Registry, -) { - let mut wakelist = Vec::new(); - // It is OK per the comments: - // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798854932 - // - https://github.com/rust-lang/rust/pull/131200#issuecomment-2798866392 - #[allow(rustc::potential_query_instability)] - let mut jobs: Vec = job_map.map.keys().copied().collect(); - - let mut found_cycle = false; - - while jobs.len() > 0 { - if remove_cycle(&job_map, &mut jobs, &mut wakelist) { - found_cycle = true; + // Latch mutexes should be at least about to unlock as we do not hold it anywhere too long + let lock = latch.info.lock(); + assert!(!lock.complete); + for (waiter_idx, waiter) in lock.waiters.iter().enumerate() { + let waited_on_query = waiter.query.expect("cannot wait on a root query"); + subqueries.entry(waited_on_query).or_default().push((query.job.id, waiter_idx)); } } - // Check that a cycle was found. It is possible for a deadlock to occur without - // a query cycle if a query which can be waited on uses Rayon to do multithreading - // internally. Such a query (X) may be executing on 2 threads (A and B) and A may - // wait using Rayon on B. Rayon may then switch to executing another query (Y) - // which in turn will wait on X causing a deadlock. We have a false dependency from - // X to Y due to Rayon waiting and a true dependency from Y to X. The algorithm here - // only considers the true dependency and won't detect a cycle. - if !found_cycle { - panic!( - "deadlock detected as we're unable to find a query cycle to break\n\ - current query map:\n{job_map:#?}", - ); + let mut visited = IndexMap::new(); + let mut last_usage = None; + let mut last_waiter_idx = usize::MAX; + let mut current = root_query; + while let indexmap::map::Entry::Vacant(entry) = visited.entry(current) { + entry.insert((last_usage, last_waiter_idx)); + last_usage = Some(current); + (current, last_waiter_idx) = subqueries.get(¤t).unwrap_or_else(|| { + panic!( + "deadlock detected as we're unable to find a query cycle to break\n\ + current query map:\n{:#?}", + query_map + ) + })[0]; } - - // Mark all the thread we're about to wake up as unblocked. This needs to be done before - // we wake the threads up as otherwise Rayon could detect a deadlock if a thread we - // resumed fell asleep and this thread had yet to mark the remaining threads as unblocked. - for _ in 0..wakelist.len() { - rustc_thread_pool::mark_unblocked(registry); + let usage = visited[¤t].0; + let mut iter = visited.keys().rev(); + let mut cycle = Vec::new(); + loop { + let query_id = *iter.next().unwrap(); + let query = &query_map.map[&query_id]; + cycle.push(QueryInfo { span: query.job.span, frame: query.frame.clone() }); + if query_id == current { + break; + } } - for waiter in wakelist.into_iter() { - waiter.condvar.notify_one(); - } + cycle.reverse(); + let cycle_error = CycleError { + usage: usage.map(|id| { + let query = &query_map.map[&id]; + (query.job.span, query.frame.clone()) + }), + cycle, + }; + + let (waited_on, waiter_idx) = if last_waiter_idx != usize::MAX { + (current, last_waiter_idx) + } else { + let (&waited_on, &(_, waiter_idx)) = + visited.iter().rev().find(|(_, (_, waiter_idx))| *waiter_idx != usize::MAX).unwrap(); + (waited_on, waiter_idx) + }; + let waited_on = &query_map.map[&waited_on]; + let latch = waited_on.job.latch.as_ref().unwrap(); + let mut latch_info_lock = latch.info.try_lock().unwrap(); + let waiter = latch_info_lock.waiters.remove(waiter_idx); + let mut cycle_lock = waiter.cycle.try_lock().unwrap(); + assert!(cycle_lock.is_none()); + *cycle_lock = Some(cycle_error); + rustc_thread_pool::mark_unblocked(registry); + waiter.condvar.notify_one(); } pub fn print_query_stack<'tcx>( From f7862e4bf66dc8f6899637224d28306dddb6363c Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 27 Feb 2026 19:57:30 +0300 Subject: [PATCH 2/9] Write down theorems new cycle breaking depends on --- compiler/rustc_query_impl/src/job.rs | 68 +++++++++++++++++++--------- 1 file changed, 46 insertions(+), 22 deletions(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index c054aaf2bc73e..8d499a24c83d0 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -100,10 +100,11 @@ pub(crate) fn find_dep_kind_root<'tcx>( /// Breaks cycle on some query. /// -/// This function uses ordered depth-first search from a single root query down to the first -/// duplicate query. -/// It doesn't distinguish between a query wait and a query execution, so both are just query calls. +/// This function doesn't distinguish between a query wait and a query execution, so both are just +/// query calls. /// As such some queries may have two or more parent query calls too. +/// It uses depth-first search from a single root query down to the first duplicate query, +/// establishing a cycle. #[allow(rustc::potential_query_instability)] pub fn break_query_cycles<'tcx>( query_map: QueryJobMap<'tcx>, @@ -118,12 +119,20 @@ pub fn break_query_cycles<'tcx>( } let root_query = root_query.expect("no root query was found"); - let mut subqueries = FxHashMap::<_, Vec<_>>::default(); + // We are allowed to keep track of just one subquery since each query has at least one subquery. + // + // If we would assume the opposite then thread of query with no subqueries cannot wait on any + // subquery. That thread neither can wait on a running parallel task in functions like + // `par_join`, `par_slice` as the thread executing this parallel task must be blocked too since + // we are in a deadlock. Rustc only tracks these two cases of blocking code to trigger a + // deadlock so our assumption has to be false. + let mut subqueries = FxHashMap::default(); for query in query_map.map.values() { let Some(parent) = query.job.parent else { continue; }; - subqueries.entry(parent).or_default().push((query.job.id, usize::MAX)); + // We are safe to only track a single subquery due to the statement above + subqueries.entry(parent).or_insert((query.job.id, usize::MAX)); } for query in query_map.map.values() { @@ -135,10 +144,30 @@ pub fn break_query_cycles<'tcx>( assert!(!lock.complete); for (waiter_idx, waiter) in lock.waiters.iter().enumerate() { let waited_on_query = waiter.query.expect("cannot wait on a root query"); - subqueries.entry(waited_on_query).or_default().push((query.job.id, waiter_idx)); + // We are safe to only track a single subquery due to the statement above + subqueries.entry(waited_on_query).or_insert((query.job.id, waiter_idx)); } } + // Debug check the statement above + if cfg!(debug_assertions) { + for query in query_map.map.values() { + assert!(subqueries.contains_key(&query.job.id)); + } + } + + // At least one thread waits on the first duplicate query in depth-first search stack. + // Consider this stack of subqueries: + // + // ```text + // a() -> b() -> c() -> b() + // ``` + // + // In order for this statement to be false, both occurences of `b()` only be query executions. + // Only a single query executes a subquery, so parents of these occurences of `b()` have to be + // the same query, aka `a()` and `c()` are equal. + // However that means `b()` is not the first duplicate query in the stack, + // so the original statement must be true. let mut visited = IndexMap::new(); let mut last_usage = None; let mut last_waiter_idx = usize::MAX; @@ -146,13 +175,7 @@ pub fn break_query_cycles<'tcx>( while let indexmap::map::Entry::Vacant(entry) = visited.entry(current) { entry.insert((last_usage, last_waiter_idx)); last_usage = Some(current); - (current, last_waiter_idx) = subqueries.get(¤t).unwrap_or_else(|| { - panic!( - "deadlock detected as we're unable to find a query cycle to break\n\ - current query map:\n{:#?}", - query_map - ) - })[0]; + (current, last_waiter_idx) = subqueries[¤t]; } let usage = visited[¤t].0; let mut iter = visited.keys().rev(); @@ -175,17 +198,18 @@ pub fn break_query_cycles<'tcx>( cycle, }; - let (waited_on, waiter_idx) = if last_waiter_idx != usize::MAX { - (current, last_waiter_idx) - } else { - let (&waited_on, &(_, waiter_idx)) = - visited.iter().rev().find(|(_, (_, waiter_idx))| *waiter_idx != usize::MAX).unwrap(); - (waited_on, waiter_idx) - }; - let waited_on = &query_map.map[&waited_on]; + // Per statement above we should have wait at either of two occurences of the duplicate query + if last_waiter_idx == usize::MAX { + last_waiter_idx = visited.get(¤t).unwrap().1; + } + + let waited_on = &query_map.map[¤t]; let latch = waited_on.job.latch.as_ref().unwrap(); let mut latch_info_lock = latch.info.try_lock().unwrap(); - let waiter = latch_info_lock.waiters.remove(waiter_idx); + + // And so this `Vec::remove` shouldn't cause a panic + let waiter = latch_info_lock.waiters.remove(last_waiter_idx); + let mut cycle_lock = waiter.cycle.try_lock().unwrap(); assert!(cycle_lock.is_none()); *cycle_lock = Some(cycle_error); From 9a3400d19d7194f69a9a778b260676f4fdbdb0ff Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Fri, 27 Feb 2026 20:08:00 +0300 Subject: [PATCH 3/9] Fix typo --- compiler/rustc_query_impl/src/job.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 8d499a24c83d0..08a39b96c7f68 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -163,8 +163,8 @@ pub fn break_query_cycles<'tcx>( // a() -> b() -> c() -> b() // ``` // - // In order for this statement to be false, both occurences of `b()` only be query executions. - // Only a single query executes a subquery, so parents of these occurences of `b()` have to be + // In order for this statement to be false, both occurrences of `b()` only be query executions. + // Only a single query executes a subquery, so parents of these occurrences of `b()` have to be // the same query, aka `a()` and `c()` are equal. // However that means `b()` is not the first duplicate query in the stack, // so the original statement must be true. @@ -198,7 +198,7 @@ pub fn break_query_cycles<'tcx>( cycle, }; - // Per statement above we should have wait at either of two occurences of the duplicate query + // Per statement above we should have wait at either of two occurrences of the duplicate query if last_waiter_idx == usize::MAX { last_waiter_idx = visited.get(¤t).unwrap().1; } From 253b3c16a71676ff1fe54fd6a1846d7e33dace96 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Sat, 28 Feb 2026 09:21:39 +0300 Subject: [PATCH 4/9] Restore `QueryWaiter::span` field --- compiler/rustc_middle/src/query/job.rs | 4 +++- compiler/rustc_query_impl/src/execution.rs | 5 +++-- 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/compiler/rustc_middle/src/query/job.rs b/compiler/rustc_middle/src/query/job.rs index 57eefee64593f..f1a2b3a34d0e8 100644 --- a/compiler/rustc_middle/src/query/job.rs +++ b/compiler/rustc_middle/src/query/job.rs @@ -73,6 +73,7 @@ impl<'tcx> QueryJob<'tcx> { pub struct QueryWaiter<'tcx> { pub query: Option, pub condvar: Condvar, + pub span: Span, pub cycle: Mutex>>>, } @@ -99,9 +100,10 @@ impl<'tcx> QueryLatch<'tcx> { &self, tcx: TyCtxt<'tcx>, query: Option, + span: Span, ) -> Result<(), CycleError>> { let waiter = - Arc::new(QueryWaiter { query, cycle: Mutex::new(None), condvar: Condvar::new() }); + Arc::new(QueryWaiter { query, span, cycle: Mutex::new(None), condvar: Condvar::new() }); self.wait_on_inner(tcx, &waiter); // FIXME: Get rid of this lock. We have ownership of the QueryWaiter // although another thread may still have a Arc reference so we cannot diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 1fb502148d7a3..53afcacb63a6c 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -228,6 +228,7 @@ fn cycle_error<'tcx, C: QueryCache>( fn wait_for_query<'tcx, C: QueryCache>( query: &'tcx QueryVTable<'tcx, C>, tcx: TyCtxt<'tcx>, + span: Span, key: C::Key, latch: QueryLatch<'tcx>, current: Option, @@ -239,7 +240,7 @@ fn wait_for_query<'tcx, C: QueryCache>( // With parallel queries we might just have to wait on some other // thread. - let result = latch.wait_on(tcx, current); + let result = latch.wait_on(tcx, current, span); match result { Ok(()) => { @@ -319,7 +320,7 @@ fn try_execute_query<'tcx, C: QueryCache, const INCR: bool>( // Only call `wait_for_query` if we're using a Rayon thread pool // as it will attempt to mark the worker thread as blocked. - return wait_for_query(query, tcx, key, latch, current_job_id); + return wait_for_query(query, tcx, span, key, latch, current_job_id); } let id = job.id; From 6d5996af440393c78262bddcb85734d7949e14c9 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 2 Mar 2026 13:15:25 +0300 Subject: [PATCH 5/9] Remove whitespace --- compiler/rustc_query_impl/src/job.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 08a39b96c7f68..ea5382f0e2612 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -100,7 +100,7 @@ pub(crate) fn find_dep_kind_root<'tcx>( /// Breaks cycle on some query. /// -/// This function doesn't distinguish between a query wait and a query execution, so both are just +/// This function doesn't distinguish between a query wait and a query execution, so both are just /// query calls. /// As such some queries may have two or more parent query calls too. /// It uses depth-first search from a single root query down to the first duplicate query, From 2e0fbb91d14a473d101f0881800740ca3afe4cc7 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 2 Mar 2026 13:17:18 +0300 Subject: [PATCH 6/9] Do not assume there's only one root query --- compiler/rustc_query_impl/src/job.rs | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index ea5382f0e2612..546c0bc5cbddd 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -110,14 +110,12 @@ pub fn break_query_cycles<'tcx>( query_map: QueryJobMap<'tcx>, registry: &rustc_thread_pool::Registry, ) { - let mut root_query = None; - for (&query, info) in &query_map.map { - if info.job.parent.is_none() { - assert!(root_query.is_none(), "found multiple threads without start"); - root_query = Some(query); - } - } - let root_query = root_query.expect("no root query was found"); + // We pick any root query we find + let (&root_query, _) = query_map + .map + .iter() + .find(|(_, info)| info.job.parent.is_none()) + .expect("no root query was found"); // We are allowed to keep track of just one subquery since each query has at least one subquery. // From 6c561885f91d5074f33f567abe582cd007dbb42d Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 2 Mar 2026 16:36:08 +0300 Subject: [PATCH 7/9] Use QueryWaiter's span --- compiler/rustc_query_impl/src/job.rs | 71 ++++++++++++++++------------ 1 file changed, 42 insertions(+), 29 deletions(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 546c0bc5cbddd..a9a375584e99f 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -9,7 +9,7 @@ use rustc_middle::query::{ }; use rustc_middle::ty::TyCtxt; use rustc_session::Session; -use rustc_span::Span; +use rustc_span::{DUMMY_SP, Span}; use crate::plumbing::collect_active_jobs_from_all_queries; @@ -117,6 +117,14 @@ pub fn break_query_cycles<'tcx>( .find(|(_, info)| info.job.parent.is_none()) .expect("no root query was found"); + #[derive(Clone, Copy)] + struct Subquery { + id: QueryJobId, + span: Span, + /// Waiter index or `usize::MAX` if subquery was executed + waiter_idx: usize, + } + // We are allowed to keep track of just one subquery since each query has at least one subquery. // // If we would assume the opposite then thread of query with no subqueries cannot wait on any @@ -130,7 +138,11 @@ pub fn break_query_cycles<'tcx>( continue; }; // We are safe to only track a single subquery due to the statement above - subqueries.entry(parent).or_insert((query.job.id, usize::MAX)); + subqueries.entry(parent).or_insert(Subquery { + id: query.job.id, + span: query.job.span, + waiter_idx: usize::MAX, + }); } for query in query_map.map.values() { @@ -143,7 +155,11 @@ pub fn break_query_cycles<'tcx>( for (waiter_idx, waiter) in lock.waiters.iter().enumerate() { let waited_on_query = waiter.query.expect("cannot wait on a root query"); // We are safe to only track a single subquery due to the statement above - subqueries.entry(waited_on_query).or_insert((query.job.id, waiter_idx)); + subqueries.entry(waited_on_query).or_insert(Subquery { + id: query.job.id, + span: waiter.span, + waiter_idx, + }); } } @@ -167,46 +183,43 @@ pub fn break_query_cycles<'tcx>( // However that means `b()` is not the first duplicate query in the stack, // so the original statement must be true. let mut visited = IndexMap::new(); - let mut last_usage = None; - let mut last_waiter_idx = usize::MAX; - let mut current = root_query; - while let indexmap::map::Entry::Vacant(entry) = visited.entry(current) { - entry.insert((last_usage, last_waiter_idx)); - last_usage = Some(current); - (current, last_waiter_idx) = subqueries[¤t]; + let mut last_parent = None; + let mut last = Subquery { id: root_query, span: DUMMY_SP, waiter_idx: usize::MAX }; + while let indexmap::map::Entry::Vacant(entry) = visited.entry(last.id) { + entry.insert((last_parent, last)); + last_parent = Some(last.id); + last = subqueries[&last.id]; } - let usage = visited[¤t].0; - let mut iter = visited.keys().rev(); + + let parent = visited[&last.id].0; + let mut iter = visited.values(); let mut cycle = Vec::new(); loop { - let query_id = *iter.next().unwrap(); - let query = &query_map.map[&query_id]; - cycle.push(QueryInfo { span: query.job.span, frame: query.frame.clone() }); - if query_id == current { + let (_, subquery) = iter.next_back().unwrap(); + let frame = query_map.map[&subquery.id].frame.clone(); + cycle.push(QueryInfo { span: subquery.span, frame }); + if subquery.id == last.id { break; } } - cycle.reverse(); - let cycle_error = CycleError { - usage: usage.map(|id| { - let query = &query_map.map[&id]; - (query.job.span, query.frame.clone()) - }), - cycle, - }; + cycle[0].span = last.span; + let usage = parent.map(|parent| (last.span, query_map.map[&parent].frame.clone())); + let cycle_error = CycleError { usage, cycle }; // Per statement above we should have wait at either of two occurrences of the duplicate query - if last_waiter_idx == usize::MAX { - last_waiter_idx = visited.get(¤t).unwrap().1; - } + let waiter_idx = if last.waiter_idx != usize::MAX { + last.waiter_idx + } else { + visited.get(&last.id).unwrap().1.waiter_idx + }; - let waited_on = &query_map.map[¤t]; + let waited_on = &query_map.map[&last.id]; let latch = waited_on.job.latch.as_ref().unwrap(); let mut latch_info_lock = latch.info.try_lock().unwrap(); // And so this `Vec::remove` shouldn't cause a panic - let waiter = latch_info_lock.waiters.remove(last_waiter_idx); + let waiter = latch_info_lock.waiters.remove(waiter_idx); let mut cycle_lock = waiter.cycle.try_lock().unwrap(); assert!(cycle_lock.is_none()); From 52c016bceef3715aefe840c6bbe5f8de361f8988 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 2 Mar 2026 16:47:55 +0300 Subject: [PATCH 8/9] Split cycle breaking into find_cycle_in_graph --- compiler/rustc_query_impl/src/job.rs | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index a9a375584e99f..12eabc535bc24 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -98,7 +98,7 @@ pub(crate) fn find_dep_kind_root<'tcx>( last_layout } -/// Breaks cycle on some query. +/// Finds a query to break cycle on. /// /// This function doesn't distinguish between a query wait and a query execution, so both are just /// query calls. @@ -106,10 +106,9 @@ pub(crate) fn find_dep_kind_root<'tcx>( /// It uses depth-first search from a single root query down to the first duplicate query, /// establishing a cycle. #[allow(rustc::potential_query_instability)] -pub fn break_query_cycles<'tcx>( - query_map: QueryJobMap<'tcx>, - registry: &rustc_thread_pool::Registry, -) { +fn find_cycle_in_graph<'tcx>( + query_map: &QueryJobMap<'tcx>, +) -> (QueryJobId, usize, CycleError>) { // We pick any root query we find let (&root_query, _) = query_map .map @@ -214,7 +213,16 @@ pub fn break_query_cycles<'tcx>( visited.get(&last.id).unwrap().1.waiter_idx }; - let waited_on = &query_map.map[&last.id]; + (last.id, waiter_idx, cycle_error) +} + +pub fn break_query_cycles<'tcx>( + query_map: QueryJobMap<'tcx>, + registry: &rustc_thread_pool::Registry, +) { + let (waited_on, waiter_idx, cycle_error) = find_cycle_in_graph(&query_map); + + let waited_on = &query_map.map[&waited_on]; let latch = waited_on.job.latch.as_ref().unwrap(); let mut latch_info_lock = latch.info.try_lock().unwrap(); From 11b79d6e859cbafbafb13380915f0c934c7134f8 Mon Sep 17 00:00:00 2001 From: Daria Sukhonina Date: Mon, 2 Mar 2026 16:48:13 +0300 Subject: [PATCH 9/9] Debug assert find_cycle_in_graph is consistent with find_cycle_in_stack --- compiler/rustc_middle/src/query/plumbing.rs | 13 ++++++++++++ compiler/rustc_middle/src/query/stack.rs | 6 ++++++ compiler/rustc_query_impl/src/execution.rs | 3 ++- compiler/rustc_query_impl/src/job.rs | 23 ++++++++++++--------- 4 files changed, 34 insertions(+), 11 deletions(-) diff --git a/compiler/rustc_middle/src/query/plumbing.rs b/compiler/rustc_middle/src/query/plumbing.rs index 9652be2551629..3840886fcd3b3 100644 --- a/compiler/rustc_middle/src/query/plumbing.rs +++ b/compiler/rustc_middle/src/query/plumbing.rs @@ -87,6 +87,19 @@ pub struct CycleError { } impl<'tcx> CycleError> { + pub fn is_similar_to(&self, other: &Self) -> bool { + (match (&self.usage, &other.usage) { + (None, None) => true, + (None, Some(_)) | (Some(_), None) => false, + (Some((s1, f1)), Some((s2, f2))) => s1 == s2 && f1.is_similar_to(&f2), + }) && self.cycle.len() == other.cycle.len() + && self + .cycle + .iter() + .zip(&other.cycle) + .all(|(q1, q2)| q1.span == q2.span && q1.frame.is_similar_to(&q2.frame)) + } + pub fn lift(&self) -> CycleError { CycleError { usage: self.usage.as_ref().map(|(span, frame)| (*span, frame.lift())), diff --git a/compiler/rustc_middle/src/query/stack.rs b/compiler/rustc_middle/src/query/stack.rs index fd80c7edd602d..feabe50af29ca 100644 --- a/compiler/rustc_middle/src/query/stack.rs +++ b/compiler/rustc_middle/src/query/stack.rs @@ -46,6 +46,12 @@ impl<'tcx> QueryStackFrame> { def_id_for_ty_in_cycle: self.def_id_for_ty_in_cycle, } } + + pub fn is_similar_to(&self, other: &Self) -> bool { + self.dep_kind == other.dep_kind + && self.def_id == other.def_id + && self.def_id_for_ty_in_cycle == other.def_id_for_ty_in_cycle + } } #[derive(Clone, Debug)] diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 53afcacb63a6c..e3aca300e638c 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -220,7 +220,8 @@ fn cycle_error<'tcx, C: QueryCache>( .ok() .expect("failed to collect active queries"); - let error = find_cycle_in_stack(try_execute, job_map, ¤t_query_job(tcx), span); + let error = find_cycle_in_stack(try_execute, &job_map, current_query_job(tcx), span) + .expect("did not find a cycle"); (mk_cycle(query, tcx, error.lift()), None) } diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs index 12eabc535bc24..11494f6c5fd79 100644 --- a/compiler/rustc_query_impl/src/job.rs +++ b/compiler/rustc_query_impl/src/job.rs @@ -41,13 +41,12 @@ pub(crate) struct QueryJobInfo<'tcx> { pub(crate) fn find_cycle_in_stack<'tcx>( id: QueryJobId, - job_map: QueryJobMap<'tcx>, - current_job: &Option, + job_map: &QueryJobMap<'tcx>, + mut current_job: Option, span: Span, -) -> CycleError> { +) -> Option>> { // Find the waitee amongst `current_job` parents let mut cycle = Vec::new(); - let mut current_job = Option::clone(current_job); while let Some(job) = current_job { let info = &job_map.map[&job]; @@ -62,17 +61,15 @@ pub(crate) fn find_cycle_in_stack<'tcx>( // Replace it with the span which caused the cycle to form cycle[0].span = span; // Find out why the cycle itself was used - let usage = try { - let parent = info.job.parent?; - (info.job.span, job_map.frame_of(parent).clone()) - }; - return CycleError { usage, cycle }; + let usage = + info.job.parent.map(|parent| (info.job.span, job_map.frame_of(parent).clone())); + return Some(CycleError { usage, cycle }); } current_job = info.job.parent; } - panic!("did not find a cycle") + None } #[cold] @@ -206,6 +203,12 @@ fn find_cycle_in_graph<'tcx>( let usage = parent.map(|parent| (last.span, query_map.map[&parent].frame.clone())); let cycle_error = CycleError { usage, cycle }; + if cfg!(debug_assertions) + && let Some(expected) = find_cycle_in_stack(last.id, query_map, last_parent, last.span) + { + assert!(cycle_error.is_similar_to(&expected)); + } + // Per statement above we should have wait at either of two occurrences of the duplicate query let waiter_idx = if last.waiter_idx != usize::MAX { last.waiter_idx