Skip to content

Commit 204d960

Browse files
author
James Duley
committed
Fix thread park/unpark synchronization
Previously the code below would not be guaranteed to exit when the first spawned thread took the `return, // already unparked` path because there was no write to synchronize with a read in `park`. ``` use std::sync::atomic::{AtomicBool, Ordering}; use std::thread::{current, spawn, park}; static FLAG: AtomicBool = AtomicBool::new(false); fn main() { let thread_0 = current(); spawn(move || { FLAG.store(true, Ordering::Relaxed); thread_0.unpark(); }); let thread_0 = current(); spawn(move || { thread_0.unpark(); }); while !FLAG.load(Ordering::Relaxed) { park(); } } ```
1 parent 7ee7207 commit 204d960

File tree

1 file changed

+11
-18
lines changed

1 file changed

+11
-18
lines changed

src/libstd/thread/mod.rs

Lines changed: 11 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -800,7 +800,7 @@ pub fn park() {
800800
match thread.inner.state.compare_exchange(EMPTY, PARKED, SeqCst, SeqCst) {
801801
Ok(_) => {}
802802
Err(NOTIFIED) => {
803-
thread.inner.state.store(EMPTY, SeqCst);
803+
thread.inner.state.swap(EMPTY, SeqCst);
804804
return;
805805
} // should consume this notification, so prohibit spurious wakeups in next park.
806806
Err(_) => panic!("inconsistent park state"),
@@ -889,7 +889,7 @@ pub fn park_timeout(dur: Duration) {
889889
match thread.inner.state.compare_exchange(EMPTY, PARKED, SeqCst, SeqCst) {
890890
Ok(_) => {}
891891
Err(NOTIFIED) => {
892-
thread.inner.state.store(EMPTY, SeqCst);
892+
thread.inner.state.swap(EMPTY, SeqCst);
893893
return;
894894
} // should consume this notification, so prohibit spurious wakeups in next park.
895895
Err(_) => panic!("inconsistent park_timeout state"),
@@ -1058,23 +1058,16 @@ impl Thread {
10581058
/// [park]: fn.park.html
10591059
#[stable(feature = "rust1", since = "1.0.0")]
10601060
pub fn unpark(&self) {
1061-
loop {
1062-
match self.inner.state.compare_exchange(EMPTY, NOTIFIED, SeqCst, SeqCst) {
1063-
Ok(_) => return, // no one was waiting
1064-
Err(NOTIFIED) => return, // already unparked
1065-
Err(PARKED) => {} // gotta go wake someone up
1066-
_ => panic!("inconsistent state in unpark"),
1067-
}
1068-
1069-
// Coordinate wakeup through the mutex and a condvar notification
1070-
let _lock = self.inner.lock.lock().unwrap();
1071-
match self.inner.state.compare_exchange(PARKED, NOTIFIED, SeqCst, SeqCst) {
1072-
Ok(_) => return self.inner.cvar.notify_one(),
1073-
Err(NOTIFIED) => return, // a different thread unparked
1074-
Err(EMPTY) => {} // parked thread went away, try again
1075-
_ => panic!("inconsistent state in unpark"),
1076-
}
1061+
match self.inner.state.swap(NOTIFIED, SeqCst) {
1062+
EMPTY => return, // no one was waiting
1063+
NOTIFIED => return, // already unparked
1064+
PARKED => {} // gotta go wake someone up
1065+
_ => panic!("inconsistent state in unpark"),
10771066
}
1067+
1068+
// Coordinate wakeup through the mutex and a condvar notification
1069+
let _lock = self.inner.lock.lock().unwrap();
1070+
self.inner.cvar.notify_one()
10781071
}
10791072

10801073
/// Gets the thread's unique identifier.

0 commit comments

Comments
 (0)