Auto merge of #49882 - Zoxc:sync-misc2, r=michaelwoerister

More thread-safety changes r? @michaelwoerister
rust-lang · Apr 17, 2018 · 6b12d36 · 6b12d36
2 parents 186db76 + fe63637
commit 6b12d36
Show file tree

Hide file tree

Showing 13 changed files with 159 additions and 53 deletions.
diff --git a/src/Cargo.lock b/src/Cargo.lock
diff --git a/src/libarena/Cargo.toml b/src/libarena/Cargo.toml
@@ -7,3 +7,6 @@ version = "0.0.0"
 name = "arena"
 path = "lib.rs"
 crate-type = ["dylib"]
+
+[dependencies]
+rustc_data_structures = { path = "../librustc_data_structures" }
diff --git a/src/libarena/lib.rs b/src/libarena/lib.rs
@@ -32,6 +32,9 @@
 #![allow(deprecated)]
 
 extern crate alloc;
+extern crate rustc_data_structures;
+
+use rustc_data_structures::sync::MTLock;
 
 use std::cell::{Cell, RefCell};
 use std::cmp;
@@ -290,6 +293,8 @@ pub struct DroplessArena {
     chunks: RefCell<Vec<TypedArenaChunk<u8>>>,
 }
 
+unsafe impl Send for DroplessArena {}
+
 impl DroplessArena {
     pub fn new() -> DroplessArena {
         DroplessArena {
@@ -410,6 +415,72 @@ impl DroplessArena {
     }
 }
 
+pub struct SyncTypedArena<T> {
+    lock: MTLock<TypedArena<T>>,
+}
+
+impl<T> SyncTypedArena<T> {
+    #[inline(always)]
+    pub fn new() -> SyncTypedArena<T> {
+        SyncTypedArena {
+            lock: MTLock::new(TypedArena::new())
+        }
+    }
+
+    #[inline(always)]
+    pub fn alloc(&self, object: T) -> &mut T {
+        // Extend the lifetime of the result since it's limited to the lock guard
+        unsafe { &mut *(self.lock.lock().alloc(object) as *mut T) }
+    }
+
+    #[inline(always)]
+    pub fn alloc_slice(&self, slice: &[T]) -> &mut [T]
+    where
+        T: Copy,
+    {
+        // Extend the lifetime of the result since it's limited to the lock guard
+        unsafe { &mut *(self.lock.lock().alloc_slice(slice) as *mut [T]) }
+    }
+
+    #[inline(always)]
+    pub fn clear(&mut self) {
+        self.lock.get_mut().clear();
+    }
+}
+
+pub struct SyncDroplessArena {
+    lock: MTLock<DroplessArena>,
+}
+
+impl SyncDroplessArena {
+    #[inline(always)]
+    pub fn new() -> SyncDroplessArena {
+        SyncDroplessArena {
+            lock: MTLock::new(DroplessArena::new())
+        }
+    }
+
+    #[inline(always)]
+    pub fn in_arena<T: ?Sized>(&self, ptr: *const T) -> bool {
+        self.lock.lock().in_arena(ptr)
+    }
+
+    #[inline(always)]
+    pub fn alloc<T>(&self, object: T) -> &mut T {
+        // Extend the lifetime of the result since it's limited to the lock guard
+        unsafe { &mut *(self.lock.lock().alloc(object) as *mut T) }
+    }
+
+    #[inline(always)]
+    pub fn alloc_slice<T>(&self, slice: &[T]) -> &mut [T]
+    where
+        T: Copy,
+    {
+        // Extend the lifetime of the result since it's limited to the lock guard
+        unsafe { &mut *(self.lock.lock().alloc_slice(slice) as *mut [T]) }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     extern crate test;

diff --git a/src/librustc/hir/map/mod.rs b/src/librustc/hir/map/mod.rs
@@ -32,7 +32,7 @@ use hir::print::Nested;
 use hir::svh::Svh;
 use util::nodemap::{DefIdMap, FxHashMap};
 
-use arena::TypedArena;
+use arena::SyncTypedArena;
 use std::io;
 use ty::TyCtxt;
 
@@ -219,15 +219,15 @@ impl<'hir> MapEntry<'hir> {
 pub struct Forest {
     krate: Crate,
     pub dep_graph: DepGraph,
-    inlined_bodies: TypedArena<Body>
+    inlined_bodies: SyncTypedArena<Body>
 }
 
 impl Forest {
     pub fn new(krate: Crate, dep_graph: &DepGraph) -> Forest {
         Forest {
             krate,
             dep_graph: dep_graph.clone(),
-            inlined_bodies: TypedArena::new()
+            inlined_bodies: SyncTypedArena::new()
         }
     }
 

diff --git a/src/librustc/infer/mod.rs b/src/librustc/infer/mod.rs
@@ -37,7 +37,7 @@ use errors::DiagnosticBuilder;
 use syntax_pos::{self, Span};
 use syntax_pos::symbol::InternedString;
 use util::nodemap::FxHashMap;
-use arena::DroplessArena;
+use arena::SyncDroplessArena;
 
 use self::combine::CombineFields;
 use self::higher_ranked::HrMatchResult;
@@ -407,15 +407,15 @@ impl fmt::Display for FixupError {
 /// F: for<'b, 'tcx> where 'gcx: 'tcx FnOnce(InferCtxt<'b, 'gcx, 'tcx>).
 pub struct InferCtxtBuilder<'a, 'gcx: 'a+'tcx, 'tcx: 'a> {
     global_tcx: TyCtxt<'a, 'gcx, 'gcx>,
-    arena: DroplessArena,
+    arena: SyncDroplessArena,
     fresh_tables: Option<RefCell<ty::TypeckTables<'tcx>>>,
 }
 
 impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'gcx> {
     pub fn infer_ctxt(self) -> InferCtxtBuilder<'a, 'gcx, 'tcx> {
         InferCtxtBuilder {
             global_tcx: self,
-            arena: DroplessArena::new(),
+            arena: SyncDroplessArena::new(),
             fresh_tables: None,
 
         }

diff --git a/src/librustc/lint/context.rs b/src/librustc/lint/context.rs
@@ -27,6 +27,7 @@
 use self::TargetLint::*;
 
 use std::slice;
+use rustc_data_structures::sync::{RwLock, ReadGuard};
 use lint::{EarlyLintPassObject, LateLintPassObject};
 use lint::{Level, Lint, LintId, LintPass, LintBuffer};
 use lint::builtin::BuiltinLintDiagnostics;
@@ -39,7 +40,6 @@ use ty::layout::{LayoutError, LayoutOf, TyLayout};
 use util::nodemap::FxHashMap;
 
 use std::default::Default as StdDefault;
-use std::cell::{Ref, RefCell};
 use syntax::ast;
 use syntax::edition;
 use syntax_pos::{MultiSpan, Span};
@@ -78,7 +78,7 @@ pub struct LintStore {
 
 pub struct LintSession<'a, PassObject> {
     /// Reference to the store of registered lints.
-    lints: Ref<'a, LintStore>,
+    lints: ReadGuard<'a, LintStore>,
 
     /// Trait objects for each lint pass.
     passes: Option<Vec<PassObject>>,
@@ -336,7 +336,7 @@ impl<'a, PassObject: LintPassObject> LintSession<'a, PassObject> {
     /// Creates a new `LintSession`, by moving out the `LintStore`'s initial
     /// lint levels and pass objects. These can be restored using the `restore`
     /// method.
-    fn new(store: &'a RefCell<LintStore>) -> LintSession<'a, PassObject> {
+    fn new(store: &'a RwLock<LintStore>) -> LintSession<'a, PassObject> {
         let mut s = store.borrow_mut();
         let passes = PassObject::take_passes(&mut *s);
         drop(s);
@@ -347,7 +347,7 @@ impl<'a, PassObject: LintPassObject> LintSession<'a, PassObject> {
     }
 
     /// Restores the levels back to the original lint store.
-    fn restore(self, store: &RefCell<LintStore>) {
+    fn restore(self, store: &RwLock<LintStore>) {
         drop(self.lints);
         let mut s = store.borrow_mut();
         PassObject::restore_passes(&mut *s, self.passes);

diff --git a/src/librustc/session/mod.rs b/src/librustc/session/mod.rs
@@ -26,7 +26,7 @@ use util::nodemap::{FxHashSet};
 use util::common::{duration_to_secs_str, ErrorReported};
 use util::common::ProfileQueriesMsg;
 
-use rustc_data_structures::sync::{Lrc, Lock, LockCell, OneThread, Once};
+use rustc_data_structures::sync::{Lrc, Lock, LockCell, OneThread, Once, RwLock};
 
 use syntax::ast::NodeId;
 use errors::{self, DiagnosticBuilder, DiagnosticId};
@@ -83,8 +83,8 @@ pub struct Session {
 
     // FIXME: lint_store and buffered_lints are not thread-safe,
     // but are only used in a single thread
-    pub lint_store: OneThread<RefCell<lint::LintStore>>,
-    pub buffered_lints: OneThread<RefCell<Option<lint::LintBuffer>>>,
+    pub lint_store: RwLock<lint::LintStore>,
+    pub buffered_lints: Lock<Option<lint::LintBuffer>>,
 
     /// Set of (DiagnosticId, Option<Span>, message) tuples tracking
     /// (sub)diagnostics that have been set once, but should not be set again,
@@ -1089,8 +1089,8 @@ pub fn build_session_(
         default_sysroot,
         local_crate_source_file,
         working_dir,
-        lint_store: OneThread::new(RefCell::new(lint::LintStore::new())),
-        buffered_lints: OneThread::new(RefCell::new(Some(lint::LintBuffer::new()))),
+        lint_store: RwLock::new(lint::LintStore::new()),
+        buffered_lints: Lock::new(Some(lint::LintBuffer::new())),
         one_time_diagnostics: RefCell::new(FxHashSet()),
         plugin_llvm_passes: OneThread::new(RefCell::new(Vec::new())),
         plugin_attributes: OneThread::new(RefCell::new(Vec::new())),

diff --git a/src/librustc/ty/context.rs b/src/librustc/ty/context.rs
@@ -57,12 +57,11 @@ use rustc_data_structures::accumulate_vec::AccumulateVec;
 use rustc_data_structures::stable_hasher::{HashStable, hash_stable_hashmap,
                                            StableHasher, StableHasherResult,
                                            StableVec};
-use arena::{TypedArena, DroplessArena};
+use arena::{TypedArena, SyncDroplessArena};
 use rustc_data_structures::indexed_vec::IndexVec;
 use rustc_data_structures::sync::{Lrc, Lock};
 use std::any::Any;
 use std::borrow::Borrow;
-use std::cell::Cell;
 use std::cmp::Ordering;
 use std::collections::hash_map::{self, Entry};
 use std::hash::{Hash, Hasher};
@@ -83,14 +82,14 @@ use hir;
 
 pub struct AllArenas<'tcx> {
     pub global: GlobalArenas<'tcx>,
-    pub interner: DroplessArena,
+    pub interner: SyncDroplessArena,
 }
 
 impl<'tcx> AllArenas<'tcx> {
     pub fn new() -> Self {
         AllArenas {
             global: GlobalArenas::new(),
-            interner: DroplessArena::new(),
+            interner: SyncDroplessArena::new(),
         }
     }
 }
@@ -130,7 +129,7 @@ type InternedSet<'tcx, T> = Lock<FxHashSet<Interned<'tcx, T>>>;
 
 pub struct CtxtInterners<'tcx> {
     /// The arena that types, regions, etc are allocated from
-    arena: &'tcx DroplessArena,
+    arena: &'tcx SyncDroplessArena,
 
     /// Specifically use a speedy hash algorithm for these hash sets,
     /// they're accessed quite often.
@@ -147,7 +146,7 @@ pub struct CtxtInterners<'tcx> {
 }
 
 impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
-    fn new(arena: &'tcx DroplessArena) -> CtxtInterners<'tcx> {
+    fn new(arena: &'tcx SyncDroplessArena) -> CtxtInterners<'tcx> {
         CtxtInterners {
             arena,
             type_: Default::default(),
@@ -174,10 +173,10 @@ impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
                 return ty;
             }
             let global_interner = global_interners.map(|interners| {
-                interners.type_.borrow_mut()
+                (interners.type_.borrow_mut(), &interners.arena)
             });
-            if let Some(ref interner) = global_interner {
-                if let Some(&Interned(ty)) = interner.get(&st) {
+            if let Some((ref type_, _)) = global_interner {
+                if let Some(&Interned(ty)) = type_.get(&st) {
                     return ty;
                 }
             }
@@ -193,18 +192,18 @@ impl<'gcx: 'tcx, 'tcx> CtxtInterners<'tcx> {
             // determine that all contents are in the global tcx.
             // See comments on Lift for why we can't use that.
             if !flags.flags.intersects(ty::TypeFlags::KEEP_IN_LOCAL_TCX) {
-                if let Some(interner) = global_interners {
+                if let Some((mut type_, arena)) = global_interner {
                     let ty_struct: TyS<'gcx> = unsafe {
                         mem::transmute(ty_struct)
                     };
-                    let ty: Ty<'gcx> = interner.arena.alloc(ty_struct);
-                    global_interner.unwrap().insert(Interned(ty));
+                    let ty: Ty<'gcx> = arena.alloc(ty_struct);
+                    type_.insert(Interned(ty));
                     return ty;
                 }
             } else {
                 // Make sure we don't end up with inference
                 // types/regions in the global tcx.
-                if global_interners.is_none() {
+                if global_interner.is_none() {
                     drop(interner);
                     bug!("Attempted to intern `{:?}` which contains \
                           inference types/regions in the global type context",
@@ -915,9 +914,6 @@ pub struct GlobalCtxt<'tcx> {
     /// Data layout specification for the current target.
     pub data_layout: TargetDataLayout,
 
-    /// Used to prevent layout from recursing too deeply.
-    pub layout_depth: Cell<usize>,
-
     stability_interner: Lock<FxHashSet<&'tcx attr::Stability>>,
 
     pub interpret_interner: InterpretInterner<'tcx>,
@@ -1292,7 +1288,6 @@ impl<'a, 'gcx, 'tcx> TyCtxt<'a, 'gcx, 'tcx> {
             crate_name: Symbol::intern(crate_name),
             data_layout,
             layout_interner: Lock::new(FxHashSet()),
-            layout_depth: Cell::new(0),
             stability_interner: Lock::new(FxHashSet()),
             interpret_interner: Default::default(),
             tx_to_llvm_workers: Lock::new(tx),
@@ -1559,7 +1554,7 @@ impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
     /// Call the closure with a local `TyCtxt` using the given arena.
     pub fn enter_local<F, R>(
         &self,
-        arena: &'tcx DroplessArena,
+        arena: &'tcx SyncDroplessArena,
         f: F
     ) -> R
     where
@@ -1574,6 +1569,7 @@ impl<'gcx: 'tcx, 'tcx> GlobalCtxt<'gcx> {
             let new_icx = ty::tls::ImplicitCtxt {
                 tcx,
                 query: icx.query.clone(),
+                layout_depth: icx.layout_depth,
             };
             ty::tls::enter_context(&new_icx, |new_icx| {
                 f(new_icx.tcx)
@@ -1768,6 +1764,9 @@ pub mod tls {
         /// The current query job, if any. This is updated by start_job in
         /// ty::maps::plumbing when executing a query
         pub query: Option<Lrc<maps::QueryJob<'gcx>>>,
+
+        /// Used to prevent layout from recursing too deeply.
+        pub layout_depth: usize,
     }
 
     // A thread local value which stores a pointer to the current ImplicitCtxt
@@ -1853,6 +1852,7 @@ pub mod tls {
             let icx = ImplicitCtxt {
                 tcx,
                 query: None,
+                layout_depth: 0,
             };
             enter_context(&icx, |_| {
                 f(tcx)