Skip to content

More thread-safety changes #49390

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

Merged
merged 10 commits into from
Apr 10, 2018
Merged
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
14 changes: 10 additions & 4 deletions src/librustc/hir/map/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,11 @@ use hir::svh::Svh;
use util::nodemap::{DefIdMap, FxHashMap};

use arena::TypedArena;
use std::cell::RefCell;
use std::io;
use ty::TyCtxt;

use rustc_data_structures::sync::Lock;

pub mod blocks;
mod collector;
mod def_collector;
Expand Down Expand Up @@ -264,7 +265,7 @@ pub struct Map<'hir> {
definitions: &'hir Definitions,

/// Bodies inlined from other crates are cached here.
inlined_bodies: RefCell<DefIdMap<&'hir Body>>,
inlined_bodies: Lock<DefIdMap<&'hir Body>>,

/// The reverse mapping of `node_to_hir_id`.
hir_to_node_id: FxHashMap<HirId, NodeId>,
Expand Down Expand Up @@ -927,8 +928,13 @@ impl<'hir> Map<'hir> {
}

pub fn intern_inlined_body(&self, def_id: DefId, body: Body) -> &'hir Body {
let mut inlined_bodies = self.inlined_bodies.borrow_mut();
if let Some(&b) = inlined_bodies.get(&def_id) {
debug_assert_eq!(&body, b);
Copy link
Member

Choose a reason for hiding this comment

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

How can this condition hold? body is on the stack and b is in the arena. Am I overlooking something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This isn't a pointer comparison. This check ensures that we always give the same body for the same def_id to intern_inlined_body.

Copy link
Member

Choose a reason for hiding this comment

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

Oh, you're right of course :)

return b;
}
let body = self.forest.inlined_bodies.alloc(body);
self.inlined_bodies.borrow_mut().insert(def_id, body);
inlined_bodies.insert(def_id, body);
body
}

Expand Down Expand Up @@ -1189,7 +1195,7 @@ pub fn map_crate<'hir>(sess: &::session::Session,
map,
hir_to_node_id,
definitions,
inlined_bodies: RefCell::new(DefIdMap()),
inlined_bodies: Lock::new(DefIdMap()),
};

hir_id_validator::check_crate(&map);
Expand Down
7 changes: 4 additions & 3 deletions src/librustc/lint/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@
pub use self::Level::*;
pub use self::LintSource::*;

use rustc_data_structures::sync::Lrc;
use rustc_data_structures::sync::{self, Lrc};

use errors::{DiagnosticBuilder, DiagnosticId};
use hir::def_id::{CrateNum, LOCAL_CRATE};
Expand Down Expand Up @@ -287,8 +287,9 @@ pub trait EarlyLintPass: LintPass {
}

/// A lint pass boxed up as a trait object.
pub type EarlyLintPassObject = Box<dyn EarlyLintPass + 'static>;
pub type LateLintPassObject = Box<dyn for<'a, 'tcx> LateLintPass<'a, 'tcx> + 'static>;
pub type EarlyLintPassObject = Box<dyn EarlyLintPass + sync::Send + sync::Sync + 'static>;
pub type LateLintPassObject = Box<dyn for<'a, 'tcx> LateLintPass<'a, 'tcx> + sync::Send
+ sync::Sync + 'static>;

/// Identifies a lint known to the compiler.
#[derive(Clone, Copy, Debug)]
Expand Down
10 changes: 5 additions & 5 deletions src/librustc/mir/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::cell::{Ref, RefCell};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::sync::{RwLock, ReadGuard};
use rustc_data_structures::stable_hasher::{HashStable, StableHasher,
StableHasherResult};
use ich::StableHashingContext;
Expand All @@ -19,7 +19,7 @@ use rustc_serialize as serialize;

#[derive(Clone, Debug)]
pub struct Cache {
predecessors: RefCell<Option<IndexVec<BasicBlock, Vec<BasicBlock>>>>
predecessors: RwLock<Option<IndexVec<BasicBlock, Vec<BasicBlock>>>>
}


