From d3d31105e99f5265880d0f010436ed5c6baab034 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Aug 2018 12:45:27 +0200 Subject: [PATCH 1/7] clarify partially initialized Mutex issues --- src/libstd/io/lazy.rs | 7 +++++++ src/libstd/sys/unix/args.rs | 3 +++ src/libstd/sys/unix/mutex.rs | 6 ++++-- src/libstd/sys/unix/os.rs | 3 +++ src/libstd/sys_common/at_exit_imp.rs | 6 ++++++ src/libstd/sys_common/mutex.rs | 4 ++++ src/libstd/sys_common/thread_local.rs | 3 +++ src/libstd/thread/mod.rs | 3 +++ 8 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/libstd/io/lazy.rs b/src/libstd/io/lazy.rs index d357966be9227..09b2ddcbcac66 100644 --- a/src/libstd/io/lazy.rs +++ b/src/libstd/io/lazy.rs @@ -27,6 +27,9 @@ unsafe impl Sync for Lazy {} impl Lazy { pub const fn new(init: fn() -> Arc) -> Lazy { + // `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 Lazy { } } + // Must only be called with `lock` held unsafe fn init(&'static self) -> Arc { // 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 Lazy { }; 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()))); diff --git a/src/libstd/sys/unix/args.rs b/src/libstd/sys/unix/args.rs index 7e32ec1347e9e..c91bd5b22afcb 100644 --- a/src/libstd/sys/unix/args.rs +++ b/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) { diff --git a/src/libstd/sys/unix/mutex.rs b/src/libstd/sys/unix/mutex.rs index 52cf3f97c5c83..f0711b603204e 100644 --- a/src/libstd/sys/unix/mutex.rs +++ b/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] diff --git a/src/libstd/sys/unix/os.rs b/src/libstd/sys/unix/os.rs index 1d92e8fc97c7a..6ef9502ba624e 100644 --- a/src/libstd/sys/unix/os.rs +++ b/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(); diff --git a/src/libstd/sys_common/at_exit_imp.rs b/src/libstd/sys_common/at_exit_imp.rs index b28a4d2f8be01..633605039afdd 100644 --- a/src/libstd/sys_common/at_exit_imp.rs +++ b/src/libstd/sys_common/at_exit_imp.rs @@ -23,6 +23,9 @@ type Queue = Vec>; // 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) -> 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 { diff --git a/src/libstd/sys_common/mutex.rs b/src/libstd/sys_common/mutex.rs index 608355b7d70f8..77a33e4c6be86 100644 --- a/src/libstd/sys_common/mutex.rs +++ b/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() } diff --git a/src/libstd/sys_common/thread_local.rs b/src/libstd/sys_common/thread_local.rs index 75f6b9ac7fd8c..2cc5372e26818 100644 --- a/src/libstd/sys_common/thread_local.rs +++ b/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); diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index bbe80df7e8bdb..98b4ca36c2633 100644 --- a/src/libstd/thread/mod.rs +++ b/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; From 819645bfc461af9c02aa60a3385b008f490ed164 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Aug 2018 13:39:47 +0200 Subject: [PATCH 2/7] I think we have to strengthen Mutex::init UB --- src/libstd/sys_common/mutex.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstd/sys_common/mutex.rs b/src/libstd/sys_common/mutex.rs index 77a33e4c6be86..a4efe4d128e69 100644 --- a/src/libstd/sys_common/mutex.rs +++ b/src/libstd/sys_common/mutex.rs @@ -32,7 +32,7 @@ impl Mutex { /// 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. + /// Behavior is undefined unless this is called before any other operation. #[inline] pub unsafe fn init(&mut self) { self.0.init() } From 22457deef7e4205b46127fbbe4d0c720aa60b999 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Aug 2018 13:52:15 +0200 Subject: [PATCH 3/7] make Lazy::new unsafe and check reentrancy condition in the callers --- src/libstd/io/lazy.rs | 6 ++++-- src/libstd/io/stdio.rs | 10 +++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/src/libstd/io/lazy.rs b/src/libstd/io/lazy.rs index 09b2ddcbcac66..9513cc7fb2d7b 100644 --- a/src/libstd/io/lazy.rs +++ b/src/libstd/io/lazy.rs @@ -26,7 +26,9 @@ const fn done() -> *mut Arc { 1_usize as *mut _ } unsafe impl Sync for Lazy {} impl Lazy { - pub const fn new(init: fn() -> Arc) -> Lazy { + /// Safety: `init` must not call `get` on the variable that is being + /// initialized. + pub const unsafe fn new(init: fn() -> Arc) -> Lazy { // `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`. @@ -66,7 +68,7 @@ impl Lazy { }); // This could reentrantly call `init` again, which is a problem // because our `lock` allows reentrancy! - // FIXME: Add argument why this is okay. + // That's why `new` is unsafe and requires the caller to ensure no reentrancy happens. let ret = (self.init)(); if registered.is_ok() { self.ptr.set(Box::into_raw(Box::new(ret.clone()))); diff --git a/src/libstd/io/stdio.rs b/src/libstd/io/stdio.rs index fffe8fc559b81..1f256f518c7ce 100644 --- a/src/libstd/io/stdio.rs +++ b/src/libstd/io/stdio.rs @@ -197,12 +197,13 @@ pub struct StdinLock<'a> { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn stdin() -> Stdin { - static INSTANCE: Lazy>>> = Lazy::new(stdin_init); + static INSTANCE: Lazy>>> = unsafe { Lazy::new(stdin_init) }; return Stdin { inner: INSTANCE.get().expect("cannot access stdin during shutdown"), }; fn stdin_init() -> Arc>>> { + // This must not reentrantly access `INSTANCE` let stdin = match stdin_raw() { Ok(stdin) => Maybe::Real(stdin), _ => Maybe::Fake @@ -396,12 +397,13 @@ pub struct StdoutLock<'a> { #[stable(feature = "rust1", since = "1.0.0")] pub fn stdout() -> Stdout { static INSTANCE: Lazy>>>> - = Lazy::new(stdout_init); + = unsafe { Lazy::new(stdout_init) }; return Stdout { inner: INSTANCE.get().expect("cannot access stdout during shutdown"), }; fn stdout_init() -> Arc>>>> { + // This must not reentrantly access `INSTANCE` let stdout = match stdout_raw() { Ok(stdout) => Maybe::Real(stdout), _ => Maybe::Fake, @@ -531,12 +533,14 @@ pub struct StderrLock<'a> { /// ``` #[stable(feature = "rust1", since = "1.0.0")] pub fn stderr() -> Stderr { - static INSTANCE: Lazy>>> = Lazy::new(stderr_init); + static INSTANCE: Lazy>>> = + unsafe { Lazy::new(stderr_init) }; return Stderr { inner: INSTANCE.get().expect("cannot access stderr during shutdown"), }; fn stderr_init() -> Arc>>> { + // This must not reentrantly access `INSTANCE` let stderr = match stderr_raw() { Ok(stderr) => Maybe::Real(stderr), _ => Maybe::Fake, From ab3e4a27894295ec0fca28b492450f2b22fbad4e Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Aug 2018 13:53:38 +0200 Subject: [PATCH 4/7] argue why at_exit_imp is fine --- src/libstd/sys_common/at_exit_imp.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstd/sys_common/at_exit_imp.rs b/src/libstd/sys_common/at_exit_imp.rs index 633605039afdd..30019088eb689 100644 --- a/src/libstd/sys_common/at_exit_imp.rs +++ b/src/libstd/sys_common/at_exit_imp.rs @@ -64,6 +64,7 @@ pub fn cleanup() { if !queue.is_null() { let queue: Box = Box::from_raw(queue); for to_run in *queue { + // We are not holding any lock, so reentrancy is fine. to_run(); } } @@ -75,9 +76,8 @@ pub fn push(f: Box) -> 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. + // We are just moving `f` around, not calling it. + // There is no possibility of reentrancy here. (*QUEUE).push(f); true } else { From 645388583ca47357a6a2e5878a9cde84e2e579d3 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Mon, 6 Aug 2018 14:39:55 +0200 Subject: [PATCH 5/7] actually, reentrant uninitialized mutex acquisition is outright UB --- src/libstd/io/lazy.rs | 5 ++--- src/libstd/sys/unix/args.rs | 5 ++--- src/libstd/sys/unix/os.rs | 5 ++--- src/libstd/sys_common/at_exit_imp.rs | 5 ++--- src/libstd/sys_common/mutex.rs | 6 +++--- src/libstd/sys_common/thread_local.rs | 5 ++--- src/libstd/thread/mod.rs | 5 ++--- 7 files changed, 15 insertions(+), 21 deletions(-) diff --git a/src/libstd/io/lazy.rs b/src/libstd/io/lazy.rs index 9513cc7fb2d7b..5743ea51af350 100644 --- a/src/libstd/io/lazy.rs +++ b/src/libstd/io/lazy.rs @@ -29,9 +29,8 @@ impl Lazy { /// Safety: `init` must not call `get` on the variable that is being /// initialized. pub const unsafe fn new(init: fn() -> Arc) -> Lazy { - // `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`. + // `lock` is never initialized fully, so it is UB to attempt to + // acquire this mutex reentrantly! Lazy { lock: Mutex::new(), ptr: Cell::new(ptr::null_mut()), diff --git a/src/libstd/sys/unix/args.rs b/src/libstd/sys/unix/args.rs index c91bd5b22afcb..220bd11b1f1e5 100644 --- a/src/libstd/sys/unix/args.rs +++ b/src/libstd/sys/unix/args.rs @@ -80,9 +80,8 @@ 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`. + // `ENV_LOCK` is never initialized fully, so it is UB to attempt to + // acquire this mutex reentrantly! static LOCK: Mutex = Mutex::new(); pub unsafe fn init(argc: isize, argv: *const *const u8) { diff --git a/src/libstd/sys/unix/os.rs b/src/libstd/sys/unix/os.rs index 6ef9502ba624e..3d98b2efdf1f7 100644 --- a/src/libstd/sys/unix/os.rs +++ b/src/libstd/sys/unix/os.rs @@ -33,9 +33,8 @@ 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`. +// `ENV_LOCK` is never initialized fully, so it is UB to attempt to +// acquire this mutex reentrantly! static ENV_LOCK: Mutex = Mutex::new(); diff --git a/src/libstd/sys_common/at_exit_imp.rs b/src/libstd/sys_common/at_exit_imp.rs index 30019088eb689..856798373128d 100644 --- a/src/libstd/sys_common/at_exit_imp.rs +++ b/src/libstd/sys_common/at_exit_imp.rs @@ -23,9 +23,8 @@ type Queue = Vec>; // 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`. +// `LOCK` is never initialized fully, so it is UB to attempt to +// acquire this mutex reentrantly! static LOCK: Mutex = Mutex::new(); static mut QUEUE: *mut Queue = ptr::null_mut(); diff --git a/src/libstd/sys_common/mutex.rs b/src/libstd/sys_common/mutex.rs index a4efe4d128e69..74e1defd9f465 100644 --- a/src/libstd/sys_common/mutex.rs +++ b/src/libstd/sys_common/mutex.rs @@ -24,9 +24,9 @@ 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. + /// Also, until `init` is called, behavior is undefined if this + /// mutex is ever used reentrantly, i.e., `raw_lock` or `try_lock` + /// are called by the thread currently holding the lock. pub const fn new() -> Mutex { Mutex(imp::Mutex::new()) } /// Prepare the mutex for use. diff --git a/src/libstd/sys_common/thread_local.rs b/src/libstd/sys_common/thread_local.rs index 2cc5372e26818..9db7d73269876 100644 --- a/src/libstd/sys_common/thread_local.rs +++ b/src/libstd/sys_common/thread_local.rs @@ -161,9 +161,8 @@ 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`. + // `INIT_LOCK` is never initialized fully, so it is UB to attempt to + // acquire this mutex reentrantly! static INIT_LOCK: Mutex = Mutex::new(); let _guard = INIT_LOCK.lock(); let mut key = self.key.load(Ordering::SeqCst); diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 98b4ca36c2633..0078a05e5971a 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -940,9 +940,8 @@ 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`. + // `GUARD` is never initialized fully, so it is UB to attempt to + // acquire this mutex reentrantly! static GUARD: mutex::Mutex = mutex::Mutex::new(); static mut COUNTER: u64 = 0; From 31bec788f46c73ab14c72868dc6141141320a058 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 8 Aug 2018 18:12:33 +0200 Subject: [PATCH 6/7] avoid using the word 'initialized' to talk about that non-reentrant-capable state of the mutex --- src/libstd/io/lazy.rs | 3 +-- src/libstd/sys/unix/args.rs | 2 +- src/libstd/sys/unix/os.rs | 2 +- src/libstd/sys_common/at_exit_imp.rs | 2 +- src/libstd/sys_common/mutex.rs | 4 +++- src/libstd/sys_common/thread_local.rs | 2 +- src/libstd/thread/mod.rs | 2 +- 7 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/libstd/io/lazy.rs b/src/libstd/io/lazy.rs index 5743ea51af350..4fb367fb6ba52 100644 --- a/src/libstd/io/lazy.rs +++ b/src/libstd/io/lazy.rs @@ -15,6 +15,7 @@ use sys_common; use sys_common::mutex::Mutex; pub struct Lazy { + // We never call `lock.init()`, so it is UB to attempt to acquire this mutex reentrantly! lock: Mutex, ptr: Cell<*mut Arc>, init: fn() -> Arc, @@ -29,8 +30,6 @@ impl Lazy { /// Safety: `init` must not call `get` on the variable that is being /// initialized. pub const unsafe fn new(init: fn() -> Arc) -> Lazy { - // `lock` is never initialized fully, so it is UB to attempt to - // acquire this mutex reentrantly! Lazy { lock: Mutex::new(), ptr: Cell::new(ptr::null_mut()), diff --git a/src/libstd/sys/unix/args.rs b/src/libstd/sys/unix/args.rs index 220bd11b1f1e5..c3c033dfbc739 100644 --- a/src/libstd/sys/unix/args.rs +++ b/src/libstd/sys/unix/args.rs @@ -80,7 +80,7 @@ mod imp { static mut ARGC: isize = 0; static mut ARGV: *const *const u8 = ptr::null(); - // `ENV_LOCK` is never initialized fully, so it is UB to attempt to + // We never call `ENV_LOCK.init()`, so it is UB to attempt to // acquire this mutex reentrantly! static LOCK: Mutex = Mutex::new(); diff --git a/src/libstd/sys/unix/os.rs b/src/libstd/sys/unix/os.rs index 3d98b2efdf1f7..08c3e15497843 100644 --- a/src/libstd/sys/unix/os.rs +++ b/src/libstd/sys/unix/os.rs @@ -33,7 +33,7 @@ use sys::fd; use vec; const TMPBUF_SZ: usize = 128; -// `ENV_LOCK` is never initialized fully, so it is UB to attempt to +// We never call `ENV_LOCK.init()`, so it is UB to attempt to // acquire this mutex reentrantly! static ENV_LOCK: Mutex = Mutex::new(); diff --git a/src/libstd/sys_common/at_exit_imp.rs b/src/libstd/sys_common/at_exit_imp.rs index 856798373128d..76e5df2c8654a 100644 --- a/src/libstd/sys_common/at_exit_imp.rs +++ b/src/libstd/sys_common/at_exit_imp.rs @@ -23,7 +23,7 @@ type Queue = Vec>; // 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 it is UB to attempt to +// We never call `LOCK.init()`, so it is UB to attempt to // acquire this mutex reentrantly! static LOCK: Mutex = Mutex::new(); static mut QUEUE: *mut Queue = ptr::null_mut(); diff --git a/src/libstd/sys_common/mutex.rs b/src/libstd/sys_common/mutex.rs index 74e1defd9f465..c6d531c7a1ac5 100644 --- a/src/libstd/sys_common/mutex.rs +++ b/src/libstd/sys_common/mutex.rs @@ -32,7 +32,9 @@ impl Mutex { /// Prepare the mutex for use. /// /// This should be called once the mutex is at a stable memory address. - /// Behavior is undefined unless this is called before any other operation. + /// If called, this must be the very first thing that happens to the mutex. + /// Calling it in parallel with or after any operation (including another + /// `init()`) is undefined behavior. #[inline] pub unsafe fn init(&mut self) { self.0.init() } diff --git a/src/libstd/sys_common/thread_local.rs b/src/libstd/sys_common/thread_local.rs index 9db7d73269876..bb72cb0930a7f 100644 --- a/src/libstd/sys_common/thread_local.rs +++ b/src/libstd/sys_common/thread_local.rs @@ -161,7 +161,7 @@ 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 it is UB to attempt to + // We never call `INIT_LOCK.init()`, so it is UB to attempt to // acquire this mutex reentrantly! static INIT_LOCK: Mutex = Mutex::new(); let _guard = INIT_LOCK.lock(); diff --git a/src/libstd/thread/mod.rs b/src/libstd/thread/mod.rs index 0078a05e5971a..61c6084a25023 100644 --- a/src/libstd/thread/mod.rs +++ b/src/libstd/thread/mod.rs @@ -940,7 +940,7 @@ pub struct ThreadId(u64); impl ThreadId { // Generate a new unique thread ID. fn new() -> ThreadId { - // `GUARD` is never initialized fully, so it is UB to attempt to + // We never call `GUARD.init()`, so it is UB to attempt to // acquire this mutex reentrantly! static GUARD: mutex::Mutex = mutex::Mutex::new(); static mut COUNTER: u64 = 0; From 25db84206b681731960d88558bc53640fe117b09 Mon Sep 17 00:00:00 2001 From: Ralf Jung Date: Wed, 8 Aug 2018 18:14:06 +0200 Subject: [PATCH 7/7] missed one --- src/libstd/sys/unix/mutex.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libstd/sys/unix/mutex.rs b/src/libstd/sys/unix/mutex.rs index f0711b603204e..efa6e8ec036ae 100644 --- a/src/libstd/sys/unix/mutex.rs +++ b/src/libstd/sys/unix/mutex.rs @@ -27,8 +27,8 @@ impl Mutex { pub const fn new() -> Mutex { // 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! + // Be very careful using this newly constructed `Mutex`, reentrant + // locking is undefined behavior until `init` is called! Mutex { inner: UnsafeCell::new(libc::PTHREAD_MUTEX_INITIALIZER) } } #[inline]