From df4457e20bb6017de97ff69af58a0a92a3a5b423 Mon Sep 17 00:00:00 2001 From: Austin Kiekintveld Date: Sun, 1 May 2022 16:46:19 -0700 Subject: [PATCH 1/3] Relax memory ordering used in SameMutexCheck `SameMutexCheck` only requires atomicity for `self.addr`, but does not need ordering of other memory accesses in either the success or failure case. Using `Relaxed`, the code still correctly handles the case when two threads race to store an address. --- library/std/src/sys_common/condvar/check.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs index 7671850ac55b8..69cad21b09608 100644 --- a/library/std/src/sys_common/condvar/check.rs +++ b/library/std/src/sys_common/condvar/check.rs @@ -24,7 +24,7 @@ impl SameMutexCheck { } pub fn verify(&self, mutex: &MovableMutex) { let addr = mutex.raw() as *const imp::Mutex as *const () as *mut _; - match self.addr.compare_exchange(ptr::null_mut(), addr, Ordering::SeqCst, Ordering::SeqCst) + match self.addr.compare_exchange(ptr::null_mut(), addr, Ordering::Relaxed, Ordering::Relaxed) { Ok(_) => {} // Stored the address Err(n) if n == addr => {} // Lost a race to store the same address From a05df2ea19c1a2c074fcf98211d3418ba9af479c Mon Sep 17 00:00:00 2001 From: Austin Kiekintveld Date: Sun, 1 May 2022 19:02:28 -0700 Subject: [PATCH 2/3] Fix formatting --- library/std/src/sys_common/condvar/check.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs index 69cad21b09608..a3842b3f857ce 100644 --- a/library/std/src/sys_common/condvar/check.rs +++ b/library/std/src/sys_common/condvar/check.rs @@ -24,8 +24,12 @@ impl SameMutexCheck { } pub fn verify(&self, mutex: &MovableMutex) { let addr = mutex.raw() as *const imp::Mutex as *const () as *mut _; - match self.addr.compare_exchange(ptr::null_mut(), addr, Ordering::Relaxed, Ordering::Relaxed) - { + match self.addr.compare_exchange( + ptr::null_mut(), + addr, + Ordering::Relaxed, + Ordering::Relaxed, + ) { 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"), From 55a7d18189cfa65a35dacf63ee9e70aedd85e3c4 Mon Sep 17 00:00:00 2001 From: Austin Kiekintveld Date: Sun, 1 May 2022 19:07:36 -0700 Subject: [PATCH 3/3] Add comment --- library/std/src/sys_common/condvar/check.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/library/std/src/sys_common/condvar/check.rs b/library/std/src/sys_common/condvar/check.rs index a3842b3f857ce..d0d0d59651895 100644 --- a/library/std/src/sys_common/condvar/check.rs +++ b/library/std/src/sys_common/condvar/check.rs @@ -24,6 +24,8 @@ impl SameMutexCheck { } pub fn verify(&self, mutex: &MovableMutex) { let addr = mutex.raw() as *const imp::Mutex as *const () as *mut _; + // Relaxed is okay here because we never read through `self.addr`, and only use it to + // compare addresses. match self.addr.compare_exchange( ptr::null_mut(), addr,