From 9a2214eb5eb55f037a2217ca9da9e3dffb3a960d Mon Sep 17 00:00:00 2001 From: james7132 Date: Fri, 16 Feb 2024 12:12:07 -0800 Subject: [PATCH 1/6] Replace non-atomic load with an atomic one --- src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index a92f272..301e6f1 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -216,8 +216,7 @@ impl ThreadLocal { } unsafe { let entry = &*bucket_ptr.add(thread.index); - // Read without atomic operations as only this thread can set the value. - if (&entry.present as *const _ as *const bool).read() { + if entry.present.load(Ordering::Acquire) { Some(&*(&*entry.value.get()).as_ptr()) } else { None From 8c739d00c8abe4b52bf07d01de36fa353623b196 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 17 Feb 2024 23:23:38 -0800 Subject: [PATCH 2/6] Add regression test for miri --- src/lib.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/src/lib.rs b/src/lib.rs index 301e6f1..b31b1d7 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -529,6 +529,7 @@ mod tests { use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering::Relaxed; use std::sync::Arc; + use std::hint::black_box; use std::thread; fn make_create() -> Arc usize + Send + Sync> { @@ -609,6 +610,28 @@ mod tests { assert_eq!(vec![1, 2, 3], v); } + #[test] + fn miri_iter_soundness_check() { + let tls = Arc::new(ThreadLocal::new()); + let _local = tls.get_or(|| Box::new(1)); + + let tls2 = tls.clone(); + let join_1 = thread::spawn(move || { + let _tls = tls2.get_or(|| Box::new(2)); + let iter = tls2.iter(); + for item in iter { + black_box(item); + } + }); + + let iter = tls.iter(); + for item in iter { + black_box(item); + } + + join_1.join().ok(); + } + #[test] fn test_drop() { let local = ThreadLocal::new(); From 97fccb6d5f720792ddf71861e16d047092bff82a Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 17 Feb 2024 23:26:51 -0800 Subject: [PATCH 3/6] Add miri to CI --- .github/workflows/ci.yml | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 80a0cde..0f24372 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -15,3 +15,13 @@ jobs: - run: cargo fmt -- --check - run: cargo test - run: cargo bench + miri: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Install Rust + run: rustup toolchain install nightly --component miri && rustup default nightly + - run: cargo miri test + env: + MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation + RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout \ No newline at end of file From 5a512c48741de59b44457cc2bf50f12b7d4f54c8 Mon Sep 17 00:00:00 2001 From: james7132 Date: Sat, 17 Feb 2024 23:47:01 -0800 Subject: [PATCH 4/6] Line ending --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 0f24372..9a8f610 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -24,4 +24,4 @@ jobs: - run: cargo miri test env: MIRIFLAGS: -Zmiri-strict-provenance -Zmiri-symbolic-alignment-check -Zmiri-disable-isolation - RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout \ No newline at end of file + RUSTFLAGS: ${{ env.RUSTFLAGS }} -Z randomize-layout From 66a0d00e81bd386d70607ed009d74eae866bdd30 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 19 Feb 2024 21:02:13 -0800 Subject: [PATCH 5/6] Formatting and addressing review comments --- src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index b31b1d7..e97d68f 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -216,7 +216,7 @@ impl ThreadLocal { } unsafe { let entry = &*bucket_ptr.add(thread.index); - if entry.present.load(Ordering::Acquire) { + if entry.present.load(Ordering::Relaxed) { Some(&*(&*entry.value.get()).as_ptr()) } else { None @@ -526,10 +526,10 @@ unsafe fn deallocate_bucket(bucket: *mut Entry, size: usize) { mod tests { use super::ThreadLocal; use std::cell::RefCell; + use std::hint::black_box; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering::Relaxed; use std::sync::Arc; - use std::hint::black_box; use std::thread; fn make_create() -> Arc usize + Send + Sync> { From 0f6c70514922bfc34c2297ef4bbe96330dfba856 Mon Sep 17 00:00:00 2001 From: james7132 Date: Mon, 19 Feb 2024 23:23:46 -0800 Subject: [PATCH 6/6] std::hint::black_box is not stable until 1.66 --- src/lib.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e97d68f..9ae10d8 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -526,7 +526,6 @@ unsafe fn deallocate_bucket(bucket: *mut Entry, size: usize) { mod tests { use super::ThreadLocal; use std::cell::RefCell; - use std::hint::black_box; use std::sync::atomic::AtomicUsize; use std::sync::atomic::Ordering::Relaxed; use std::sync::Arc; @@ -620,13 +619,13 @@ mod tests { let _tls = tls2.get_or(|| Box::new(2)); let iter = tls2.iter(); for item in iter { - black_box(item); + println!("{:?}", item); } }); let iter = tls.iter(); for item in iter { - black_box(item); + println!("{:?}", item); } join_1.join().ok();