Skip to content

Commit d06ca0f

Browse files
committed
Auto merge of rust-lang#114591 - joboet:thread_parking_ordering_fix, r=thomcc
Synchronize with all calls to `unpark` in id-based thread parker [The documentation for `thread::park`](https://doc.rust-lang.org/nightly/std/thread/fn.park.html#memory-ordering) guarantees that "park synchronizes-with all prior unpark operations". In the id-based thread parking implementation, this is not implemented correctly, as the state variable is reset with a simple store, so there will not be a *synchronizes-with* edge if an `unpark` happens just before the reset. This PR corrects this, replacing the load-check-reset sequence with a single `compare_exchange`.
2 parents ee5cb9e + fd9fcc5 commit d06ca0f

File tree

1 file changed

+6
-11
lines changed
  • library/std/src/sys_common/thread_parking

1 file changed

+6
-11
lines changed

library/std/src/sys_common/thread_parking/id.rs

+6-11
Original file line numberDiff line numberDiff line change
@@ -56,18 +56,14 @@ impl Parker {
5656
self.init_tid();
5757

5858
// Changes NOTIFIED to EMPTY and EMPTY to PARKED.
59-
let mut state = self.state.fetch_sub(1, Acquire).wrapping_sub(1);
60-
if state == PARKED {
59+
let state = self.state.fetch_sub(1, Acquire);
60+
if state == EMPTY {
6161
// Loop to guard against spurious wakeups.
62-
while state == PARKED {
62+
// The state must be reset with acquire ordering to ensure that all
63+
// calls to `unpark` synchronize with this thread.
64+
while self.state.compare_exchange(NOTIFIED, EMPTY, Acquire, Relaxed).is_err() {
6365
park(self.state.as_ptr().addr());
64-
state = self.state.load(Acquire);
6566
}
66-
67-
// Since the state change has already been observed with acquire
68-
// ordering, the state can be reset with a relaxed store instead
69-
// of a swap.
70-
self.state.store(EMPTY, Relaxed);
7167
}
7268
}
7369

@@ -78,8 +74,7 @@ impl Parker {
7874
if state == PARKED {
7975
park_timeout(dur, self.state.as_ptr().addr());
8076
// Swap to ensure that we observe all state changes with acquire
81-
// ordering, even if the state has been changed after the timeout
82-
// occurred.
77+
// ordering.
8378
self.state.swap(EMPTY, Acquire);
8479
}
8580
}

0 commit comments

Comments
 (0)