Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Give deadlock handler access to GlobalCtxt #121971

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 14 additions & 2 deletions compiler/rustc_interface/src/interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use rustc_span::symbol::sym;
use rustc_span::FileName;
use std::path::PathBuf;
use std::result;
use std::sync::atomic::AtomicPtr;
use std::sync::Arc;

pub type Result<T> = result::Result<T, ErrorGuaranteed>;
Expand All @@ -39,8 +40,12 @@ pub struct Compiler {
pub sess: Session,
pub codegen_backend: Box<dyn CodegenBackend>,
pub(crate) override_queries: Option<fn(&Session, &mut Providers)>,
pub compiler_for_deadlock_handler: Arc<AtomicPtr<()>>,
}

impl !Sync for Compiler {}
impl !rustc_data_structures::sync::DynSync for Compiler {}

/// Converts strings provided as `--cfg [cfgspec]` into a `Cfg`.
pub(crate) fn parse_cfg(dcx: &DiagCtxt, cfgs: Vec<String>) -> Cfg {
cfgs.into_iter()
Expand Down Expand Up @@ -331,9 +336,12 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
let early_dcx = EarlyDiagCtxt::new(config.opts.error_format);
early_dcx.initialize_checked_jobserver();

let compiler_for_deadlock_handler = Arc::<AtomicPtr<_>>::default();

util::run_in_thread_pool_with_globals(
config.opts.edition,
config.opts.unstable_opts.threads,
compiler_for_deadlock_handler.clone(),
|| {
crate::callbacks::setup_callbacks();

Expand Down Expand Up @@ -422,8 +430,12 @@ pub fn run_compiler<R: Send>(config: Config, f: impl FnOnce(&Compiler) -> R + Se
}
sess.lint_store = Some(Lrc::new(lint_store));

let compiler =
Compiler { sess, codegen_backend, override_queries: config.override_queries };
let compiler = Arc::new(Compiler {
sess,
codegen_backend,
override_queries: config.override_queries,
compiler_for_deadlock_handler,
});

rustc_span::set_source_map(compiler.sess.psess.clone_source_map(), move || {
// There are two paths out of `f`.
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#![feature(let_chains)]
#![feature(thread_spawn_unchecked)]
#![feature(try_blocks)]
#![feature(negative_impls)]

#[macro_use]
extern crate tracing;
Expand Down
13 changes: 13 additions & 0 deletions compiler/rustc_interface/src/queries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use rustc_session::Session;
use rustc_span::symbol::sym;
use std::any::Any;
use std::cell::{RefCell, RefMut};
use std::sync::atomic::Ordering;
use std::sync::Arc;

/// Represent the result of a query.
Expand Down Expand Up @@ -160,6 +161,12 @@ impl<'tcx> Queries<'tcx> {
&self.hir_arena,
);

let old = self
.compiler
.compiler_for_deadlock_handler
.swap(qcx as *const _ as *mut _, Ordering::Relaxed);
assert!(old.is_null());

qcx.enter(|tcx| {
let feed = tcx.feed_local_crate();
feed.crate_name(crate_name);
Expand Down Expand Up @@ -311,6 +318,12 @@ impl Compiler {
{
// Must declare `_timer` first so that it is dropped after `queries`.
let mut _timer = None;
let _guard = rustc_data_structures::defer(|| {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would make more sense for this to be declared after queries so that we store null before the destructor of GlobalCtxt gains unique access, even though it offers no synchronization.

// If we got here, there was no deadlock, so we can't be in a situation where a deadlock handler
// is accessing the `GlobalCtxt`.
self.compiler_for_deadlock_handler.store(std::ptr::null_mut(), Ordering::Relaxed);
});
assert!(self.compiler_for_deadlock_handler.load(Ordering::Relaxed).is_null());
let queries = Queries::new(self);
let ret = f(&queries);

Expand Down
18 changes: 12 additions & 6 deletions compiler/rustc_interface/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ use session::EarlyDiagCtxt;
use std::env;
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
use std::path::{Path, PathBuf};
use std::sync::atomic::{AtomicBool, Ordering};
use std::sync::OnceLock;
use std::sync::atomic::{AtomicBool, AtomicPtr, Ordering};
use std::sync::{Arc, OnceLock};
use std::thread;

/// Function pointer type that constructs a new CodegenBackend.
Expand Down Expand Up @@ -90,6 +90,7 @@ pub(crate) fn run_in_thread_with_globals<F: FnOnce() -> R + Send, R: Send>(
pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
edition: Edition,
_threads: usize,
_compiler: Arc<AtomicPtr<()>>,
f: F,
) -> R {
run_in_thread_with_globals(edition, f)
Expand All @@ -99,10 +100,11 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
edition: Edition,
threads: usize,
compiler: Arc<AtomicPtr<()>>,
f: F,
) -> R {
use rustc_data_structures::{jobserver, sync::FromDyn};
use rustc_middle::ty::tls;
use rustc_middle::ty::GlobalCtxt;
use rustc_query_impl::QueryCtxt;
use rustc_query_system::query::{deadlock, QueryContext};

Expand All @@ -122,11 +124,15 @@ pub(crate) fn run_in_thread_pool_with_globals<F: FnOnce() -> R + Send, R: Send>(
.acquire_thread_handler(jobserver::acquire_thread)
.release_thread_handler(jobserver::release_thread)
.num_threads(threads)
.deadlock_handler(|| {
.deadlock_handler(move || {
let compiler = compiler.load(Ordering::Relaxed);
assert!(!compiler.is_null());
let compiler = compiler as *const GlobalCtxt<'_>;
// On deadlock, creates a new thread and forwards information in thread
// locals to it. The new thread runs the deadlock handler.
let query_map =
FromDyn::from(tls::with(|tcx| QueryCtxt::new(tcx).collect_active_jobs()));
let query_map = FromDyn::from(unsafe {
(*compiler).enter(|tcx| QueryCtxt::new(tcx).collect_active_jobs())
});
let registry = rayon_core::Registry::current();
thread::spawn(move || deadlock(query_map.into_inner(), &registry));
});
Expand Down
Loading