Repository: tokio-rs/tokio
File: tokio/src/sync/notify.rs (Rust)

Code before:
pub struct Notify {
    // `state` uses 2 bits to store one of `EMPTY`,
    // `WAITING` or `NOTIFIED`. The rest of the bits
    // are used to store the number of times `notify_waiters`
    // was called.
    //
    // Throughout the code there are two assumptions:
    // - state can be transitioned *from* `WAITING` only if
    //   `waiters` lock is held
    // - number of times `notify_waiters` was called can
    //   be modified only if `waiters` lock is held
    state: AtomicUsize,
    waiters: Mutex<WaitList>,
}

#[derive(Debug)]
struct Waiter {
    /// Intrusive linked-list pointers.
    pointers: linked_list::Pointers<Waiter>,

    /// Waiting task's waker.
    waker: UnsafeCell<Option<Waker>>,

    /// Notification for this waiter. It is an enum and uses
    /// `NOTIFICATION_{NONE, ONE, ALL}` values.
    notification: AtomicUsize,

    /// Should not be `Unpin`.
    _p: PhantomPinned,
}

impl Waiter {
    fn new() -> Waiter {
        Waiter {
            pointers: linked_list::Pointers::new(),
            waker: UnsafeCell::new(None),
            notification: AtomicUsize::new(NOTIFICATION_NONE),
            _p: PhantomPinned,
        }
    }
}

generate_addr_of_methods! {
    impl<> Waiter {
        unsafe fn addr_of_pointers(self: NonNull<Self>) -> NonNull<linked_list::Pointers<Waiter>> {
            &self.pointers
        }
    }
}

// No notification.

Review comment (suggestion):
I think the code is ok, but the safety invariants invovled with this atomic should be documented more clearly.

Code after:
pub struct Notify {
    // `state` uses 2 bits to store one of `EMPTY`,
    // `WAITING` or `NOTIFIED`. The rest of the bits
    // are used to store the number of times `notify_waiters`
    // was called.
    //
    // Throughout the code there are two assumptions:
    // - state can be transitioned *from* `WAITING` only if
    //   `waiters` lock is held
    // - number of times `notify_waiters` was called can
    //   be modified only if `waiters` lock is held
    state: AtomicUsize,
    waiters: Mutex<WaitList>,
}

#[derive(Debug)]
struct Waiter {
    /// Intrusive linked-list pointers.
    pointers: linked_list::Pointers<Waiter>,

    /// Waiting task's waker. Depending on the value of `notification`,
    /// this field is either protected by the `waiters` lock in
    /// `Notify`, or it is exclusively owned by the enclosing `Waiter`.
    waker: UnsafeCell<Option<Waker>>,

    /// Notification for this waiter.
    /// * if it's `None`, then `waker` is protected by the `waiters` lock.
    /// * if it's `Some`, then `waker` is exclusively owned by the
    ///   enclosing `Waiter` and can be accessed without locking.
    notification: AtomicNotification,

    /// Should not be `Unpin`.
    _p: PhantomPinned,
}

impl Waiter {
    fn new() -> Waiter {
        Waiter {
            pointers: linked_list::Pointers::new(),
            waker: UnsafeCell::new(None),
            notification: AtomicNotification::none(),
            _p: PhantomPinned,
        }
    }
}

generate_addr_of_methods! {
    impl<> Waiter {
        unsafe fn addr_of_pointers(self: NonNull<Self>) -> NonNull<linked_list::Pointers<Waiter>> {
            &self.pointers
        }