mvdnes / spin-rs

Spin-based synchronization primitives
MIT License
466 stars 86 forks source link

Documentation: Clarify guaranteed happens-before semantics for `Once` #158

Open briansmith opened 8 months ago

briansmith commented 8 months ago

The implementation of Once provides acquire/release semantics but this is not promised anywhere in the documentation. I would like to submit a PR that provides this guarantee. In OnceCell, the issue was addressed in https://github.com/matklad/once_cell/pull/85/files by adding this to its documentation:

    /// Reading a non-`None` value out of `OnceCell` establishes a
    /// happens-before relationship with a corresponding write. For example, if
    /// thread A initializes the cell with `get_or_init(f)`, and thread B
    /// subsequently reads the result of this call, B also observes all the side
    /// effects of `f`.

I would like to add this guarantee to spin-rs's documentation too.

Notable, ring (https://github.com/briansmith/ring) has been depending on this for several years. See https://github.com/briansmith/ring/issues/931. Basically, ring uses a Once<()> to guard the initialization of a global variable (static mut) that is shared between Rust, C, and assembly language code, which all accesses it without any synchronization other than verifying that the Once<()> has been initialized.

In once.rs we have:

    let xchg = self.status.compare_exchange(
                Status::Incomplete,
                Status::Running,
                Ordering::Acquire,
                Ordering::Acquire,
            );

and then we have:

            // SAFETY: Release is required here, so that all memory accesses done in the
            // closure when initializing, become visible to other threads that perform Acquire
            // loads.
            self.status.store(Status::Complete, Ordering::Release);
zesterer commented 8 months ago

I think this is a very reasonable thing to promise at the API level given the intended (and existing) uses of Once. I'd be happy to accept this.