From 79783084c4dc7484bb96c4d6238398f0ef84dfa4 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Tue, 22 Dec 2020 16:28:10 +0900 Subject: [PATCH 01/13] Update crossbeam-channel to 0.5 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 573d27e..8674ff8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -21,7 +21,7 @@ maintenance = { status = "passively-maintained" } atomic-option = "0.1" num_cpus = "1.6.2" parking_lot_core = "0.7" -crossbeam-channel = "0.4" +crossbeam-channel = "0.5" [profile.release] debug = true From c54d6f0b39a907be5258b74a3566964711dbcf77 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Tue, 22 Dec 2020 16:28:28 +0900 Subject: [PATCH 02/13] Update parking_lot_core to 0.8 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 8674ff8..939fe00 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ maintenance = { status = "passively-maintained" } [dependencies] atomic-option = "0.1" num_cpus = "1.6.2" -parking_lot_core = "0.7" +parking_lot_core = "0.8" crossbeam-channel = "0.5" [profile.release] From 5618b7bfaa7911251b7e2ca3bee06ba1416de9ea Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Sat, 26 Dec 2020 12:07:11 +0900 Subject: [PATCH 03/13] Fix miri test --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 3a793dd..9975035 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -13,7 +13,7 @@ jobs: components: - miri # ignore leaks due to https://github.com/crossbeam-rs/crossbeam/issues/464 - - bash: yes | cargo miri -Zmiri-ignore-leaks test + - bash: MIRIFLAGS="-Zmiri-ignore-leaks" cargo miri test displayName: cargo miri test - job: asan displayName: "Run address sanitizer on test suite" From 95aa5bf8b160ee245bcfd3d61c43cbda17663a72 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 11:40:31 +0900 Subject: [PATCH 04/13] Move drop(state) to fix UB --- src/lib.rs | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index 3016c71..6471395 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -195,16 +195,18 @@ impl Seat { // safely take a mutable reference, and just take the val instead of cloning it. unsafe { &mut *self.state.get() }.val.take().unwrap() } else { - state + let v = state .val .clone() - .expect("seat that should be occupied was empty") + .expect("seat that should be occupied was empty"); + + // let writer know that we no longer need this item. + // state is no longer safe to access. + #[allow(clippy::drop_ref)] + drop(state); + v }; - // let writer know that we no longer need this item. - // state is no longer safe to access. - #[allow(clippy::drop_ref)] - drop(state); self.read.fetch_add(1, atomic::Ordering::AcqRel); if let Some(t) = waiting { From 732dc9f4f5e45856640a1aafafb7f22af3f5a9dd Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 11:40:57 +0900 Subject: [PATCH 05/13] Disable Miri weak memory emulation and preemption --- azure-pipelines.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 9975035..58b63cd 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -13,7 +13,9 @@ jobs: components: - miri # ignore leaks due to https://github.com/crossbeam-rs/crossbeam/issues/464 - - bash: MIRIFLAGS="-Zmiri-ignore-leaks" cargo miri test + # disable preemption due to https://github.com/rust-lang/rust/issues/55005 + # disable weak memory emulation due to https://github.com/rust-lang/miri/issues/2223 + - bash: MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-preemption-rate=0 -Zmiri-disable-weak-memory-emulation" cargo miri test displayName: cargo miri test - job: asan displayName: "Run address sanitizer on test suite" From 4384de5a493e06d7c2472bb5bd4e5aa77ff0903c Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 11:41:27 +0900 Subject: [PATCH 06/13] Update parking_lot_core to 0.9 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 939fe00..907bcfe 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -20,7 +20,7 @@ maintenance = { status = "passively-maintained" } [dependencies] atomic-option = "0.1" num_cpus = "1.6.2" -parking_lot_core = "0.8" +parking_lot_core = "0.9" crossbeam-channel = "0.5" [profile.release] From 183dc7260dfd08b2c4953fcfb6c7c46b907394b0 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 11:54:48 +0900 Subject: [PATCH 07/13] Use ubuntu-latest instead of ubuntu-16.04 in CI --- azure-pipelines.yml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 58b63cd..fac8820 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -5,7 +5,7 @@ jobs: - job: miri displayName: "Run miri on test suite" pool: - vmImage: ubuntu-16.04 + vmImage: ubuntu-latest steps: - template: install-rust.yml@templates parameters: @@ -20,7 +20,7 @@ jobs: - job: asan displayName: "Run address sanitizer on test suite" pool: - vmImage: ubuntu-16.04 + vmImage: ubuntu-latest steps: - template: install-rust.yml@templates parameters: @@ -35,7 +35,7 @@ jobs: - job: lsan displayName: "Run leak sanitizer on test suite" pool: - vmImage: ubuntu-16.04 + vmImage: ubuntu-latest steps: - template: install-rust.yml@templates parameters: From a1177d4f84d96949b83c9b8aa63cb6e231f3070a Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 12:04:37 +0900 Subject: [PATCH 08/13] Fix Miri installation --- azure-pipelines.yml | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index fac8820..4908115 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -7,11 +7,8 @@ jobs: pool: vmImage: ubuntu-latest steps: - - template: install-rust.yml@templates - parameters: - rust: nightly - components: - - miri + - bash: rustup toolchain install nightly --component miri && rustup default nightly + displayName: install rust # ignore leaks due to https://github.com/crossbeam-rs/crossbeam/issues/464 # disable preemption due to https://github.com/rust-lang/rust/issues/55005 # disable weak memory emulation due to https://github.com/rust-lang/miri/issues/2223 From a26092c77a76c670445e9f627a8e60b62f3e508c Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 12:15:07 +0900 Subject: [PATCH 09/13] Pass -Zmiri-disable-isolation --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 4908115..14f2ac1 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -12,7 +12,7 @@ jobs: # ignore leaks due to https://github.com/crossbeam-rs/crossbeam/issues/464 # disable preemption due to https://github.com/rust-lang/rust/issues/55005 # disable weak memory emulation due to https://github.com/rust-lang/miri/issues/2223 - - bash: MIRIFLAGS="-Zmiri-ignore-leaks -Zmiri-preemption-rate=0 -Zmiri-disable-weak-memory-emulation" cargo miri test + - bash: MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-ignore-leaks -Zmiri-preemption-rate=0 -Zmiri-disable-weak-memory-emulation" cargo miri test displayName: cargo miri test - job: asan displayName: "Run address sanitizer on test suite" From b70eee7a1e966e11c877fa54d4027bdbf07acfce Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 12:58:55 +0900 Subject: [PATCH 10/13] Update azure pipelines templates --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 14f2ac1..1b0c56d 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -57,5 +57,5 @@ resources: - repository: templates type: github name: crate-ci/azure-pipelines - ref: refs/heads/v0.3 + ref: refs/heads/v0.4 endpoint: jonhoo From eaed299388144156fe079d922c739af911c24a12 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 12:59:27 +0900 Subject: [PATCH 11/13] Bump MSRV to 1.49 --- azure-pipelines.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/azure-pipelines.yml b/azure-pipelines.yml index 1b0c56d..f457f9c 100644 --- a/azure-pipelines.yml +++ b/azure-pipelines.yml @@ -1,7 +1,7 @@ jobs: - template: default.yml@templates parameters: - minrust: 1.36.0 # MaybeUninit + minrust: 1.49.0 # parking_lot_core - job: miri displayName: "Run miri on test suite" pool: From a92614c7c1e415806e7ee49d557b11512477af34 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 14:05:02 +0900 Subject: [PATCH 12/13] Ignore a doc test with Miri --- src/lib.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib.rs b/src/lib.rs index 6471395..bd3247b 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -35,6 +35,7 @@ //! Multi-send, multi-consumer example //! //! ```rust +//! # if cfg!(miri) { return } // Miri is too slow //! use bus::Bus; //! use std::thread; //! From 5d85f31845e03916a3b8746e086883bd72c1bf25 Mon Sep 17 00:00:00 2001 From: Taiki Endo Date: Fri, 17 Jun 2022 14:05:09 +0900 Subject: [PATCH 13/13] Remove dependency on unsound atomic-option crate --- Cargo.toml | 1 - src/lib.rs | 54 ++++++++++++++++++++++++++++++++++++++++++++++++++---- 2 files changed, 50 insertions(+), 5 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 907bcfe..a1b647a 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -18,7 +18,6 @@ codecov = { repository = "jonhoo/bus", branch = "master", service = "github" } maintenance = { status = "passively-maintained" } [dependencies] -atomic-option = "0.1" num_cpus = "1.6.2" parking_lot_core = "0.9" crossbeam-channel = "0.5" diff --git a/src/lib.rs b/src/lib.rs index bd3247b..bf5618d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -100,12 +100,13 @@ #![deny(missing_docs)] #![warn(rust_2018_idioms)] -use atomic_option::AtomicOption; use crossbeam_channel as mpsc; use parking_lot_core::SpinWait; use std::cell::UnsafeCell; +use std::marker::PhantomData; use std::ops::Deref; +use std::ptr; use std::sync::atomic; use std::sync::mpsc as std_mpsc; use std::sync::Arc; @@ -190,7 +191,7 @@ impl Seat { // we're the last reader, so we may need to notify the writer there's space in the buf. // can be relaxed, since the acquire at the top already guarantees that we'll see // updates. - waiting = self.waiting.take(atomic::Ordering::Relaxed); + waiting = self.waiting.take(); // since we're the last reader, no-one else will be cloning this value, so we can // safely take a mutable reference, and just take the val instead of cloning it. @@ -381,7 +382,7 @@ impl Bus { // no, so block by parking and telling readers to notify on last read self.state.ring[fence] .waiting - .replace(Some(Box::new(thread::current())), atomic::Ordering::Relaxed); + .swap(Some(Box::new(thread::current()))); // need the atomic fetch_add to ensure reader threads will see the new .waiting self.state.ring[fence] @@ -419,7 +420,7 @@ impl Bus { let state = unsafe { &mut *next.state.get() }; state.max = readers; state.val = Some(val); - next.waiting.replace(None, atomic::Ordering::Relaxed); + next.waiting.take(); next.read.store(0, atomic::Ordering::Release); } self.rleft[tail] = 0; @@ -820,3 +821,48 @@ impl Iterator for BusIntoIter { self.0.recv().ok() } } + +struct AtomicOption { + ptr: atomic::AtomicPtr, + _marker: PhantomData>>, +} + +unsafe impl Send for AtomicOption {} +unsafe impl Sync for AtomicOption {} + +impl AtomicOption { + fn empty() -> Self { + Self { + ptr: atomic::AtomicPtr::new(ptr::null_mut()), + _marker: PhantomData, + } + } + + fn swap(&self, val: Option>) -> Option> { + let old = match val { + Some(val) => self.ptr.swap(Box::into_raw(val), atomic::Ordering::AcqRel), + // Acquire is needed to synchronize with the store of a non-null ptr, but since a null ptr + // will never be dereferenced, there is no need to synchronize the store of a null ptr. + None => self.ptr.swap(ptr::null_mut(), atomic::Ordering::Acquire), + }; + if old.is_null() { + None + } else { + // SAFETY: + // - AcqRel/Acquire ensures that it does not read a pointer to potentially invalid memory. + // - We've checked that old is not null. + // - We do not store invalid pointers other than null in self.ptr. + Some(unsafe { Box::from_raw(old) }) + } + } + + fn take(&self) -> Option> { + self.swap(None) + } +} + +impl Drop for AtomicOption { + fn drop(&mut self) { + drop(self.take()); + } +}