Skip to content

Commit

Permalink
Auto merge of #98570 - SparrowLii:deadlock, r=cjgillot
Browse files Browse the repository at this point in the history
get rid of `tcx` in deadlock handler when parallel compilation

This is a very obscure and hard-to-trace problem that affects thread scheduling. If we copy `tcx` to the deadlock handler thread, it will perform unpredictable behavior and cause very weird problems when executing `try_collect_active_jobs`(For example, the deadlock handler thread suddenly preempts the content of the blocked worker thread and executes the unknown judgment branch, like #94654).
Fortunately we can avoid this behavior by precomputing `query_map`. This change fixes the following ui tests failure on my environment when set `parallel-compiler = true`:
```
    [ui] src/test\ui\async-await\no-const-async.rs
    [ui] src/test\ui\infinite\infinite-struct.rs
    [ui] src/test\ui\infinite\infinite-tag-type-recursion.rs
    [ui] src/test\ui\issues\issue-3008-1.rs
    [ui] src/test\ui\issues\issue-3008-2.rs
    [ui] src/test\ui\issues\issue-32326.rs
    [ui] src/test\ui\issues\issue-57271.rs
    [ui] src/test\ui\issues\issue-72554.rs
    [ui] src/test\ui\parser\fn-header-semantic-fail.rs
    [ui] src/test\ui\union\union-nonrepresentable.rs
```

Updates #75760
Fixes #94654
  • Loading branch information
bors committed Jul 3, 2022
2 parents 5f98537 + fbca21e commit 8c52a83
Show file tree
Hide file tree
Showing 4 changed files with 9 additions and 21 deletions.
20 changes: 6 additions & 14 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use rustc_errors::registry::Registry;
use rustc_middle::ty::tls;
use rustc_parse::validate_attr;
#[cfg(parallel_compiler)]
use rustc_query_impl::QueryCtxt;
use rustc_query_impl::{QueryContext, QueryCtxt};
use rustc_session as session;
use rustc_session::config::CheckCfg;
use rustc_session::config::{self, CrateType};
Expand Down Expand Up @@ -166,20 +166,12 @@ pub fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
unsafe fn handle_deadlock() {
let registry = rustc_rayon_core::Registry::current();

let context = tls::get_tlv();
assert!(context != 0);
rustc_data_structures::sync::assert_sync::<tls::ImplicitCtxt<'_, '_>>();
let icx: &tls::ImplicitCtxt<'_, '_> = &*(context as *const tls::ImplicitCtxt<'_, '_>);

let session_globals = rustc_span::with_session_globals(|sg| sg as *const _);
let session_globals = &*session_globals;
thread::spawn(move || {
tls::enter_context(icx, |_| {
rustc_span::set_session_globals_then(session_globals, || {
tls::with(|tcx| QueryCtxt::from_tcx(tcx).deadlock(&registry))
})
});
let query_map = tls::with(|tcx| {
QueryCtxt::from_tcx(tcx)
.try_collect_active_jobs()
.expect("active jobs shouldn't be locked in deadlock handler")
});
thread::spawn(move || rustc_query_impl::deadlock(query_map, &registry));
}

#[cfg(parallel_compiler)]
Expand Down
2 changes: 2 additions & 0 deletions compiler/rustc_query_impl/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ use rustc_span::Span;
mod plumbing;
pub use plumbing::QueryCtxt;
use rustc_query_system::query::*;
#[cfg(parallel_compiler)]
pub use rustc_query_system::query::{deadlock, QueryContext};

mod keys;
use keys::Key;
Expand Down
5 changes: 0 additions & 5 deletions compiler/rustc_query_impl/src/plumbing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,11 +132,6 @@ impl<'tcx> QueryCtxt<'tcx> {
self.queries.on_disk_cache.as_ref()
}

#[cfg(parallel_compiler)]
pub unsafe fn deadlock(self, registry: &rustc_rayon_core::Registry) {
rustc_query_system::query::deadlock(self, registry)
}

pub(super) fn encode_query_results(
self,
encoder: &mut on_disk_cache::CacheEncoder<'_, 'tcx>,
Expand Down
3 changes: 1 addition & 2 deletions compiler/rustc_query_system/src/query/job.rs
Original file line number Diff line number Diff line change
Expand Up @@ -492,14 +492,13 @@ fn remove_cycle(
/// 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.
#[cfg(parallel_compiler)]
pub fn deadlock<CTX: QueryContext>(tcx: CTX, registry: &rayon_core::Registry) {
pub fn deadlock(query_map: QueryMap, registry: &rayon_core::Registry) {
let on_panic = OnDrop(|| {
eprintln!("deadlock handler panicked, aborting process");
process::abort();
});

let mut wakelist = Vec::new();
let query_map = tcx.try_collect_active_jobs().unwrap();
let mut jobs: Vec<QueryJobId> = query_map.keys().cloned().collect();

let mut found_cycle = false;
Expand Down

0 comments on commit 8c52a83

Please sign in to comment.