Skip to content

Safety hazard: thread::park() doesn't guarantee being unwind-free but sync::Once implicitly relies on it being unwind-free #89571

Closed
@Kixunil

Description

@Kixunil

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 in Once)
  • Explicitly document that this is not guaranteed with a warning about this footgun and change Once to abort if park panics.

Metadata

Metadata

Assignees

No one assigned

    Labels

    A-atomicArea: Atomics, barriers, and sync primitivesI-unsoundIssue: A soundness hole (worst kind of bug), see: https://en.wikipedia.org/wiki/SoundnessT-libsRelevant to the library team, which will review and decide on the PR/issue.

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions