Skip to content
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.

Commit 7730356

Browse files
authoredAug 19, 2024
Rollup merge of rust-lang#129271 - futile:query-system/prevent-double-panic, r=michaelwoerister
Prevent double panic in query system, improve diagnostics I stumbled upon a double-panic in the query system while working on something else (rust-lang#129102), which hid the real error cause for what I was debugging. This PR remedies that, so unwinding should be able to present more errors. It shouldn't really be relevant for code that doesn't ICE.
2 parents 8dc8589 + 2bf2455 commit 7730356

File tree

2 files changed

+17
-4
lines changed

2 files changed

+17
-4
lines changed
 

‎compiler/rustc_query_impl/src/plumbing.rs

+8-2
Original file line numberDiff line numberDiff line change
@@ -702,11 +702,17 @@ macro_rules! define_queries {
702702
let name = stringify!($name);
703703
$crate::plumbing::create_query_frame(tcx, rustc_middle::query::descs::$name, key, kind, name)
704704
};
705-
tcx.query_system.states.$name.try_collect_active_jobs(
705+
let res = tcx.query_system.states.$name.try_collect_active_jobs(
706706
tcx,
707707
make_query,
708708
qmap,
709-
).unwrap();
709+
);
710+
// this can be called during unwinding, and the function has a `try_`-prefix, so
711+
// don't `unwrap()` here, just manually check for `None` and do best-effort error
712+
// reporting.
713+
if res.is_none() {
714+
tracing::warn!("Failed to collect active jobs for query with name `{}`!", stringify!($name));
715+
}
710716
}
711717

712718
pub fn alloc_self_profile_query_strings<'tcx>(tcx: TyCtxt<'tcx>, string_cache: &mut QueryKeyStringCache) {

‎compiler/rustc_query_system/src/query/plumbing.rs

+9-2
Original file line numberDiff line numberDiff line change
@@ -181,8 +181,15 @@ where
181181
cache.complete(key, result, dep_node_index);
182182

183183
let job = {
184-
let mut lock = state.active.lock_shard_by_value(&key);
185-
lock.remove(&key).unwrap().expect_job()
184+
let val = {
185+
// don't keep the lock during the `unwrap()` of the retrieved value, or we taint the
186+
// underlying shard.
187+
// since unwinding also wants to look at this map, this can also prevent a double
188+
// panic.
189+
let mut lock = state.active.lock_shard_by_value(&key);
190+
lock.remove(&key)
191+
};
192+
val.unwrap().expect_job()
186193
};
187194

188195
job.signal_complete();

0 commit comments

Comments
 (0)
Failed to load comments.