From 4edb539159f26c366d6b0606a20e66fe727b78ec Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?John=20K=C3=A5re=20Alsaker?= Date: Thu, 15 Feb 2018 10:52:26 +0100 Subject: [PATCH] Make CrateMetadata thread-safe --- src/librustc_metadata/creader.rs | 29 +++++++++++++++------------- src/librustc_metadata/cstore.rs | 13 ++++++------- src/librustc_metadata/cstore_impl.rs | 24 ++++++++++++++++------- src/librustc_metadata/decoder.rs | 21 +++++++++++++++----- 4 files changed, 55 insertions(+), 32 deletions(-) diff --git a/src/librustc_metadata/creader.rs b/src/librustc_metadata/creader.rs index 789ecd0f6136b..b2f014b930d60 100644 --- a/src/librustc_metadata/creader.rs +++ b/src/librustc_metadata/creader.rs @@ -14,7 +14,7 @@ use cstore::{self, CStore, CrateSource, MetadataBlob}; use locator::{self, CratePaths}; use native_libs::relevant_lib; use schema::CrateRoot; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{Lrc, RwLock, Lock}; use rustc::hir::def_id::{CrateNum, CRATE_DEF_INDEX}; use rustc::hir::svh::Svh; @@ -30,7 +30,6 @@ use rustc::util::common::record_time; use rustc::util::nodemap::FxHashSet; use rustc::hir::map::Definitions; -use std::cell::{RefCell, Cell}; use std::ops::Deref; use std::path::PathBuf; use std::{cmp, fs}; @@ -63,7 +62,7 @@ fn dump_crates(cstore: &CStore) { info!(" name: {}", data.name()); info!(" cnum: {}", data.cnum); info!(" hash: {}", data.hash()); - info!(" reqd: {:?}", data.dep_kind.get()); + info!(" reqd: {:?}", *data.dep_kind.lock()); let CrateSource { dylib, rlib, rmeta } = data.source.clone(); dylib.map(|dl| info!(" dylib: {}", dl.0.display())); rlib.map(|rl| info!(" rlib: {}", rl.0.display())); @@ -233,7 +232,7 @@ impl<'a> CrateLoader<'a> { let mut cmeta = cstore::CrateMetadata { name, - extern_crate: Cell::new(None), + extern_crate: Lock::new(None), def_path_table: Lrc::new(def_path_table), trait_impls, proc_macros: crate_root.macro_derive_registrar.map(|_| { @@ -241,11 +240,11 @@ impl<'a> CrateLoader<'a> { }), root: crate_root, blob: metadata, - cnum_map: RefCell::new(cnum_map), + cnum_map: Lock::new(cnum_map), cnum, - codemap_import_info: RefCell::new(vec![]), - attribute_cache: RefCell::new([Vec::new(), Vec::new()]), - dep_kind: Cell::new(dep_kind), + codemap_import_info: RwLock::new(vec![]), + attribute_cache: Lock::new([Vec::new(), Vec::new()]), + dep_kind: Lock::new(dep_kind), source: cstore::CrateSource { dylib, rlib, @@ -335,7 +334,9 @@ impl<'a> CrateLoader<'a> { if data.root.macro_derive_registrar.is_some() { dep_kind = DepKind::UnexportedMacrosOnly; } - data.dep_kind.set(cmp::max(data.dep_kind.get(), dep_kind)); + data.dep_kind.with_lock(|data_dep_kind| { + *data_dep_kind = cmp::max(*data_dep_kind, dep_kind); + }); (cnum, data) } LoadResult::Loaded(library) => { @@ -379,14 +380,14 @@ impl<'a> CrateLoader<'a> { if !visited.insert((cnum, extern_crate.direct)) { return } let cmeta = self.cstore.get_crate_data(cnum); - let old_extern_crate = cmeta.extern_crate.get(); + let mut old_extern_crate = cmeta.extern_crate.borrow_mut(); // Prefer: // - something over nothing (tuple.0); // - direct extern crate to indirect (tuple.1); // - shorter paths to longer (tuple.2). let new_rank = (true, extern_crate.direct, !extern_crate.path_len); - let old_rank = match old_extern_crate { + let old_rank = match *old_extern_crate { None => (false, false, !0), Some(ref c) => (true, c.direct, !c.path_len), }; @@ -395,7 +396,9 @@ impl<'a> CrateLoader<'a> { return; // no change needed } - cmeta.extern_crate.set(Some(extern_crate)); + *old_extern_crate = Some(extern_crate); + drop(old_extern_crate); + // Propagate the extern crate info to dependencies. extern_crate.direct = false; for &dep_cnum in cmeta.cnum_map.borrow().iter() { @@ -646,7 +649,7 @@ impl<'a> CrateLoader<'a> { // #![panic_runtime] crate. self.inject_dependency_if(cnum, "a panic runtime", &|data| data.needs_panic_runtime(sess)); - runtime_found = runtime_found || data.dep_kind.get() == DepKind::Explicit; + runtime_found = runtime_found || *data.dep_kind.lock() == DepKind::Explicit; } }); diff --git a/src/librustc_metadata/cstore.rs b/src/librustc_metadata/cstore.rs index 22c80e8db3efa..19f43c180de38 100644 --- a/src/librustc_metadata/cstore.rs +++ b/src/librustc_metadata/cstore.rs @@ -22,8 +22,7 @@ use rustc_back::PanicStrategy; use rustc_data_structures::indexed_vec::IndexVec; use rustc::util::nodemap::{FxHashMap, FxHashSet, NodeMap}; -use std::cell::{RefCell, Cell}; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{Lrc, RwLock, Lock}; use syntax::{ast, attr}; use syntax::ext::base::SyntaxExtension; use syntax::symbol::Symbol; @@ -62,13 +61,13 @@ pub struct CrateMetadata { /// Information about the extern crate that caused this crate to /// be loaded. If this is `None`, then the crate was injected /// (e.g., by the allocator) - pub extern_crate: Cell>, + pub extern_crate: Lock>, pub blob: MetadataBlob, - pub cnum_map: RefCell, + pub cnum_map: Lock, pub cnum: CrateNum, - pub codemap_import_info: RefCell>, - pub attribute_cache: RefCell<[Vec>>; 2]>, + pub codemap_import_info: RwLock>, + pub attribute_cache: Lock<[Vec>>; 2]>, pub root: schema::CrateRoot, @@ -81,7 +80,7 @@ pub struct CrateMetadata { pub trait_impls: FxHashMap<(u32, DefIndex), schema::LazySeq>, - pub dep_kind: Cell, + pub dep_kind: Lock, pub source: CrateSource, pub proc_macros: Option)>>, diff --git a/src/librustc_metadata/cstore_impl.rs b/src/librustc_metadata/cstore_impl.rs index 0b50f5c44962b..34c93097a51f5 100644 --- a/src/librustc_metadata/cstore_impl.rs +++ b/src/librustc_metadata/cstore_impl.rs @@ -175,7 +175,10 @@ provide! { <'tcx> tcx, def_id, other, cdata, is_sanitizer_runtime => { cdata.is_sanitizer_runtime(tcx.sess) } is_profiler_runtime => { cdata.is_profiler_runtime(tcx.sess) } panic_strategy => { cdata.panic_strategy() } - extern_crate => { Lrc::new(cdata.extern_crate.get()) } + extern_crate => { + let r = Lrc::new(*cdata.extern_crate.lock()); + r + } is_no_builtins => { cdata.is_no_builtins(tcx.sess) } impl_defaultness => { cdata.get_impl_defaultness(def_id.index) } reachable_non_generics => { @@ -225,7 +228,10 @@ provide! { <'tcx> tcx, def_id, other, cdata, cdata.is_dllimport_foreign_item(def_id.index) } visibility => { cdata.get_visibility(def_id.index) } - dep_kind => { cdata.dep_kind.get() } + dep_kind => { + let r = *cdata.dep_kind.lock(); + r + } crate_name => { cdata.name } item_children => { let mut result = vec![]; @@ -241,10 +247,11 @@ provide! { <'tcx> tcx, def_id, other, cdata, } missing_extern_crate_item => { - match cdata.extern_crate.get() { + let r = match *cdata.extern_crate.borrow() { Some(extern_crate) if !extern_crate.direct => true, _ => false, - } + }; + r } used_crate_source => { Lrc::new(cdata.source.clone()) } @@ -419,13 +426,16 @@ impl CrateStore for cstore::CStore { fn dep_kind_untracked(&self, cnum: CrateNum) -> DepKind { - self.get_crate_data(cnum).dep_kind.get() + let data = self.get_crate_data(cnum); + let r = *data.dep_kind.lock(); + r } fn export_macros_untracked(&self, cnum: CrateNum) { let data = self.get_crate_data(cnum); - if data.dep_kind.get() == DepKind::UnexportedMacrosOnly { - data.dep_kind.set(DepKind::MacrosOnly) + let mut dep_kind = data.dep_kind.lock(); + if *dep_kind == DepKind::UnexportedMacrosOnly { + *dep_kind = DepKind::MacrosOnly; } } diff --git a/src/librustc_metadata/decoder.rs b/src/librustc_metadata/decoder.rs index f44703b9335e2..d83d6f26393d1 100644 --- a/src/librustc_metadata/decoder.rs +++ b/src/librustc_metadata/decoder.rs @@ -13,7 +13,7 @@ use cstore::{self, CrateMetadata, MetadataBlob, NativeLibrary}; use schema::*; -use rustc_data_structures::sync::Lrc; +use rustc_data_structures::sync::{Lrc, ReadGuard}; use rustc::hir::map::{DefKey, DefPath, DefPathData, DefPathHash}; use rustc::hir; use rustc::middle::cstore::{LinkagePreference, ExternConstBody, @@ -31,7 +31,6 @@ use rustc::ty::codec::TyDecoder; use rustc::mir::Mir; use rustc::util::nodemap::FxHashMap; -use std::cell::Ref; use std::collections::BTreeMap; use std::io; use std::mem; @@ -714,7 +713,7 @@ impl<'a, 'tcx> CrateMetadata { }; // Iterate over all children. - let macros_only = self.dep_kind.get().macros_only(); + let macros_only = self.dep_kind.lock().macros_only(); for child_index in item.children.decode((self, sess)) { if macros_only { continue @@ -950,6 +949,8 @@ impl<'a, 'tcx> CrateMetadata { if vec_.len() < node_index + 1 { vec_.resize(node_index + 1, None); } + // This can overwrite the result produced by another thread, but the value + // written should be the same vec_[node_index] = Some(result.clone()); result } @@ -1156,7 +1157,7 @@ impl<'a, 'tcx> CrateMetadata { /// for items inlined from other crates. pub fn imported_filemaps(&'a self, local_codemap: &codemap::CodeMap) - -> Ref<'a, Vec> { + -> ReadGuard<'a, Vec> { { let filemaps = self.codemap_import_info.borrow(); if !filemaps.is_empty() { @@ -1164,6 +1165,14 @@ impl<'a, 'tcx> CrateMetadata { } } + // Lock the codemap_import_info to ensure this only happens once + let mut codemap_import_info = self.codemap_import_info.borrow_mut(); + + if !codemap_import_info.is_empty() { + drop(codemap_import_info); + return self.codemap_import_info.borrow(); + } + let external_codemap = self.root.codemap.decode(self); let imported_filemaps = external_codemap.map(|filemap_to_import| { @@ -1222,8 +1231,10 @@ impl<'a, 'tcx> CrateMetadata { } }).collect(); + *codemap_import_info = imported_filemaps; + drop(codemap_import_info); + // This shouldn't borrow twice, but there is no way to downgrade RefMut to Ref. - *self.codemap_import_info.borrow_mut() = imported_filemaps; self.codemap_import_info.borrow() } }