Skip to content

Commit

Permalink
Migrate standard library away from compare_and_swap
Browse files Browse the repository at this point in the history
  • Loading branch information
faern committed Nov 21, 2020
1 parent 7bd770c commit df2ef1d
Show file tree
Hide file tree
Showing 15 changed files with 79 additions and 36 deletions.
6 changes: 3 additions & 3 deletions library/core/tests/atomic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,11 @@ use core::sync::atomic::*;
#[test]
fn bool_() {
let a = AtomicBool::new(false);
assert_eq!(a.compare_and_swap(false, true, SeqCst), false);
assert_eq!(a.compare_and_swap(false, true, SeqCst), true);
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false));
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Err(true));

a.store(false, SeqCst);
assert_eq!(a.compare_and_swap(false, true, SeqCst), false);
assert_eq!(a.compare_exchange(false, true, SeqCst, SeqCst), Ok(false));
}

#[test]
Expand Down
6 changes: 5 additions & 1 deletion library/std/src/sync/mpsc/blocking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,11 @@ pub fn tokens() -> (WaitToken, SignalToken) {

impl SignalToken {
pub fn signal(&self) -> bool {
let wake = !self.inner.woken.compare_and_swap(false, true, Ordering::SeqCst);
let wake = self
.inner
.woken
.compare_exchange(false, true, Ordering::SeqCst, Ordering::SeqCst)
.is_ok();
if wake {
self.inner.thread.unpark();
}
Expand Down
14 changes: 11 additions & 3 deletions library/std/src/sync/mpsc/oneshot.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ impl<T> Packet<T> {
let ptr = unsafe { signal_token.cast_to_usize() };

// race with senders to enter the blocking state
if self.state.compare_and_swap(EMPTY, ptr, Ordering::SeqCst) == EMPTY {
if self.state.compare_exchange(EMPTY, ptr, Ordering::SeqCst, Ordering::SeqCst).is_ok() {
if let Some(deadline) = deadline {
let timed_out = !wait_token.wait_max_until(deadline);
// Try to reset the state
Expand Down Expand Up @@ -161,7 +161,12 @@ impl<T> Packet<T> {
// the state changes under our feet we'd rather just see that state
// change.
DATA => {
self.state.compare_and_swap(DATA, EMPTY, Ordering::SeqCst);
let _ = self.state.compare_exchange(
DATA,
EMPTY,
Ordering::SeqCst,
Ordering::SeqCst,
);
match (&mut *self.data.get()).take() {
Some(data) => Ok(data),
None => unreachable!(),
Expand Down Expand Up @@ -264,7 +269,10 @@ impl<T> Packet<T> {

// If we've got a blocked thread, then use an atomic to gain ownership
// of it (may fail)
ptr => self.state.compare_and_swap(ptr, EMPTY, Ordering::SeqCst),
ptr => self
.state
.compare_exchange(ptr, EMPTY, Ordering::SeqCst, Ordering::SeqCst)
.unwrap_or_else(|x| x),
};

// Now that we've got ownership of our state, figure out what to do
Expand Down
11 changes: 9 additions & 2 deletions library/std/src/sync/mpsc/shared.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,8 +385,15 @@ impl<T> Packet<T> {
self.port_dropped.store(true, Ordering::SeqCst);
let mut steals = unsafe { *self.steals.get() };
while {
let cnt = self.cnt.compare_and_swap(steals, DISCONNECTED, Ordering::SeqCst);
cnt != DISCONNECTED && cnt != steals
match self.cnt.compare_exchange(
steals,
DISCONNECTED,
Ordering::SeqCst,
Ordering::SeqCst,
) {
Ok(_) => false,
Err(old) => old != DISCONNECTED,
}
} {
// See the discussion in 'try_recv' for why we yield
// control of this thread.
Expand Down
9 changes: 6 additions & 3 deletions library/std/src/sync/mpsc/stream.rs
Original file line number Diff line number Diff line change
Expand Up @@ -322,12 +322,15 @@ impl<T> Packet<T> {
// (because there is a bounded number of senders).
let mut steals = unsafe { *self.queue.consumer_addition().steals.get() };
while {
let cnt = self.queue.producer_addition().cnt.compare_and_swap(
match self.queue.producer_addition().cnt.compare_exchange(
steals,
DISCONNECTED,
Ordering::SeqCst,
);
cnt != DISCONNECTED && cnt != steals
Ordering::SeqCst,
) {
Ok(_) => false,
Err(old) => old != DISCONNECTED,
}
} {
while self.queue.pop().is_some() {
steals += 1;
Expand Down
16 changes: 11 additions & 5 deletions library/std/src/sync/once.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@
// must do so with Release ordering to make the result available.
// - `wait` inserts `Waiter` nodes as a pointer in `state_and_queue`, and
// needs to make the nodes available with Release ordering. The load in
// its `compare_and_swap` can be Relaxed because it only has to compare
// its `compare_exchange` can be Relaxed because it only has to compare
// the atomic, not to read other data.
// - `WaiterQueue::Drop` must see the `Waiter` nodes, so it must load
// `state_and_queue` with Acquire ordering.
Expand Down Expand Up @@ -395,12 +395,13 @@ impl Once {
}
POISONED | INCOMPLETE => {
// Try to register this thread as the one RUNNING.
let old = self.state_and_queue.compare_and_swap(
let exchange_result = self.state_and_queue.compare_exchange(
state_and_queue,
RUNNING,
Ordering::Acquire,
Ordering::Acquire,
);
if old != state_and_queue {
if let Err(old) = exchange_result {
state_and_queue = old;
continue;
}
Expand Down Expand Up @@ -452,8 +453,13 @@ fn wait(state_and_queue: &AtomicUsize, mut current_state: usize) {

// Try to slide in the node at the head of the linked list, making sure
// that another thread didn't just replace the head of the linked list.
let old = state_and_queue.compare_and_swap(current_state, me | RUNNING, Ordering::Release);
if old != current_state {
let exchange_result = state_and_queue.compare_exchange(
current_state,
me | RUNNING,
Ordering::Release,
Ordering::Relaxed,
);
if let Err(old) = exchange_result {
current_state = old;
continue;
}
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys/sgx/abi/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,20 +36,20 @@ unsafe extern "C" fn tcs_init(secondary: bool) {
}

// Try to atomically swap UNINIT with BUSY. The returned state can be:
match RELOC_STATE.compare_and_swap(UNINIT, BUSY, Ordering::Acquire) {
match RELOC_STATE.compare_exchange(UNINIT, BUSY, Ordering::Acquire, Ordering::Acquire) {
// This thread just obtained the lock and other threads will observe BUSY
UNINIT => {
Ok(_) => {
reloc::relocate_elf_rela();
RELOC_STATE.store(DONE, Ordering::Release);
}
// We need to wait until the initialization is done.
BUSY => {
Err(BUSY) => {
while RELOC_STATE.load(Ordering::Acquire) == BUSY {
core::hint::spin_loop();
}
}
// Initialization is done.
DONE => {}
Err(DONE) => {}
_ => unreachable!(),
}
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys/sgx/waitqueue/spin_mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ impl<T> SpinMutex<T> {

#[inline(always)]
pub fn try_lock(&self) -> Option<SpinMutexGuard<'_, T>> {
if !self.lock.compare_and_swap(false, true, Ordering::Acquire) {
if self.lock.compare_exchange(false, true, Ordering::Acquire, Ordering::Acquire).is_ok() {
Some(SpinMutexGuard { mutex: self })
} else {
None
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys/windows/mutex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,9 +123,9 @@ impl Mutex {
let inner = box Inner { remutex: ReentrantMutex::uninitialized(), held: Cell::new(false) };
inner.remutex.init();
let inner = Box::into_raw(inner);
match self.lock.compare_and_swap(0, inner as usize, Ordering::SeqCst) {
0 => inner,
n => {
match self.lock.compare_exchange(0, inner as usize, Ordering::SeqCst, Ordering::SeqCst) {
Ok(_) => inner,
Err(n) => {
Box::from_raw(inner).remutex.destroy();
n as *const _
}
Expand Down
6 changes: 3 additions & 3 deletions library/std/src/sys_common/condvar/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ impl SameMutexCheck {
}
pub fn verify(&self, mutex: &MovableMutex) {
let addr = mutex.raw() as *const mutex_imp::Mutex as usize;
match self.addr.compare_and_swap(0, addr, Ordering::SeqCst) {
0 => {} // Stored the address
n if n == addr => {} // Lost a race to store the same address
match self.addr.compare_exchange(0, addr, Ordering::SeqCst, Ordering::SeqCst) {
Ok(_) => {} // Stored the address
Err(n) if n == addr => {} // Lost a race to store the same address
_ => panic!("attempted to use a condition variable with two mutexes"),
}
}
Expand Down
8 changes: 4 additions & 4 deletions library/std/src/sys_common/thread_local_key.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ impl StaticKey {
return key;
}

// POSIX allows the key created here to be 0, but the compare_and_swap
// POSIX allows the key created here to be 0, but the compare_exchange
// below relies on using 0 as a sentinel value to check who won the
// race to set the shared TLS key. As far as I know, there is no
// guaranteed value that cannot be returned as a posix_key_create key,
Expand All @@ -186,11 +186,11 @@ impl StaticKey {
key2
};
rtassert!(key != 0);
match self.key.compare_and_swap(0, key as usize, Ordering::SeqCst) {
match self.key.compare_exchange(0, key as usize, Ordering::SeqCst, Ordering::SeqCst) {
// The CAS succeeded, so we've created the actual key
0 => key as usize,
Ok(_) => key as usize,
// If someone beat us to the punch, use their key instead
n => {
Err(n) => {
imp::destroy(key);
n
}
Expand Down
2 changes: 1 addition & 1 deletion library/std/src/sys_common/thread_parker/futex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ impl Parker {
// Wait for something to happen, assuming it's still set to PARKED.
futex_wait(&self.state, PARKED, None);
// Change NOTIFIED=>EMPTY and return in that case.
if self.state.compare_and_swap(NOTIFIED, EMPTY, Acquire) == NOTIFIED {
if self.state.compare_exchange_weak(NOTIFIED, EMPTY, Acquire, Acquire).is_ok() {
return;
} else {
// Spurious wake up. We loop to try again.
Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/array-slice-vec/box-of-array-of-drop-1.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst
);
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/array-slice-vec/box-of-array-of-drop-2.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst
);
}
}

Expand Down
7 changes: 6 additions & 1 deletion src/test/ui/array-slice-vec/nested-vec-3.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,12 @@ impl Drop for D {
fn drop(&mut self) {
println!("Dropping {}", self.0);
let old = LOG.load(Ordering::SeqCst);
LOG.compare_and_swap(old, old << 4 | self.0 as usize, Ordering::SeqCst);
let _ = LOG.compare_exchange(
old,
old << 4 | self.0 as usize,
Ordering::SeqCst,
Ordering::SeqCst,
);
}
}

Expand Down

0 comments on commit df2ef1d

Please sign in to comment.