From 066a935b0c25608db20da43951def47f3b1539e6 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Tue, 10 Feb 2026 10:17:13 +1100 Subject: [PATCH] Move parts of `rustc_query_system::query::job` to `rustc_middle::job`. The latter is a new module. As well as the code motion, some other changes were required. - `QueryJobId` methods became free functions so they could move while `QueryJobId` itself stayed put. This was so `QueryMap` and `QueryJobInfo` could be moved. - Some visibilities in `rustc_query_system` required changing. - `collect_active_jobs_from_all_queries` is no longer required in `trait QueryContext`. --- Cargo.lock | 1 + compiler/rustc_interface/src/interface.rs | 3 +- compiler/rustc_interface/src/util.rs | 3 +- compiler/rustc_query_impl/Cargo.toml | 1 + compiler/rustc_query_impl/src/execution.rs | 9 +- compiler/rustc_query_impl/src/job.rs | 446 ++++++++++++++++++ compiler/rustc_query_impl/src/lib.rs | 4 +- compiler/rustc_query_impl/src/plumbing.rs | 59 +-- compiler/rustc_query_system/src/query/job.rs | 460 +------------------ compiler/rustc_query_system/src/query/mod.rs | 12 +- 10 files changed, 504 insertions(+), 494 deletions(-) create mode 100644 compiler/rustc_query_impl/src/job.rs diff --git a/Cargo.lock b/Cargo.lock index 545c776b4805..7230d633602d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4503,6 +4503,7 @@ dependencies = [ "rustc_query_system", "rustc_serialize", "rustc_span", + "rustc_thread_pool", "tracing", ] diff --git a/compiler/rustc_interface/src/interface.rs b/compiler/rustc_interface/src/interface.rs index 72080c5f5369..6cbc184e8f1b 100644 --- a/compiler/rustc_interface/src/interface.rs +++ b/compiler/rustc_interface/src/interface.rs @@ -16,8 +16,7 @@ use rustc_parse::lexer::StripTokens; use rustc_parse::new_parser_from_source_str; use rustc_parse::parser::Recovery; use rustc_parse::parser::attr::AllowLeadingUnsafe; -use rustc_query_impl::QueryCtxt; -use rustc_query_system::query::print_query_stack; +use rustc_query_impl::{QueryCtxt, print_query_stack}; use rustc_session::config::{self, Cfg, CheckCfg, ExpectedValues, Input, OutFileName}; use rustc_session::parse::ParseSess; use rustc_session::{CompilerIO, EarlyDiagCtxt, Session, lint}; diff --git a/compiler/rustc_interface/src/util.rs b/compiler/rustc_interface/src/util.rs index 458e7ad32143..59f0b67d6729 100644 --- a/compiler/rustc_interface/src/util.rs +++ b/compiler/rustc_interface/src/util.rs @@ -184,8 +184,7 @@ pub(crate) fn run_in_thread_pool_with_globals< use rustc_data_structures::defer; use rustc_data_structures::sync::FromDyn; use rustc_middle::ty::tls; - use rustc_query_impl::QueryCtxt; - use rustc_query_system::query::{QueryContext, break_query_cycles}; + use rustc_query_impl::{QueryCtxt, break_query_cycles}; let thread_stack_size = init_stack_size(thread_builder_diag); diff --git a/compiler/rustc_query_impl/Cargo.toml b/compiler/rustc_query_impl/Cargo.toml index d611629671a0..511337090d7f 100644 --- a/compiler/rustc_query_impl/Cargo.toml +++ b/compiler/rustc_query_impl/Cargo.toml @@ -16,5 +16,6 @@ rustc_middle = { path = "../rustc_middle" } rustc_query_system = { path = "../rustc_query_system" } rustc_serialize = { path = "../rustc_serialize" } rustc_span = { path = "../rustc_span" } +rustc_thread_pool = { path = "../rustc_thread_pool" } tracing = "0.1" # tidy-alphabetical-end diff --git a/compiler/rustc_query_impl/src/execution.rs b/compiler/rustc_query_impl/src/execution.rs index 88604c91d025..6f16932cb2eb 100644 --- a/compiler/rustc_query_impl/src/execution.rs +++ b/compiler/rustc_query_impl/src/execution.rs @@ -9,13 +9,14 @@ use rustc_middle::dep_graph::DepsType; use rustc_middle::ty::TyCtxt; use rustc_query_system::dep_graph::{DepGraphData, DepNodeKey, HasDepContext}; use rustc_query_system::query::{ - ActiveKeyStatus, CycleError, CycleErrorHandling, QueryCache, QueryContext, QueryJob, - QueryJobId, QueryJobInfo, QueryLatch, QueryMap, QueryMode, QueryStackDeferred, QueryStackFrame, - QueryState, incremental_verify_ich, report_cycle, + ActiveKeyStatus, CycleError, CycleErrorHandling, QueryCache, QueryJob, QueryJobId, QueryLatch, + QueryMode, QueryStackDeferred, QueryStackFrame, QueryState, incremental_verify_ich, + report_cycle, }; use rustc_span::{DUMMY_SP, Span}; use crate::dep_graph::{DepContext, DepNode, DepNodeIndex}; +use crate::job::{QueryJobInfo, QueryMap, find_cycle_in_stack}; use crate::{QueryCtxt, QueryFlags, SemiDynamicQueryDispatcher}; #[inline] @@ -218,7 +219,7 @@ fn cycle_error<'tcx, C: QueryCache, const FLAGS: QueryFlags>( .ok() .expect("failed to collect active queries"); - let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); + let error = find_cycle_in_stack(try_execute, query_map, &qcx.current_query_job(), span); (mk_cycle(query, qcx, error.lift()), None) } diff --git a/compiler/rustc_query_impl/src/job.rs b/compiler/rustc_query_impl/src/job.rs new file mode 100644 index 000000000000..cb0a13d32ad2 --- /dev/null +++ b/compiler/rustc_query_impl/src/job.rs @@ -0,0 +1,446 @@ +use std::io::Write; +use std::iter; +use std::sync::Arc; + +use rustc_data_structures::fx::{FxHashMap, FxHashSet}; +use rustc_errors::DiagCtxtHandle; +use rustc_query_system::query::{ + CycleError, QueryInfo, QueryJob, QueryJobId, QueryLatch, QueryStackDeferred, QueryStackFrame, + QueryWaiter, +}; +use rustc_span::{DUMMY_SP, Span}; + +use crate::QueryCtxt; +use crate::dep_graph::DepContext; + +/// Map from query job IDs to job information collected by +/// `collect_active_jobs_from_all_queries`. +pub type QueryMap<'tcx> = FxHashMap>; + +fn query_job_id_frame<'a, 'tcx>( + id: QueryJobId, + map: &'a QueryMap<'tcx>, +) -> QueryStackFrame> { + map.get(&id).unwrap().frame.clone() +} + +fn query_job_id_span<'a, 'tcx>(id: QueryJobId, map: &'a QueryMap<'tcx>) -> Span { + map.get(&id).unwrap().job.span +} + +fn query_job_id_parent<'a, 'tcx>(id: QueryJobId, map: &'a QueryMap<'tcx>) -> Option { + map.get(&id).unwrap().job.parent +} + +fn query_job_id_latch<'a, 'tcx>( + id: QueryJobId, + map: &'a QueryMap<'tcx>, +) -> Option<&'a QueryLatch<'tcx>> { + map.get(&id).unwrap().job.latch.as_ref() +} + +#[derive(Clone, Debug)] +pub struct QueryJobInfo<'tcx> { + pub frame: QueryStackFrame>, + pub job: QueryJob<'tcx>, +} + +pub(crate) fn find_cycle_in_stack<'tcx>( + id: QueryJobId, + query_map: QueryMap<'tcx>, + current_job: &Option, + span: Span, +) -> CycleError> { + // 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 = query_map.get(&job).unwrap(); + cycle.push(QueryInfo { span: info.job.span, frame: info.frame.clone() }); + + if job == id { + cycle.reverse(); + + // This is the end of the cycle + // The span entry we included was for the usage + // of the cycle itself, and not part of the cycle + // 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 = info + .job + .parent + .as_ref() + .map(|parent| (info.job.span, query_job_id_frame(*parent, &query_map))); + return CycleError { usage, cycle }; + } + + current_job = info.job.parent; + } + + panic!("did not find a cycle") +} + +#[cold] +#[inline(never)] +pub(crate) fn find_dep_kind_root<'tcx>( + id: QueryJobId, + query_map: QueryMap<'tcx>, +) -> (QueryJobInfo<'tcx>, usize) { + let mut depth = 1; + let info = query_map.get(&id).unwrap(); + let dep_kind = info.frame.dep_kind; + let mut current_id = info.job.parent; + let mut last_layout = (info.clone(), depth); + + while let Some(id) = current_id { + let info = query_map.get(&id).unwrap(); + if info.frame.dep_kind == dep_kind { + depth += 1; + last_layout = (info.clone(), depth); + } + current_id = info.job.parent; + } + 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_ref` +/// and a span indicating the reason the query waited on `query_ref`. +/// If `visit` returns Some, this function returns. +/// For visits of non-resumable waiters it returns the return value of `visit`. +/// For visits of resumable waiters it returns Some(Some(Waiter)) which has the +/// required information to resume the waiter. +/// If all `visit` calls returns None, this function also returns None. +fn visit_waiters<'tcx, F>( + query_map: &QueryMap<'tcx>, + query: QueryJobId, + mut visit: F, +) -> Option> +where + F: FnMut(Span, QueryJobId) -> Option>, +{ + // Visit the parent query which is a non-resumable waiter since it's on the same stack + if let Some(parent) = query_job_id_parent(query, query_map) + && let Some(cycle) = visit(query_job_id_span(query, query_map), parent) + { + return Some(cycle); + } + + // Visit the explicit waiters which use condvars and are resumable + if let Some(latch) = query_job_id_latch(query, query_map) { + for (i, waiter) in latch.info.lock().waiters.iter().enumerate() { + if let Some(waiter_query) = waiter.query { + if visit(waiter.span, waiter_query).is_some() { + // Return a value which indicates that this waiter can be resumed + return Some(Some((query, i))); + } + } + } + } + + None +} + +/// 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>( + query_map: &QueryMap<'tcx>, + query: QueryJobId, + span: Span, + stack: &mut Vec<(Span, QueryJobId)>, + visited: &mut FxHashSet, +) -> Option> { + 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; + Some(None) + } else { + None + }; + } + + // Query marked as visited is added it to the stack + stack.push((span, query)); + + // Visit all the waiters + let r = visit_waiters(query_map, query, |span, successor| { + cycle_check(query_map, successor, span, stack, visited) + }); + + // Remove the entry in our stack if we didn't find a cycle + if r.is_none() { + 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>( + query_map: &QueryMap<'tcx>, + query: QueryJobId, + visited: &mut FxHashSet, +) -> bool { + // We already visited this or we're deliberately ignoring it + if !visited.insert(query) { + return false; + } + + // This query is connected to the root (it has no query parent), return true + if query_job_id_parent(query, query_map).is_none() { + return true; + } + + visit_waiters(query_map, query, |_, successor| { + connected_to_root(query_map, successor, visited).then_some(None) + }) + .is_some() +} + +// Deterministically pick an query from a list +fn pick_query<'a, 'tcx, T, F>(query_map: &QueryMap<'tcx>, queries: &'a [T], f: F) -> &'a T +where + F: Fn(&T) -> (Span, QueryJobId), +{ + // Deterministically pick an entry point + // FIXME: Sort this instead + queries + .iter() + .min_by_key(|v| { + let (span, query) = f(v); + let hash = query_job_id_frame(query, query_map).hash; + // Prefer entry points which have valid spans for nicer error messages + // We add an integer to the tuple ensuring that entry points + // with valid spans are picked first + let span_cmp = if span == DUMMY_SP { 1 } else { 0 }; + (span_cmp, hash) + }) + .unwrap() +} + +/// 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>( + query_map: &QueryMap<'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 Some(waiter) = + cycle_check(query_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); + } + } + + // Find the queries in the cycle which are + // connected to queries outside the cycle + let entry_points = stack + .iter() + .filter_map(|&(span, query)| { + if query_job_id_parent(query, query_map).is_none() { + // This query is connected to the root (it has no query parent) + Some((span, query, None)) + } else { + let mut waiters = Vec::new(); + // Find all the direct waiters who lead to the root + visit_waiters(query_map, query, |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(query_map, waiter, &mut visited) { + waiters.push((span, waiter)); + } + + None + }); + if waiters.is_empty() { + None + } else { + // Deterministically pick one of the waiters to show to the user + let waiter = *pick_query(query_map, &waiters, |s| *s); + Some((span, query, Some(waiter))) + } + } + }) + .collect::)>>(); + + // Deterministically pick an entry point + let (_, entry_point, usage) = pick_query(query_map, &entry_points, |e| (e.0, e.1)); + + // Shift the stack so that our entry point is first + let entry_point_pos = stack.iter().position(|(_, query)| query == entry_point); + if let Some(pos) = entry_point_pos { + stack.rotate_left(pos); + } + + let usage = + usage.as_ref().map(|(span, query)| (*span, query_job_id_frame(*query, query_map))); + + // Create the cycle error + let error = CycleError { + usage, + cycle: stack + .iter() + .map(|&(s, ref q)| QueryInfo { span: s, frame: query_job_id_frame(*q, query_map) }) + .collect(), + }; + + // 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 = + query_job_id_latch(waitee_query, query_map).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>(query_map: QueryMap<'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 = query_map.keys().cloned().collect(); + + let mut found_cycle = false; + + while jobs.len() > 0 { + if remove_cycle(&query_map, &mut jobs, &mut wakelist) { + found_cycle = true; + } + } + + // 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{:#?}", + query_map + ); + } + + // 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); + } + + for waiter in wakelist.into_iter() { + waiter.condvar.notify_one(); + } +} + +pub fn print_query_stack<'tcx>( + qcx: QueryCtxt<'tcx>, + mut current_query: Option, + dcx: DiagCtxtHandle<'_>, + limit_frames: Option, + mut file: Option, +) -> usize { + // Be careful relying on global state here: this code is called from + // a panic hook, which means that the global `DiagCtxt` may be in a weird + // state if it was responsible for triggering the panic. + let mut count_printed = 0; + let mut count_total = 0; + + // Make use of a partial query map if we fail to take locks collecting active queries. + let query_map = match qcx.collect_active_jobs_from_all_queries(false) { + Ok(query_map) => query_map, + Err(query_map) => query_map, + }; + + if let Some(ref mut file) = file { + let _ = writeln!(file, "\n\nquery stack during panic:"); + } + while let Some(query) = current_query { + let Some(query_info) = query_map.get(&query) else { + break; + }; + let query_extra = query_info.frame.info.extract(); + if Some(count_printed) < limit_frames || limit_frames.is_none() { + // Only print to stderr as many stack frames as `num_frames` when present. + dcx.struct_failure_note(format!( + "#{} [{:?}] {}", + count_printed, query_info.frame.dep_kind, query_extra.description + )) + .with_span(query_info.job.span) + .emit(); + count_printed += 1; + } + + if let Some(ref mut file) = file { + let _ = writeln!( + file, + "#{} [{}] {}", + count_total, + qcx.tcx.dep_kind_vtable(query_info.frame.dep_kind).name, + query_extra.description + ); + } + + current_query = query_info.job.parent; + count_total += 1; + } + + if let Some(ref mut file) = file { + let _ = writeln!(file, "end of query stack"); + } + count_total +} diff --git a/compiler/rustc_query_impl/src/lib.rs b/compiler/rustc_query_impl/src/lib.rs index a33bdd22a797..c1ead8da344b 100644 --- a/compiler/rustc_query_impl/src/lib.rs +++ b/compiler/rustc_query_impl/src/lib.rs @@ -23,10 +23,11 @@ use rustc_middle::query::values::Value; use rustc_middle::ty::TyCtxt; use rustc_query_system::dep_graph::SerializedDepNodeIndex; use rustc_query_system::query::{ - CycleError, CycleErrorHandling, QueryCache, QueryMap, QueryMode, QueryState, + CycleError, CycleErrorHandling, QueryCache, QueryMode, QueryState, }; use rustc_span::{ErrorGuaranteed, Span}; +pub use crate::job::{QueryMap, break_query_cycles, print_query_stack}; pub use crate::plumbing::{QueryCtxt, query_key_hash_verify_all}; use crate::plumbing::{encode_all_query_results, try_mark_green}; use crate::profiling_support::QueryKeyStringCache; @@ -34,6 +35,7 @@ pub use crate::profiling_support::alloc_self_profile_query_strings; mod error; mod execution; +mod job; #[macro_use] mod plumbing; mod profiling_support; diff --git a/compiler/rustc_query_impl/src/plumbing.rs b/compiler/rustc_query_impl/src/plumbing.rs index 776d720f500b..5fba9dd4a68a 100644 --- a/compiler/rustc_query_impl/src/plumbing.rs +++ b/compiler/rustc_query_impl/src/plumbing.rs @@ -28,14 +28,15 @@ use rustc_middle::ty::tls::{self, ImplicitCtxt}; use rustc_middle::ty::{self, TyCtxt}; use rustc_query_system::dep_graph::{DepNodeKey, FingerprintStyle, HasDepContext}; use rustc_query_system::query::{ - QueryCache, QueryContext, QueryJobId, QueryMap, QuerySideEffect, QueryStackDeferred, - QueryStackFrame, QueryStackFrameExtra, + QueryCache, QueryContext, QueryJobId, QuerySideEffect, QueryStackDeferred, QueryStackFrame, + QueryStackFrameExtra, }; use rustc_serialize::{Decodable, Encodable}; use rustc_span::def_id::LOCAL_CRATE; use crate::error::{QueryOverflow, QueryOverflowNote}; use crate::execution::{all_inactive, force_query}; +use crate::job::{QueryMap, find_dep_kind_root}; use crate::{QueryDispatcherUnerased, QueryFlags, SemiDynamicQueryDispatcher}; /// Implements [`QueryContext`] for use by [`rustc_query_system`], since that @@ -55,7 +56,7 @@ impl<'tcx> QueryCtxt<'tcx> { let query_map = self .collect_active_jobs_from_all_queries(true) .expect("failed to collect active queries"); - let (info, depth) = job.find_dep_kind_root(query_map); + let (info, depth) = find_dep_kind_root(job, query_map); let suggested_limit = match self.tcx.recursion_limit() { Limit(0) => Limit(2), @@ -116,6 +117,32 @@ impl<'tcx> QueryCtxt<'tcx> { tls::enter_context(&new_icx, compute) }) } + + /// Returns a map of currently active query jobs, collected from all queries. + /// + /// If `require_complete` is `true`, this function locks all shards of the + /// query results to produce a complete map, which always returns `Ok`. + /// Otherwise, it may return an incomplete map as an error if any shard + /// lock cannot be acquired. + /// + /// Prefer passing `false` to `require_complete` to avoid potential deadlocks, + /// especially when called from within a deadlock handler, unless a + /// complete map is needed and no deadlock is possible at this call site. + pub fn collect_active_jobs_from_all_queries( + self, + require_complete: bool, + ) -> Result, QueryMap<'tcx>> { + let mut jobs = QueryMap::default(); + let mut complete = true; + + for gather_fn in crate::PER_QUERY_GATHER_ACTIVE_JOBS_FNS.iter() { + if gather_fn(self.tcx, &mut jobs, require_complete).is_none() { + complete = false; + } + } + + if complete { Ok(jobs) } else { Err(jobs) } + } } impl<'tcx> HasDepContext for QueryCtxt<'tcx> { @@ -134,32 +161,6 @@ impl<'tcx> QueryContext<'tcx> for QueryCtxt<'tcx> { &self.tcx.jobserver_proxy } - /// Returns a map of currently active query jobs, collected from all queries. - /// - /// If `require_complete` is `true`, this function locks all shards of the - /// query results to produce a complete map, which always returns `Ok`. - /// Otherwise, it may return an incomplete map as an error if any shard - /// lock cannot be acquired. - /// - /// Prefer passing `false` to `require_complete` to avoid potential deadlocks, - /// especially when called from within a deadlock handler, unless a - /// complete map is needed and no deadlock is possible at this call site. - fn collect_active_jobs_from_all_queries( - self, - require_complete: bool, - ) -> Result, QueryMap<'tcx>> { - let mut jobs = QueryMap::default(); - let mut complete = true; - - for gather_fn in crate::PER_QUERY_GATHER_ACTIVE_JOBS_FNS.iter() { - if gather_fn(self.tcx, &mut jobs, require_complete).is_none() { - complete = false; - } - } - - if complete { Ok(jobs) } else { Err(jobs) } - } - // Interactions with on_disk_cache fn load_side_effect( self, diff --git a/compiler/rustc_query_system/src/query/job.rs b/compiler/rustc_query_system/src/query/job.rs index c77ae38ba96f..e8acb8064f7e 100644 --- a/compiler/rustc_query_system/src/query/job.rs +++ b/compiler/rustc_query_system/src/query/job.rs @@ -1,19 +1,15 @@ use std::fmt::Debug; use std::hash::Hash; -use std::io::Write; -use std::iter; use std::num::NonZero; use std::sync::Arc; use parking_lot::{Condvar, Mutex}; -use rustc_data_structures::fx::{FxHashMap, FxHashSet}; -use rustc_errors::{Diag, DiagCtxtHandle}; +use rustc_errors::Diag; use rustc_hir::def::DefKind; use rustc_session::Session; -use rustc_span::{DUMMY_SP, Span}; +use rustc_span::Span; use super::{QueryStackDeferred, QueryStackFrameExtra}; -use crate::dep_graph::DepContext; use crate::error::CycleStack; use crate::query::plumbing::CycleError; use crate::query::{QueryContext, QueryStackFrame}; @@ -32,38 +28,10 @@ impl<'tcx> QueryInfo> { } } -/// Map from query job IDs to job information collected by -/// [`QueryContext::collect_active_jobs_from_all_queries`]. -pub type QueryMap<'tcx> = FxHashMap>; - /// A value uniquely identifying an active query job. #[derive(Copy, Clone, Eq, PartialEq, Hash, Debug)] pub struct QueryJobId(pub NonZero); -impl QueryJobId { - fn frame<'a, 'tcx>(self, map: &'a QueryMap<'tcx>) -> QueryStackFrame> { - map.get(&self).unwrap().frame.clone() - } - - fn span<'a, 'tcx>(self, map: &'a QueryMap<'tcx>) -> Span { - map.get(&self).unwrap().job.span - } - - fn parent<'a, 'tcx>(self, map: &'a QueryMap<'tcx>) -> Option { - map.get(&self).unwrap().job.parent - } - - fn latch<'a, 'tcx>(self, map: &'a QueryMap<'tcx>) -> Option<&'a QueryLatch<'tcx>> { - map.get(&self).unwrap().job.latch.as_ref() - } -} - -#[derive(Clone, Debug)] -pub struct QueryJobInfo<'tcx> { - pub frame: QueryStackFrame>, - pub job: QueryJob<'tcx>, -} - /// Represents an active query job. #[derive(Clone, Debug)] pub struct QueryJob<'tcx> { @@ -76,7 +44,7 @@ pub struct QueryJob<'tcx> { pub parent: Option, /// The latch that is used to wait on this job. - latch: Option>, + pub latch: Option>, } impl<'tcx> QueryJob<'tcx> { @@ -105,85 +73,23 @@ impl<'tcx> QueryJob<'tcx> { } } -impl QueryJobId { - pub fn find_cycle_in_stack<'tcx>( - &self, - query_map: QueryMap<'tcx>, - current_job: &Option, - span: Span, - ) -> CycleError> { - // 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 = query_map.get(&job).unwrap(); - cycle.push(QueryInfo { span: info.job.span, frame: info.frame.clone() }); - - if job == *self { - cycle.reverse(); - - // This is the end of the cycle - // The span entry we included was for the usage - // of the cycle itself, and not part of the cycle - // 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 = info - .job - .parent - .as_ref() - .map(|parent| (info.job.span, parent.frame(&query_map))); - return CycleError { usage, cycle }; - } - - current_job = info.job.parent; - } - - panic!("did not find a cycle") - } - - #[cold] - #[inline(never)] - pub fn find_dep_kind_root<'tcx>( - &self, - query_map: QueryMap<'tcx>, - ) -> (QueryJobInfo<'tcx>, usize) { - let mut depth = 1; - let info = query_map.get(&self).unwrap(); - let dep_kind = info.frame.dep_kind; - let mut current_id = info.job.parent; - let mut last_layout = (info.clone(), depth); - - while let Some(id) = current_id { - let info = query_map.get(&id).unwrap(); - if info.frame.dep_kind == dep_kind { - depth += 1; - last_layout = (info.clone(), depth); - } - current_id = info.job.parent; - } - last_layout - } +#[derive(Debug)] +pub struct QueryWaiter<'tcx> { + pub query: Option, + pub condvar: Condvar, + pub span: Span, + pub cycle: Mutex>>>, } #[derive(Debug)] -struct QueryWaiter<'tcx> { - query: Option, - condvar: Condvar, - span: Span, - cycle: Mutex>>>, -} - -#[derive(Debug)] -struct QueryLatchInfo<'tcx> { - complete: bool, - waiters: Vec>>, +pub struct QueryLatchInfo<'tcx> { + pub complete: bool, + pub waiters: Vec>>, } #[derive(Clone, Debug)] pub struct QueryLatch<'tcx> { - info: Arc>>, + pub info: Arc>>, } impl<'tcx> QueryLatch<'tcx> { @@ -250,7 +156,7 @@ impl<'tcx> QueryLatch<'tcx> { /// Removes a single waiter from the list of waiters. /// This is used to break query cycles. - fn extract_waiter(&self, waiter: usize) -> Arc> { + pub fn extract_waiter(&self, waiter: usize) -> Arc> { let mut info = self.info.lock(); debug_assert!(!info.complete); // Remove the waiter from the list of waiters @@ -258,286 +164,6 @@ impl<'tcx> QueryLatch<'tcx> { } } -/// 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_ref` -/// and a span indicating the reason the query waited on `query_ref`. -/// If `visit` returns Some, this function returns. -/// For visits of non-resumable waiters it returns the return value of `visit`. -/// For visits of resumable waiters it returns Some(Some(Waiter)) which has the -/// required information to resume the waiter. -/// If all `visit` calls returns None, this function also returns None. -fn visit_waiters<'tcx, F>( - query_map: &QueryMap<'tcx>, - query: QueryJobId, - mut visit: F, -) -> Option> -where - F: FnMut(Span, QueryJobId) -> Option>, -{ - // Visit the parent query which is a non-resumable waiter since it's on the same stack - if let Some(parent) = query.parent(query_map) - && let Some(cycle) = visit(query.span(query_map), parent) - { - return Some(cycle); - } - - // Visit the explicit waiters which use condvars and are resumable - if let Some(latch) = query.latch(query_map) { - for (i, waiter) in latch.info.lock().waiters.iter().enumerate() { - if let Some(waiter_query) = waiter.query { - if visit(waiter.span, waiter_query).is_some() { - // Return a value which indicates that this waiter can be resumed - return Some(Some((query, i))); - } - } - } - } - - None -} - -/// 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>( - query_map: &QueryMap<'tcx>, - query: QueryJobId, - span: Span, - stack: &mut Vec<(Span, QueryJobId)>, - visited: &mut FxHashSet, -) -> Option> { - 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; - Some(None) - } else { - None - }; - } - - // Query marked as visited is added it to the stack - stack.push((span, query)); - - // Visit all the waiters - let r = visit_waiters(query_map, query, |span, successor| { - cycle_check(query_map, successor, span, stack, visited) - }); - - // Remove the entry in our stack if we didn't find a cycle - if r.is_none() { - 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>( - query_map: &QueryMap<'tcx>, - query: QueryJobId, - visited: &mut FxHashSet, -) -> bool { - // We already visited this or we're deliberately ignoring it - if !visited.insert(query) { - return false; - } - - // This query is connected to the root (it has no query parent), return true - if query.parent(query_map).is_none() { - return true; - } - - visit_waiters(query_map, query, |_, successor| { - connected_to_root(query_map, successor, visited).then_some(None) - }) - .is_some() -} - -// Deterministically pick an query from a list -fn pick_query<'a, 'tcx, T, F>(query_map: &QueryMap<'tcx>, queries: &'a [T], f: F) -> &'a T -where - F: Fn(&T) -> (Span, QueryJobId), -{ - // Deterministically pick an entry point - // FIXME: Sort this instead - queries - .iter() - .min_by_key(|v| { - let (span, query) = f(v); - let hash = query.frame(query_map).hash; - // Prefer entry points which have valid spans for nicer error messages - // We add an integer to the tuple ensuring that entry points - // with valid spans are picked first - let span_cmp = if span == DUMMY_SP { 1 } else { 0 }; - (span_cmp, hash) - }) - .unwrap() -} - -/// 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>( - query_map: &QueryMap<'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 Some(waiter) = - cycle_check(query_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); - } - } - - // Find the queries in the cycle which are - // connected to queries outside the cycle - let entry_points = stack - .iter() - .filter_map(|&(span, query)| { - if query.parent(query_map).is_none() { - // This query is connected to the root (it has no query parent) - Some((span, query, None)) - } else { - let mut waiters = Vec::new(); - // Find all the direct waiters who lead to the root - visit_waiters(query_map, query, |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(query_map, waiter, &mut visited) { - waiters.push((span, waiter)); - } - - None - }); - if waiters.is_empty() { - None - } else { - // Deterministically pick one of the waiters to show to the user - let waiter = *pick_query(query_map, &waiters, |s| *s); - Some((span, query, Some(waiter))) - } - } - }) - .collect::)>>(); - - // Deterministically pick an entry point - let (_, entry_point, usage) = pick_query(query_map, &entry_points, |e| (e.0, e.1)); - - // Shift the stack so that our entry point is first - let entry_point_pos = stack.iter().position(|(_, query)| query == entry_point); - if let Some(pos) = entry_point_pos { - stack.rotate_left(pos); - } - - let usage = usage.as_ref().map(|(span, query)| (*span, query.frame(query_map))); - - // Create the cycle error - let error = CycleError { - usage, - cycle: stack - .iter() - .map(|&(s, ref q)| QueryInfo { span: s, frame: q.frame(query_map) }) - .collect(), - }; - - // 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 = waitee_query.latch(query_map).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>(query_map: QueryMap<'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 = query_map.keys().cloned().collect(); - - let mut found_cycle = false; - - while jobs.len() > 0 { - if remove_cycle(&query_map, &mut jobs, &mut wakelist) { - found_cycle = true; - } - } - - // 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{:#?}", - query_map - ); - } - - // 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); - } - - for waiter in wakelist.into_iter() { - waiter.condvar.notify_one(); - } -} - #[inline(never)] #[cold] pub fn report_cycle<'a>( @@ -588,61 +214,3 @@ pub fn report_cycle<'a>( sess.dcx().create_err(cycle_diag) } - -pub fn print_query_stack<'tcx, Qcx: QueryContext<'tcx>>( - qcx: Qcx, - mut current_query: Option, - dcx: DiagCtxtHandle<'_>, - limit_frames: Option, - mut file: Option, -) -> usize { - // Be careful relying on global state here: this code is called from - // a panic hook, which means that the global `DiagCtxt` may be in a weird - // state if it was responsible for triggering the panic. - let mut count_printed = 0; - let mut count_total = 0; - - // Make use of a partial query map if we fail to take locks collecting active queries. - let query_map = match qcx.collect_active_jobs_from_all_queries(false) { - Ok(query_map) => query_map, - Err(query_map) => query_map, - }; - - if let Some(ref mut file) = file { - let _ = writeln!(file, "\n\nquery stack during panic:"); - } - while let Some(query) = current_query { - let Some(query_info) = query_map.get(&query) else { - break; - }; - let query_extra = query_info.frame.info.extract(); - if Some(count_printed) < limit_frames || limit_frames.is_none() { - // Only print to stderr as many stack frames as `num_frames` when present. - dcx.struct_failure_note(format!( - "#{} [{:?}] {}", - count_printed, query_info.frame.dep_kind, query_extra.description - )) - .with_span(query_info.job.span) - .emit(); - count_printed += 1; - } - - if let Some(ref mut file) = file { - let _ = writeln!( - file, - "#{} [{}] {}", - count_total, - qcx.dep_context().dep_kind_vtable(query_info.frame.dep_kind).name, - query_extra.description - ); - } - - current_query = query_info.job.parent; - count_total += 1; - } - - if let Some(ref mut file) = file { - let _ = writeln!(file, "end of query stack"); - } - count_total -} diff --git a/compiler/rustc_query_system/src/query/mod.rs b/compiler/rustc_query_system/src/query/mod.rs index f4a3fda7e372..6067f7dafd5c 100644 --- a/compiler/rustc_query_system/src/query/mod.rs +++ b/compiler/rustc_query_system/src/query/mod.rs @@ -15,10 +15,7 @@ use rustc_span::def_id::DefId; pub use self::caches::{ DefIdCache, DefaultCache, QueryCache, QueryCacheKey, SingleCache, VecCache, }; -pub use self::job::{ - QueryInfo, QueryJob, QueryJobId, QueryJobInfo, QueryLatch, QueryMap, break_query_cycles, - print_query_stack, report_cycle, -}; +pub use self::job::{QueryInfo, QueryJob, QueryJobId, QueryLatch, QueryWaiter, report_cycle}; pub use self::plumbing::*; use crate::dep_graph::{DepKind, DepNodeIndex, HasDepContext, SerializedDepNodeIndex}; @@ -52,7 +49,7 @@ pub struct QueryStackFrame { pub dep_kind: DepKind, /// This hash is used to deterministically pick /// a query to remove cycles in the parallel compiler. - hash: Hash64, + pub hash: Hash64, pub def_id: Option, /// A def-id that is extracted from a `Ty` in a query key pub def_id_for_ty_in_cycle: Option, @@ -161,11 +158,6 @@ pub trait QueryContext<'tcx>: HasDepContext { /// a token while waiting on a query. fn jobserver_proxy(&self) -> &Proxy; - fn collect_active_jobs_from_all_queries( - self, - require_complete: bool, - ) -> Result, QueryMap<'tcx>>; - /// Load a side effect associated to the node in the previous session. fn load_side_effect( self,