Closed
Description
I noticed Once
contains this code:
// We have enqueued ourselves, now lets wait.
// It is important not to return before being signaled, otherwise we
// would drop our `Waiter` node and leave a hole in the linked list
// (and a dangling reference). Guard against spurious wakeups by
// reparking ourselves until we are signaled.
while !node.signaled.load(Ordering::Acquire) {
// If the managing thread happens to signal and unpark us before we
// can park ourselves, the result could be this thread never gets
// unparked. Luckily `park` comes with the guarantee that if it got
// an `unpark` just before on an unparked thread it does not park.
thread::park();
}
As the comment correctly explains the while loop must not end prematurely. However if thread::park()
unwinds it would break the loop.
Does thread::park()
panic though?
I found at least two instances: 1 2 where it explicitly could and I didn't review all implementations, nor internals of Mutex
and CondVar
. I understand those are supposed to be unlikely but I guess they were intended to reduce unsafety but amusingly they increased it for Once
.
I think it'd be safer to either:
- Guarantee that
park
can't unwind, change those panics to aborts and document it, ideally make this a public guarantee as well. (I noticed this issue because I'm writing something with structure similar to that piece inOnce
) - Explicitly document that this is not guaranteed with a warning about this footgun and change
Once
to abort ifpark
panics.