1
Fork 0

Address comments

This commit is contained in:
John Kåre Alsaker 2025-03-21 08:09:42 +01:00
parent 157008d711
commit 34244c1477
5 changed files with 33 additions and 21 deletions

View file

@ -193,15 +193,16 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce(CurrentGcx) -> R + Send,
let query_map = current_gcx2.access(|gcx| { let query_map = current_gcx2.access(|gcx| {
tls::enter_context(&tls::ImplicitCtxt::new(gcx), || { tls::enter_context(&tls::ImplicitCtxt::new(gcx), || {
tls::with(|tcx| { tls::with(|tcx| {
let (query_map, complete) = QueryCtxt::new(tcx).collect_active_jobs(); match QueryCtxt::new(tcx).collect_active_jobs() {
if !complete { Ok(query_map) => query_map,
// There was an unexpected error collecting all active jobs, which we need Err(_) => {
// to find cycles to break. // There was an unexpected error collecting all active jobs, which we need
// We want to avoid panicking in the deadlock handler, so we abort instead. // to find cycles to break.
eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process"); // We want to avoid panicking in the deadlock handler, so we abort instead.
process::abort(); eprintln!("internal compiler error: failed to get query map in deadlock handler, aborting process");
process::abort();
}
} }
query_map
}) })
}) })
}); });

View file

@ -79,17 +79,20 @@ impl QueryContext for QueryCtxt<'_> {
tls::with_related_context(self.tcx, |icx| icx.query) tls::with_related_context(self.tcx, |icx| icx.query)
} }
/// Returns a query map representing active query jobs and a bool being false /// Returns a query map representing active query jobs.
/// if there was an error constructing the map. /// It returns an incomplete map as an error if it fails
fn collect_active_jobs(self) -> (QueryMap, bool) { /// to take locks.
fn collect_active_jobs(self) -> Result<QueryMap, QueryMap> {
let mut jobs = QueryMap::default(); let mut jobs = QueryMap::default();
let mut complete = true; let mut complete = true;
for collect in super::TRY_COLLECT_ACTIVE_JOBS.iter() { for collect in super::TRY_COLLECT_ACTIVE_JOBS.iter() {
collect(self.tcx, &mut jobs, &mut complete); if collect(self.tcx, &mut jobs).is_none() {
complete = false;
}
} }
(jobs, complete) if complete { Ok(jobs) } else { Err(jobs) }
} }
// Interactions with on_disk_cache // Interactions with on_disk_cache
@ -143,7 +146,11 @@ impl QueryContext for QueryCtxt<'_> {
fn depth_limit_error(self, job: QueryJobId) { fn depth_limit_error(self, job: QueryJobId) {
// FIXME: `collect_active_jobs` expects no locks to be held, which doesn't hold for this call. // FIXME: `collect_active_jobs` expects no locks to be held, which doesn't hold for this call.
let (info, depth) = job.find_dep_kind_root(self.collect_active_jobs().0); let query_map = match self.collect_active_jobs() {
Ok(query_map) => query_map,
Err(query_map) => query_map,
};
let (info, depth) = job.find_dep_kind_root(query_map);
let suggested_limit = match self.recursion_limit() { let suggested_limit = match self.recursion_limit() {
Limit(0) => Limit(2), Limit(0) => Limit(2),
@ -681,7 +688,7 @@ macro_rules! define_queries {
} }
} }
pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap, complete: &mut bool) { pub(crate) fn try_collect_active_jobs<'tcx>(tcx: TyCtxt<'tcx>, qmap: &mut QueryMap) -> Option<()> {
let make_query = |tcx, key| { let make_query = |tcx, key| {
let kind = rustc_middle::dep_graph::dep_kinds::$name; let kind = rustc_middle::dep_graph::dep_kinds::$name;
let name = stringify!($name); let name = stringify!($name);
@ -696,12 +703,12 @@ macro_rules! define_queries {
// don't `unwrap()` here, just manually check for `None` and do best-effort error // don't `unwrap()` here, just manually check for `None` and do best-effort error
// reporting. // reporting.
if res.is_none() { if res.is_none() {
*complete = false;
tracing::warn!( tracing::warn!(
"Failed to collect active jobs for query with name `{}`!", "Failed to collect active jobs for query with name `{}`!",
stringify!($name) stringify!($name)
); );
} }
res
} }
pub(crate) fn alloc_self_profile_query_strings<'tcx>( pub(crate) fn alloc_self_profile_query_strings<'tcx>(
@ -761,7 +768,7 @@ macro_rules! define_queries {
// These arrays are used for iteration and can't be indexed by `DepKind`. // These arrays are used for iteration and can't be indexed by `DepKind`.
const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap, &mut bool)] = const TRY_COLLECT_ACTIVE_JOBS: &[for<'tcx> fn(TyCtxt<'tcx>, &mut QueryMap) -> Option<()>] =
&[$(query_impl::$name::try_collect_active_jobs),*]; &[$(query_impl::$name::try_collect_active_jobs),*];
const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[ const ALLOC_SELF_PROFILE_QUERY_STRINGS: &[

View file

@ -588,7 +588,12 @@ pub fn print_query_stack<Qcx: QueryContext>(
// state if it was responsible for triggering the panic. // state if it was responsible for triggering the panic.
let mut count_printed = 0; let mut count_printed = 0;
let mut count_total = 0; let mut count_total = 0;
let query_map = qcx.collect_active_jobs().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() {
Ok(query_map) => query_map,
Err(query_map) => query_map,
};
if let Some(ref mut file) = file { if let Some(ref mut file) = file {
let _ = writeln!(file, "\n\nquery stack during panic:"); let _ = writeln!(file, "\n\nquery stack during panic:");

View file

@ -86,7 +86,7 @@ pub trait QueryContext: HasDepContext {
/// Get the query information from the TLS context. /// Get the query information from the TLS context.
fn current_query_job(self) -> Option<QueryJobId>; fn current_query_job(self) -> Option<QueryJobId>;
fn collect_active_jobs(self) -> (QueryMap, bool); fn collect_active_jobs(self) -> Result<QueryMap, QueryMap>;
/// Load a side effect associated to the node in the previous session. /// Load a side effect associated to the node in the previous session.
fn load_side_effect( fn load_side_effect(

View file

@ -250,10 +250,9 @@ where
Q: QueryConfig<Qcx>, Q: QueryConfig<Qcx>,
Qcx: QueryContext, Qcx: QueryContext,
{ {
let (query_map, complete) = qcx.collect_active_jobs();
// Ensure there was no errors collecting all active jobs. // Ensure there was no errors collecting all active jobs.
// We need the complete map to ensure we find a cycle to break. // We need the complete map to ensure we find a cycle to break.
assert!(complete, "failed to collect active queries"); let query_map = qcx.collect_active_jobs().expect("failed to collect active queries");
let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span); let error = try_execute.find_cycle_in_stack(query_map, &qcx.current_query_job(), span);
(mk_cycle(query, qcx, error), None) (mk_cycle(query, qcx, error), None)