From f3370295b738c8b4c80fa8fc8449c0c3545b1b35 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 28 Dec 2013 19:32:16 -0800 Subject: [PATCH 1/2] Implement a Once primitive for initialization Of the 8 static mutexes that are currently in-use by the compiler and its libraries, 4 of them are currently used for one-time initialization. The unforunate side effect of using a static mutex is that the mutex is leaked. This primitive should provide the basis for efficiently keeping track of one-time initialization as well as ensuring that it does not leak the internal mutex that is used. I have chosen to put this in libstd because libstd is currently making use of a static initialization mutex (rt::local_ptr), but I can also see a more refined version of this type being suitable to initialize FFI bindings (such as initializing LLVM and initializing winsock networking on windows). I also intend on adding "helper threads" to libnative, and those will greatly benefit from a simple "once" primitive rather than always reinventing the wheel by using mutexes and bools. I would much rather see this primitive built on a mutex that blocks green threads appropriately, but that does not exist at this time, so it does not belong outside of `std::unstable`. --- src/libstd/unstable/mutex.rs | 146 ++++++++++++++++++++++++++++++++++- 1 file changed, 145 insertions(+), 1 deletion(-) diff --git a/src/libstd/unstable/mutex.rs b/src/libstd/unstable/mutex.rs index 5b2fac8e74e2e..0361296ff38cf 100644 --- a/src/libstd/unstable/mutex.rs +++ b/src/libstd/unstable/mutex.rs @@ -315,10 +315,154 @@ mod imp { } } +/// A type which can be used to run a one-time global initialization. This type +/// is *unsafe* to use because it is built on top of the `Mutex` in this module. +/// It does not know whether the currently running task is in a green or native +/// context, and a blocking mutex should *not* be used under normal +/// circumstances on a green task. +/// +/// Despite its unsafety, it is often useful to have a one-time initialization +/// routine run for FFI bindings or related external functionality. This type +/// can only be statically constructed with the `ONCE_INIT` value. +/// +/// # Example +/// +/// ```rust +/// use std::unstable::mutex::{Once, ONCE_INIT}; +/// +/// static mut START: Once = ONCE_INIT; +/// unsafe { +/// START.doit(|| { +/// // run initialization here +/// }); +/// } +/// ``` +pub struct Once { + priv mutex: Mutex, + priv cnt: AtomicInt, + priv lock_cnt: AtomicInt, +} + +/// Initialization value for static `Once` values. +pub static ONCE_INIT: Once = Once { + mutex: MUTEX_INIT, + cnt: INIT_ATOMIC_INT, + lock_cnt: INIT_ATOMIC_INT, +}; + +impl Once { + /// Perform an initialization routine once and only once. The given closure + /// will be executed if this is the first time `doit` has been called, and + /// otherwise the routine will *not* be invoked. + /// + /// This method will block the calling *os thread* if another initialization + /// routine is currently running. + /// + /// When this function returns, it is guaranteed that some initialization + /// has run and completed (it may not be the closure specified). + pub fn doit(&mut self, f: ||) { + // Implementation-wise, this would seem like a fairly trivial primitive. + // The stickler part is where our mutexes currently require an + // allocation, and usage of a `Once` should't leak this allocation. + // + // This means that there must be a deterministic destroyer of the mutex + // contained within (because it's not needed after the initialization + // has run). + // + // The general scheme here is to gate all future threads once + // initialization has completed with a "very negative" count, and to + // allow through threads to lock the mutex if they see a non negative + // count. For all threads grabbing the mutex, exactly one of them should + // be responsible for unlocking the mutex, and this should only be done + // once everyone else is done with the mutex. + // + // This atomicity is achieved by swapping a very negative value into the + // shared count when the initialization routine has completed. This will + // read the number of threads which will at some point attempt to + // acquire the mutex. This count is then squirreled away in a separate + // variable, and the last person on the way out of the mutex is then + // responsible for destroying the mutex. + // + // It is crucial that the negative value is swapped in *after* the + // initialization routine has completed because otherwise new threads + // calling `doit` will return immediately before the initialization has + // completed. + + let prev = self.cnt.fetch_add(1, SeqCst); + if prev < 0 { + // Make sure we never overflow, we'll never have int::min_value + // simultaneous calls to `doit` to make this value go back to 0 + self.cnt.store(int::min_value, SeqCst); + return + } + + // If the count is negative, then someone else finished the job, + // otherwise we run the job and record how many people will try to grab + // this lock + unsafe { self.mutex.lock() } + if self.cnt.load(SeqCst) > 0 { + f(); + let prev = self.cnt.swap(int::min_value, SeqCst); + self.lock_cnt.store(prev, SeqCst); + } + unsafe { self.mutex.unlock() } + + // Last one out cleans up after everyone else, no leaks! + if self.lock_cnt.fetch_add(-1, SeqCst) == 1 { + unsafe { self.mutex.destroy() } + } + } +} + #[cfg(test)] mod test { - use super::{Mutex, MUTEX_INIT}; use rt::thread::Thread; + use super::{ONCE_INIT, Once, Mutex, MUTEX_INIT}; + use task; + + #[test] + fn smoke_once() { + static mut o: Once = ONCE_INIT; + let mut a = 0; + unsafe { o.doit(|| a += 1); } + assert_eq!(a, 1); + unsafe { o.doit(|| a += 1); } + assert_eq!(a, 1); + } + + #[test] + fn stampede_once() { + static mut o: Once = ONCE_INIT; + static mut run: bool = false; + + let (p, c) = SharedChan::new(); + for _ in range(0, 10) { + let c = c.clone(); + do spawn { + for _ in range(0, 4) { task::deschedule() } + unsafe { + o.doit(|| { + assert!(!run); + run = true; + }); + assert!(run); + } + c.send(()); + } + } + + unsafe { + o.doit(|| { + assert!(!run); + run = true; + }); + assert!(run); + } + + for _ in range(0, 10) { + p.recv(); + } + } #[test] fn somke_lock() { From c22fed9424a907beab53f6c6cd54afeff039f1b3 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Sat, 28 Dec 2013 19:44:52 -0800 Subject: [PATCH 2/2] Convert relevant static mutexes to Once --- src/libnative/io/net.rs | 13 ++++--------- src/librustc/back/link.rs | 12 ++++-------- src/librustc/lib/llvm.rs | 5 +++-- src/librustc/middle/trans/base.rs | 17 +++++++++++++++-- src/libstd/rt/local_ptr.rs | 14 ++------------ src/libstd/unstable/mutex.rs | 23 +++++++++++++---------- src/rustllvm/RustWrapper.cpp | 22 ---------------------- 7 files changed, 41 insertions(+), 65 deletions(-) diff --git a/src/libnative/io/net.rs b/src/libnative/io/net.rs index aaa95ce0cfbe2..9f307fcebb042 100644 --- a/src/libnative/io/net.rs +++ b/src/libnative/io/net.rs @@ -164,19 +164,14 @@ pub fn init() { } unsafe { - use std::unstable::mutex::{Mutex, MUTEX_INIT}; - static mut LOCK: Mutex = MUTEX_INIT; - static mut INITIALIZED: bool = false; - if INITIALIZED { return } - LOCK.lock(); - if !INITIALIZED { + use std::unstable::mutex::{Once, ONCE_INIT}; + static mut INIT: Once = ONCE_INIT; + INIT.doit(|| { let mut data: WSADATA = intrinsics::init(); let ret = WSAStartup(0x202, // version 2.2 &mut data); assert_eq!(ret, 0); - INITIALIZED = true; - } - LOCK.unlock(); + }); } } diff --git a/src/librustc/back/link.rs b/src/librustc/back/link.rs index e761a14a3acec..75edf6c80e66c 100644 --- a/src/librustc/back/link.rs +++ b/src/librustc/back/link.rs @@ -310,9 +310,8 @@ pub mod write { } unsafe fn configure_llvm(sess: Session) { - use std::unstable::mutex::{MUTEX_INIT, Mutex}; - static mut LOCK: Mutex = MUTEX_INIT; - static mut CONFIGURED: bool = false; + use std::unstable::mutex::{Once, ONCE_INIT}; + static mut INIT: Once = ONCE_INIT; // Copy what clan does by turning on loop vectorization at O2 and // slp vectorization at O3 @@ -341,8 +340,7 @@ pub mod write { add(*arg); } - LOCK.lock(); - if !CONFIGURED { + INIT.doit(|| { llvm::LLVMInitializePasses(); // Only initialize the platforms supported by Rust here, because @@ -369,9 +367,7 @@ pub mod write { llvm::LLVMRustSetLLVMOptions(llvm_args.len() as c_int, llvm_args.as_ptr()); - CONFIGURED = true; - } - LOCK.unlock(); + }); } unsafe fn populate_llvm_passes(fpm: lib::llvm::PassManagerRef, diff --git a/src/librustc/lib/llvm.rs b/src/librustc/lib/llvm.rs index 50a35e9d1bf81..46bc79e3e3d88 100644 --- a/src/librustc/lib/llvm.rs +++ b/src/librustc/lib/llvm.rs @@ -1455,6 +1455,9 @@ pub mod llvm { BufferName: *c_char) -> MemoryBufferRef; + pub fn LLVMIsMultithreaded() -> Bool; + pub fn LLVMStartMultithreaded() -> Bool; + /** Returns a string describing the last error caused by an LLVMRust* call. */ pub fn LLVMRustGetLastError() -> *c_char; @@ -1462,8 +1465,6 @@ pub mod llvm { /// Print the pass timings since static dtors aren't picking them up. pub fn LLVMRustPrintPassTimings(); - pub fn LLVMRustStartMultithreading() -> bool; - pub fn LLVMStructCreateNamed(C: ContextRef, Name: *c_char) -> TypeRef; pub fn LLVMStructSetBody(StructTy: TypeRef, diff --git a/src/librustc/middle/trans/base.rs b/src/librustc/middle/trans/base.rs index 0b2ee710e9913..f308b8a02915e 100644 --- a/src/librustc/middle/trans/base.rs +++ b/src/librustc/middle/trans/base.rs @@ -3189,8 +3189,21 @@ pub fn trans_crate(sess: session::Session, analysis: &CrateAnalysis, output: &Path) -> CrateTranslation { // Before we touch LLVM, make sure that multithreading is enabled. - if unsafe { !llvm::LLVMRustStartMultithreading() } { - sess.bug("couldn't enable multi-threaded LLVM"); + unsafe { + use std::unstable::mutex::{Once, ONCE_INIT}; + static mut INIT: Once = ONCE_INIT; + static mut POISONED: bool = false; + INIT.doit(|| { + if llvm::LLVMStartMultithreaded() != 1 { + // use an extra bool to make sure that all future usage of LLVM + // cannot proceed despite the Once not running more than once. + POISONED = true; + } + }); + + if POISONED { + sess.bug("couldn't enable multi-threaded LLVM"); + } } let mut symbol_hasher = Sha256::new(); diff --git a/src/libstd/rt/local_ptr.rs b/src/libstd/rt/local_ptr.rs index 42cce272e4430..f13691a7bfe99 100644 --- a/src/libstd/rt/local_ptr.rs +++ b/src/libstd/rt/local_ptr.rs @@ -160,30 +160,20 @@ pub mod native { use option::{Option, Some, None}; use ptr; use tls = rt::thread_local_storage; - use unstable::mutex::{Mutex, MUTEX_INIT}; - static mut LOCK: Mutex = MUTEX_INIT; - static mut INITIALIZED: bool = false; static mut RT_TLS_KEY: tls::Key = -1; /// Initialize the TLS key. Other ops will fail if this isn't executed /// first. pub fn init() { unsafe { - LOCK.lock(); - if !INITIALIZED { - tls::create(&mut RT_TLS_KEY); - INITIALIZED = true; - } - LOCK.unlock(); + tls::create(&mut RT_TLS_KEY); } } pub unsafe fn cleanup() { - rtassert!(INITIALIZED); + rtassert!(RT_TLS_KEY != -1); tls::destroy(RT_TLS_KEY); - LOCK.destroy(); - INITIALIZED = false; } /// Give a pointer to thread-local storage. diff --git a/src/libstd/unstable/mutex.rs b/src/libstd/unstable/mutex.rs index 0361296ff38cf..6dbac90ef664a 100644 --- a/src/libstd/unstable/mutex.rs +++ b/src/libstd/unstable/mutex.rs @@ -47,6 +47,7 @@ #[allow(non_camel_case_types)]; +use int; use libc::c_void; use sync::atomics; @@ -339,15 +340,15 @@ mod imp { /// ``` pub struct Once { priv mutex: Mutex, - priv cnt: AtomicInt, - priv lock_cnt: AtomicInt, + priv cnt: atomics::AtomicInt, + priv lock_cnt: atomics::AtomicInt, } /// Initialization value for static `Once` values. pub static ONCE_INIT: Once = Once { mutex: MUTEX_INIT, - cnt: INIT_ATOMIC_INT, - lock_cnt: INIT_ATOMIC_INT, + cnt: atomics::INIT_ATOMIC_INT, + lock_cnt: atomics::INIT_ATOMIC_INT, }; impl Once { @@ -388,11 +389,11 @@ impl Once { // calling `doit` will return immediately before the initialization has // completed. - let prev = self.cnt.fetch_add(1, SeqCst); + let prev = self.cnt.fetch_add(1, atomics::SeqCst); if prev < 0 { // Make sure we never overflow, we'll never have int::min_value // simultaneous calls to `doit` to make this value go back to 0 - self.cnt.store(int::min_value, SeqCst); + self.cnt.store(int::min_value, atomics::SeqCst); return } @@ -400,15 +401,15 @@ impl Once { // otherwise we run the job and record how many people will try to grab // this lock unsafe { self.mutex.lock() } - if self.cnt.load(SeqCst) > 0 { + if self.cnt.load(atomics::SeqCst) > 0 { f(); - let prev = self.cnt.swap(int::min_value, SeqCst); - self.lock_cnt.store(prev, SeqCst); + let prev = self.cnt.swap(int::min_value, atomics::SeqCst); + self.lock_cnt.store(prev, atomics::SeqCst); } unsafe { self.mutex.unlock() } // Last one out cleans up after everyone else, no leaks! - if self.lock_cnt.fetch_add(-1, SeqCst) == 1 { + if self.lock_cnt.fetch_add(-1, atomics::SeqCst) == 1 { unsafe { self.mutex.destroy() } } } @@ -416,6 +417,8 @@ impl Once { #[cfg(test)] mod test { + use prelude::*; + use rt::thread::Thread; use super::{ONCE_INIT, Once, Mutex, MUTEX_INIT}; use task; diff --git a/src/rustllvm/RustWrapper.cpp b/src/rustllvm/RustWrapper.cpp index d66f90a5352c1..335c7b2c65b7f 100644 --- a/src/rustllvm/RustWrapper.cpp +++ b/src/rustllvm/RustWrapper.cpp @@ -149,28 +149,6 @@ extern "C" LLVMValueRef LLVMInlineAsm(LLVMTypeRef Ty, IsAlignStack, (InlineAsm::AsmDialect) Dialect)); } -/** - * This function is intended to be a threadsafe interface into enabling a - * multithreaded LLVM. This is invoked at the start of the translation phase of - * compilation to ensure that LLVM is ready. - * - * All of trans properly isolates LLVM with the use of a different - * LLVMContextRef per task, thus allowing parallel compilation of different - * crates in the same process. At the time of this writing, the use case for - * this is unit tests for rusti, but there are possible other applications. - */ -extern "C" bool LLVMRustStartMultithreading() { - static Mutex lock; - bool ret = true; - assert(lock.acquire()); - if (!LLVMIsMultithreaded()) { - ret = LLVMStartMultithreaded(); - } - assert(lock.release()); - return ret; -} - - typedef DIBuilder* DIBuilderRef; template