I’m looking at the WaiterQueue used in std::sync::once, in particular I was curious about how thread::park and Thread::unpark can be used correctly.
In this use-case, park and unpark are used together with an AtomicBool like this:
parking:
// the AtomicBool is in `node.signaled`
while !node.signaled.load(Ordering::Acquire) {
thread::park();
}
(source)
unparking:
(*queue).signaled.store(true, Ordering::Release);
// ^- FIXME (maybe): This is another case of issue #55005
// `store()` has a potentially dangling ref to `signaled`.
queue = next;
thread.unpark();
(source)
As far as I can tell, #55005 is about something different, unrelated to what I’m discussing here.
To come to my question, I’m trying to understand the logic here, in particular the code for unparking. Let me quote the nomicon:
Intuitively, an acquire access ensures that every access after it stays after it. However operations that occur before an acquire are free to be reordered to occur after it. Similarly, a release access ensures that every access before it stays before it. However operations that occur after a release are free to be reordered to occur before it.
Now, on the unparking side, an Ordering::Release is used, so I would naively want to assume that the compiler could legally re-order this into something like
thread.unpark();
(*queue).signaled.store(true, Ordering::Release);
queue = next;
except it can’t (I hope) since otherwise, we could have this kind of interleaving
// unparking thread:
thread.unpark();
// CONTEXT SWITCH
// parked thread, leaving its `thread::park()` call
while !node.signaled.load(Ordering::Acquire) // -> `signaled` still false, loop again
thread::park(); // we’re parked again, waiting for the next `unpark`
// CONTEXT SWITCH
// unparking thread:
(*queue).signaled.store(true, Ordering::Release);
queue = next;
// ...
where the parked thread is not woken up successfully.
So my question is: What kind of atomics-intricacy prevents this reordering? Or is there a bug in the standard library and what’s actually needed is something like the following?
(*queue).signaled.swap(true, Ordering::SeqCst);
queue = next;
thread.unpark();
I’m getting really insecure about atomics these days—perhaps the thread.unpark() call would even need be changed to logically depend on the result of the .swap() to really ensure that no reordering happens?
Edit: Iʼm only just now noticing that the example on the thread::park docs uses the same kind of AtomicBool flag with a Release store. Also: sorry if this thread seems like a better fit for URLO; I thought it should fit IRLO, too, because Iʼd like to potentially get some bugs fixed in the standard library or at least have its documentation improved by adding more explanation for why this example code is safe.