Expand All @@ -46,7 +46,7 @@ impl<'a> HashStable<StableHashingContext<'a>> for Cache {
impl Cache {
pub fn new() -> Self {
Cache {
predecessors: RefCell::new(None)
predecessors: RwLock::new(None)
}
}

Expand All @@ -55,12 +55,12 @@ impl Cache {
*self.predecessors.borrow_mut() = None;
}

pub fn predecessors(&self, mir: &Mir) -> Ref<IndexVec<BasicBlock, Vec<BasicBlock>>> {
pub fn predecessors(&self, mir: &Mir) -> ReadGuard<IndexVec<BasicBlock, Vec<BasicBlock>>> {
if self.predecessors.borrow().is_none() {
*self.predecessors.borrow_mut() = Some(calculate_predecessors(mir));
}

Ref::map(self.predecessors.borrow(), |p| p.as_ref().unwrap())
ReadGuard::map(self.predecessors.borrow(), |p| p.as_ref().unwrap())
}
}

Expand Down
8 changes: 4 additions & 4 deletions src/librustc/mir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ use util::ppaux;
use std::slice;
use hir::{self, InlineAsm};
use std::borrow::{Cow};
use std::cell::Ref;
use rustc_data_structures::sync::ReadGuard;
use std::fmt::{self, Debug, Formatter, Write};
use std::{iter, mem, u32};
use std::ops::{Index, IndexMut};
Expand Down Expand Up @@ -187,13 +187,13 @@ impl<'tcx> Mir<'tcx> {
}

#[inline]
pub fn predecessors(&self) -> Ref<IndexVec<BasicBlock, Vec<BasicBlock>>> {
pub fn predecessors(&self) -> ReadGuard<IndexVec<BasicBlock, Vec<BasicBlock>>> {
self.cache.predecessors(self)
}

#[inline]
pub fn predecessors_for(&self, bb: BasicBlock) -> Ref<Vec<BasicBlock>> {
Ref::map(self.predecessors(), |p| &p[bb])
pub fn predecessors_for(&self, bb: BasicBlock) -> ReadGuard<Vec<BasicBlock>> {
ReadGuard::map(self.predecessors(), |p| &p[bb])
}

#[inline]
Expand Down
50 changes: 25 additions & 25 deletions src/librustc/ty/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use rustc_data_structures::stable_hasher::{HashStable, hash_stable_hashmap,
StableVec};
use arena::{TypedArena, DroplessArena};
use rustc_data_structures::indexed_vec::IndexVec;
use rustc_data_structures::sync::Lrc;
use rustc_data_structures::sync::{Lrc, Lock};
use std::any::Any;
use std::borrow::Borrow;
use std::cell::{Cell, RefCell};
Expand Down Expand Up @@ -130,28 +130,28 @@ pub struct CtxtInterners<'tcx> {

/// Specifically use a speedy hash algorithm for these hash sets,
/// they're accessed quite often.
type_: RefCell<FxHashSet<Interned<'tcx, TyS<'tcx>>>>,
type_list: RefCell<FxHashSet<Interned<'tcx, Slice<Ty<'tcx>>>>>,
substs: RefCell<FxHashSet<Interned<'tcx, Substs<'tcx>>>>,
canonical_var_infos: RefCell<FxHashSet<Interned<'tcx, Slice<CanonicalVarInfo>>>>,
region: RefCell<FxHashSet<Interned<'tcx, RegionKind>>>,
existential_predicates: RefCell<FxHashSet<Interned<'tcx, Slice<ExistentialPredicate<'tcx>>>>>,
predicates: RefCell<FxHashSet<Interned<'tcx, Slice<Predicate<'tcx>>>>>,
const_: RefCell<FxHashSet<Interned<'tcx, Const<'tcx>>>>,
type_: Lock<FxHashSet<Interned<'tcx, TyS<'tcx>>>>,
type_list: Lock<FxHashSet<Interned<'tcx, Slice<Ty<'tcx>>>>>,
substs: Lock<FxHashSet<Interned<'tcx, Substs<'tcx>>>>,
canonical_var_infos: Lock<FxHashSet<Interned<'tcx, Slice<CanonicalVarInfo>>>>,
region: Lock<FxHashSet<Interned<'tcx, RegionKind>>>,
existential_predicates: Lock<FxHashSet<Interned<'tcx, Slice<ExistentialPredicate<'tcx>>>>>,
predicates: Lock<FxHashSet<Interned<'tcx, Slice<Predicate<'tcx>>>>>,
const_: Lock<FxHashSet<Interned<'tcx, Const<'tcx>>>>,
}

impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
fn new(arena: &'tcx DroplessArena) -> CtxtInterners<'tcx> {
CtxtInterners {
arena,
type_: RefCell::new(FxHashSet()),
type_list: RefCell::new(FxHashSet()),
substs: RefCell::new(FxHashSet()),
region: RefCell::new(FxHashSet()),
existential_predicates: RefCell::new(FxHashSet()),
canonical_var_infos: RefCell::new(FxHashSet()),
predicates: RefCell::new(FxHashSet()),
const_: RefCell::new(FxHashSet()),
arena: arena,
type_: Lock::new(FxHashSet()),
type_list: Lock::new(FxHashSet()),
substs: Lock::new(FxHashSet()),
canonical_var_infos: Lock::new(FxHashSet()),
region: Lock::new(FxHashSet()),
existential_predicates: Lock::new(FxHashSet()),
predicates: Lock::new(FxHashSet()),
const_: Lock::new(FxHashSet()),
}
}

Expand Down Expand Up @@ -891,11 +891,11 @@ pub struct GlobalCtxt<'tcx> {
/// by `proc-macro` crates.
pub derive_macros: RefCell<NodeMap<Symbol>>,

stability_interner: RefCell<FxHashSet<&'tcx attr::Stability>>,
stability_interner: Lock<FxHashSet<&'tcx attr::Stability>>,

pub interpret_interner: InterpretInterner<'tcx>,

layout_interner: RefCell<FxHashSet<&'tcx LayoutDetails>>,
layout_interner: Lock<FxHashSet<&'tcx LayoutDetails>>,

/// A vector of every trait accessible in the whole crate
/// (i.e. including those from subcrates). This is used only for
Expand All @@ -909,15 +909,15 @@ pub struct GlobalCtxt<'tcx> {
/// This is intended to only get used during the trans phase of the compiler
/// when satisfying the query for a particular codegen unit. Internally in
/// the query it'll send data along this channel to get processed later.
pub tx_to_llvm_workers: mpsc::Sender<Box<dyn Any + Send>>,
pub tx_to_llvm_workers: Lock<mpsc::Sender<Box<dyn Any + Send>>>,

output_filenames: Arc<OutputFilenames>,
}

/// Everything needed to efficiently work with interned allocations
#[derive(Debug, Default)]
pub struct InterpretInterner<'tcx> {
inner: RefCell<InterpretInternerInner<'tcx>>,
inner: Lock<InterpretInternerInner<'tcx>>,
}

#[derive(Debug, Default)]
Expand Down Expand Up @@ -1277,13 +1277,13 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
evaluation_cache: traits::EvaluationCache::new(),
crate_name: Symbol::intern(crate_name),
data_layout,
layout_interner: RefCell::new(FxHashSet()),
layout_interner: Lock::new(FxHashSet()),
layout_depth: Cell::new(0),
derive_macros: RefCell::new(NodeMap()),
stability_interner: RefCell::new(FxHashSet()),
stability_interner: Lock::new(FxHashSet()),
interpret_interner: Default::default(),
all_traits: RefCell::new(None),
tx_to_llvm_workers: tx,
tx_to_llvm_workers: Lock::new(tx),
output_filenames: Arc::new(output_filenames.clone()),
}, f)
}
Expand Down
12 changes: 6 additions & 6 deletions src/librustc/ty/steal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
// option. This file may not be copied, modified, or distributed
// except according to those terms.

