diff options
| author | Ralf Jung <post@ralfj.de> | 2025-05-19 15:18:24 +0200 |
|---|---|---|
| committer | Ralf Jung <post@ralfj.de> | 2025-05-19 15:21:25 +0200 |
| commit | 8286487c0ceef102e162d39d0c75adc1e3068b6c (patch) | |
| tree | e8d196c20228788177ff007f1eeaeb09c2ef67ac | |
| parent | e42bbfe1f7c26f8760a99c4b1f27d33aba1040bb (diff) | |
| download | rust-8286487c0ceef102e162d39d0c75adc1e3068b6c.tar.gz rust-8286487c0ceef102e162d39d0c75adc1e3068b6c.zip | |
fix data race in ReentrantLock fallback for targets without 64bit atomics
| -rw-r--r-- | library/std/src/sync/reentrant_lock.rs | 8 | ||||
| -rw-r--r-- | src/tools/miri/tests/many-seeds/reentrant-lock.rs | 19 |
2 files changed, 25 insertions, 2 deletions
diff --git a/library/std/src/sync/reentrant_lock.rs b/library/std/src/sync/reentrant_lock.rs index 24539d4e830..96a4cf12659 100644 --- a/library/std/src/sync/reentrant_lock.rs +++ b/library/std/src/sync/reentrant_lock.rs @@ -136,7 +136,7 @@ cfg_if!( // we only ever read from the tid if `tls_addr` matches the current // TLS address. In that case, either the tid has been set by // the current thread, or by a thread that has terminated before - // the current thread was created. In either case, no further + // the current thread's `tls_addr` was allocated. In either case, no further // synchronization is needed (as per <https://github.com/rust-lang/miri/issues/3450>) tls_addr: Atomic<usize>, tid: UnsafeCell<u64>, @@ -154,8 +154,12 @@ cfg_if!( // NOTE: This assumes that `owner` is the ID of the current // thread, and may spuriously return `false` if that's not the case. fn contains(&self, owner: ThreadId) -> bool { + // We must call `tls_addr()` *before* doing the load to ensure that if we reuse an + // earlier thread's address, the `tls_addr.load()` below happens-after everything + // that thread did. + let tls_addr = tls_addr(); // SAFETY: See the comments in the struct definition. - self.tls_addr.load(Ordering::Relaxed) == tls_addr() + self.tls_addr.load(Ordering::Relaxed) == tls_addr && unsafe { *self.tid.get() } == owner.as_u64().get() } diff --git a/src/tools/miri/tests/many-seeds/reentrant-lock.rs b/src/tools/miri/tests/many-seeds/reentrant-lock.rs new file mode 100644 index 00000000000..8a363179a9c --- /dev/null +++ b/src/tools/miri/tests/many-seeds/reentrant-lock.rs @@ -0,0 +1,19 @@ +#![feature(reentrant_lock)] +//! This is a regression test for +//! <https://rust-lang.zulipchat.com/#narrow/channel/269128-miri/topic/reentrant.20lock.20failure.20on.20musl>. + +use std::cell::Cell; +use std::sync::ReentrantLock; +use std::thread; + +static LOCK: ReentrantLock<Cell<i32>> = ReentrantLock::new(Cell::new(0)); + +fn main() { + for _ in 0..20 { + thread::spawn(move || { + let val = LOCK.lock(); + val.set(val.get() + 1); + drop(val); + }); + } +} |
