Skip to content

LocalPool's ThreadNotify atomic orderings are overly strict #2601

@talchas

Description

@talchas

local_pool.rs:94 is

            let unparked = thread_notify.unparked.swap(false, Ordering::Acquire);
            if !unparked {
                thread::park();
                thread_notify.unparked.store(false, Ordering::Release);
            }

The only other thread that accesses this is the ArcWake impl, which uses Relaxed, which means these orderings cannot synchronize with anything, and thus are useless. SinceWakers can be ignored and just loop { future.poll() } is valid, all three orderings can be Relaxed. If it wasn't then Release in wake_by_ref combined with Acquire in both of these operations would make sense (and possibly a loop instead of a single check, in case of thread::unpark being spammed randomly); it might also be vaguely defensible to do those orderings anyways in order to handle buggy Future impls that don't properly redo wake() if they know it has been called. In no case is there any use to the current orderings and they're just confusing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions