clarify partially initialized Mutex issues

Author: RalfJung

src/libstd/io/lazy.rs

@@ -27,6 +27,9 @@ unsafe impl<T> Sync for Lazy<T> {}
 
 impl<T: Send + Sync + 'static> Lazy<T> {
     pub const fn new(init: fn() -> Arc<T>) -> Lazy<T> {
+        // `lock` is never initialized fully, so this mutex is reentrant!
+        // Do not use it in a way that might be reentrant, that could lead to
+        // aliasing `&mut`.
         Lazy {
             lock: Mutex::new(),
             ptr: Cell::new(ptr::null_mut()),
@@ -48,6 +51,7 @@ impl<T: Send + Sync + 'static> Lazy<T> {
         }
     }
 
+    // Must only be called with `lock` held
     unsafe fn init(&'static self) -> Arc<T> {
         // If we successfully register an at exit handler, then we cache the
         // `Arc` allocation in our own internal box (it will get deallocated by
@@ -60,6 +64,9 @@ impl<T: Send + Sync + 'static> Lazy<T> {
             };
             drop(Box::from_raw(ptr))
         });
+        // This could reentrantly call `init` again, which is a problem
+        // because our `lock` allows reentrancy!
+        // FIXME: Add argument why this is okay.
         let ret = (self.init)();
         if registered.is_ok() {
             self.ptr.set(Box::into_raw(Box::new(ret.clone())));

src/libstd/sys/unix/args.rs

@@ -80,6 +80,9 @@ mod imp {
 
     static mut ARGC: isize = 0;
     static mut ARGV: *const *const u8 = ptr::null();
+    // `ENV_LOCK` is never initialized fully, so this mutex is reentrant!
+    // Do not use it in a way that might be reentrant, that could lead to
+    // aliasing `&mut`.
     static LOCK: Mutex = Mutex::new();
 
     pub unsafe fn init(argc: isize, argv: *const *const u8) {

src/libstd/sys/unix/mutex.rs

@@ -25,8 +25,10 @@ unsafe impl Sync for Mutex {}
 #[allow(dead_code)] // sys isn't exported yet
 impl Mutex {
     pub const fn new() -> Mutex {
-        // Might be moved and address is changing it is better to avoid
-        // initialization of potentially opaque OS data before it landed
+        // Might be moved to a different address, so it is better to avoid
+        // initialization of potentially opaque OS data before it landed.
+        // Be very careful using this newly constructed `Mutex`, it should
+        // be initialized by calling `init()` first!
         Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) }
     }
     #[inline]

src/libstd/sys/unix/os.rs

@@ -33,6 +33,9 @@ use sys::fd;
 use vec;
 
 const TMPBUF_SZ: usize = 128;
+// `ENV_LOCK` is never initialized fully, so this mutex is reentrant!
+// Do not use it in a way that might be reentrant, that could lead to
+// aliasing `&mut`.
 static ENV_LOCK: Mutex = Mutex::new();
 
 

src/libstd/sys_common/at_exit_imp.rs

@@ -23,6 +23,9 @@ type Queue = Vec<Box<dyn FnBox()>>;
 // on poisoning and this module needs to operate at a lower level than requiring
 // the thread infrastructure to be in place (useful on the borders of
 // initialization/destruction).
+// `LOCK` is never initialized fully, so this mutex is reentrant!
+// Do not use it in a way that might be reentrant, that could lead to
+// aliasing `&mut`.
 static LOCK: Mutex = Mutex::new();
 static mut QUEUE: *mut Queue = ptr::null_mut();
 
@@ -72,6 +75,9 @@ pub fn push(f: Box<dyn FnBox()>) -> bool {
     unsafe {
         let _guard = LOCK.lock();
         if init() {
+            // This could reentrantly call `push` again, which is a problem because
+            // `LOCK` allows reentrancy!
+            // FIXME: Add argument why this is okay.
             (*QUEUE).push(f);
             true
         } else {

src/libstd/sys_common/mutex.rs

@@ -24,11 +24,15 @@ impl Mutex {
     ///
     /// Behavior is undefined if the mutex is moved after it is
     /// first used with any of the functions below.
+    /// Also, the mutex might not be fully functional without calling
+    /// `init`!  For example, on unix, the mutex is reentrant
+    /// until `init` reconfigures it appropriately.
     pub const fn new() -> Mutex { Mutex(imp::Mutex::new()) }
 
     /// Prepare the mutex for use.
     ///
     /// This should be called once the mutex is at a stable memory address.
+    /// It must not be called concurrently with any other operation.
     #[inline]
     pub unsafe fn init(&mut self) { self.0.init() }
 

src/libstd/sys_common/thread_local.rs

@@ -161,6 +161,9 @@ impl StaticKey {
         // Additionally a 0-index of a tls key hasn't been seen on windows, so
         // we just simplify the whole branch.
         if imp::requires_synchronized_create() {
+            // `INIT_LOCK` is never initialized fully, so this mutex is reentrant!
+            // Do not use it in a way that might be reentrant, that could lead to
+            // aliasing `&mut`.
             static INIT_LOCK: Mutex = Mutex::new();
             let _guard = INIT_LOCK.lock();
             let mut key = self.key.load(Ordering::SeqCst);

src/libstd/thread/mod.rs

@@ -940,6 +940,9 @@ pub struct ThreadId(u64);
 impl ThreadId {
     // Generate a new unique thread ID.
     fn new() -> ThreadId {
+        // `GUARD` is never initialized fully, so this mutex is reentrant!
+        // Do not use it in a way that might be reentrant, that could lead to
+        // aliasing `&mut`.
         static GUARD: mutex::Mutex = mutex::Mutex::new();
         static mut COUNTER: u64 = 0;