use std::cell::{Ref, RefCell};
use rustc_data_structures::sync::{RwLock, ReadGuard};
use std::mem;

/// The `Steal` struct is intended to used as the value for a query.
Expand All @@ -32,25 +32,25 @@ use std::mem;
///
/// FIXME(#41710) -- what is the best way to model linear queries?
pub struct Steal<T> {
value: RefCell<Option<T>>
value: RwLock<Option<T>>
}

impl<T> Steal<T> {
pub fn new(value: T) -> Self {
Steal {
value: RefCell::new(Some(value))
value: RwLock::new(Some(value))
}
}

pub fn borrow(&self) -> Ref<T> {
Ref::map(self.value.borrow(), |opt| match *opt {
pub fn borrow(&self) -> ReadGuard<T> {
ReadGuard::map(self.value.borrow(), |opt| match *opt {
None => bug!("attempted to read from stolen value"),
Some(ref v) => v
})
}

pub fn steal(&self) -> T {
let value_ref = &mut *self.value.borrow_mut();
let value_ref = &mut *self.value.try_write().expect("stealing value which is locked");
let value = mem::replace(value_ref, None);
value.expect("attempt to read from stolen value")
}
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_borrowck/borrowck/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ fn borrowck<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>, owner_def_id: DefId)
// Note that `mir_validated` is a "stealable" result; the
// thief, `optimized_mir()`, forces borrowck, so we know that
// is not yet stolen.
tcx.mir_validated(owner_def_id).borrow();
ty::maps::queries::mir_validated::ensure(tcx, owner_def_id);

// option dance because you can't capture an uninitialized variable
// by mut-ref.
Expand Down
12 changes: 12 additions & 0 deletions src/librustc_data_structures/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -388,6 +388,18 @@ impl<T> RwLock<T> {
f(&*self.read())
}

#[cfg(not(parallel_queries))]
#[inline(always)]
pub fn try_write(&self) -> Result<WriteGuard<T>, ()> {
self.0.try_borrow_mut().map_err(|_| ())
}

#[cfg(parallel_queries)]
#[inline(always)]
pub fn try_write(&self) -> Result<WriteGuard<T>, ()> {
self.0.try_write().ok_or(())
}

#[cfg(not(parallel_queries))]
#[inline(always)]
pub fn write(&self) -> WriteGuard<T> {
Expand Down
6 changes: 3 additions & 3 deletions src/librustc_trans/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1035,7 +1035,7 @@ pub fn start_async_translation(tcx: TyCtxt,
crate_info,

time_graph,
coordinator_send: tcx.tx_to_llvm_workers.clone(),
coordinator_send: tcx.tx_to_llvm_workers.lock().clone(),
trans_worker_receive,
shared_emitter_main,
future: coordinator_thread,
Expand Down Expand Up @@ -1428,7 +1428,7 @@ fn start_executing_work(tcx: TyCtxt,
metadata_config: Arc<ModuleConfig>,
allocator_config: Arc<ModuleConfig>)
-> thread::JoinHandle<Result<CompiledModules, ()>> {
let coordinator_send = tcx.tx_to_llvm_workers.clone();
let coordinator_send = tcx.tx_to_llvm_workers.lock().clone();
let sess = tcx.sess;

// Compute the set of symbols we need to retain when doing LTO (if we need to)
Expand Down Expand Up @@ -2340,7 +2340,7 @@ pub(crate) fn submit_translated_module_to_llvm(tcx: TyCtxt,
mtrans: ModuleTranslation,
cost: u64) {
let llvm_work_item = WorkItem::Optimize(mtrans);
drop(tcx.tx_to_llvm_workers.send(Box::new(Message::TranslationDone {
drop(tcx.tx_to_llvm_workers.lock().send(Box::new(Message::TranslationDone {
llvm_work_item,
cost,
})));
Expand Down
Loading