Soundess improvements

Cargo.toml

@@ -7,9 +7,6 @@ description = "Low-overhead parallel and async work scheduler"
 repository = "https://github.com/NthTensor/Forte"
 rust-version = "1.96.0"
 
-# Lie to cargo
-links = "forte"
-
 [workspace]
 resolver = "2"
 members = ["ci"]

src/latch.rs

@@ -13,7 +13,7 @@ use crate::platform::*;
 // States
 
 /// The default state of a latch is `LOCKED`. When in the locked state, `check`
-/// returns `false` and `wait` blocks.
+/// returns `Pending` and `wait` blocks.
 const LOCKED: u32 = 0b000;
 
 /// The latch enters the `SIGNAL` state when it is set (with error flag false).
@@ -36,17 +36,23 @@ const ASLEEP: u32 = 0b100;
 /// The latch begins as *unset* (In the `LOCKED` state), and can later be *set*
 /// by any thread (entering the `SIGNAL`) state. Each latch is "owned" by a
 /// single thread at a time; other threads may set the latch, but only the
-/// owning thread may wait on it.
+/// owning thread may call `wait` or `check`.
 ///
 /// The general idea and spirit for latches (as well as some of the
 /// documentation) is due to rayon. However the implementation is specific to
 /// forte.
 ///
 /// ## Memory Ordering
 ///
-/// Latches _do not synchronize memory_. They are only used for signaling. If
-/// the thread that sets a latch wishes to transmit a value to the thread
-/// waiting for that latch, explicit fences must be used.
+/// `set` stores the latch state with `Release` and `check` loads it with
+/// `Acquire`. A `check` that observes a set latch therefore *happens-after*
+/// the `set`.
+///
+/// Therefore, after the owning thread observes a signal through `check`:
+///
+/// 1. It may reclaim the latch (drop or `reset` it).
+///
+/// 2. It may access any writes ordered before the call to `set`.
 pub struct Latch {
     /// Holds the internal state of the latch. This tracks if the latch has been
     /// set or not.
@@ -55,9 +61,14 @@ pub struct Latch {
     semaphore: &'static Semaphore,
 }
 
+/// The result of a [`Latch::check`]: whether the latch has been set, and if so
+/// whether it was set with an error.
 pub enum Status {
+    /// The latch has not been set.
     Pending,
+    /// The latch was set without an error.
     Ok,
+    /// The latch was set with an error.
     Error,
 }
 
@@ -70,10 +81,16 @@ impl Latch {
         }
     }
 
-    /// Checks to see if the latch has been set. Returns true if it has been.
+    /// Checks to see if the latch has been set.
+    ///
+    /// # Memory Ordering
+    ///
+    /// A call to `check` that observes a signal establishes a happens-after
+    /// relationship with the `set` that produced the signal, which can be used
+    /// to safely reclaim the latch or transmit values between threads.
     #[inline(always)]
     pub fn check(&self) -> Status {
-        match self.state.load(Ordering::Relaxed) {
+        match self.state.load(Ordering::Acquire) {
             SIGNAL => Status::Ok,
             ERROR => Status::Error,
             _ => Status::Pending,
@@ -87,10 +104,8 @@ impl Latch {
     ///
     /// # Memory Ordering
     ///
-    /// This does not synchronize memory. To synchronize memory with the thread
-    /// setting the latch, call `fence(Ordering::Acquire)` after this function.
-    /// The other thread must issue a corresponding `fence(Ordering::Release)`
-    /// call.
+    /// A call to `wait` does not synchronize memory, and so must be used in
+    /// conjunction with `check` (see the type-level "Memory Ordering" section).
     #[cold]
     pub fn wait(&self, seat_bitmask: u32, waiting_bitmask: &'static AtomicU32) {
         // First, check if the latch has been set.
@@ -120,9 +135,10 @@ impl Latch {
     ///
     /// # Memory Ordering
     ///
-    /// This does not synchronize memory. To synchronize memory with the waiting
-    /// thread, call `fence(Ordering::Release)` before this function. The other
-    /// thread must issue a corresponding `fence(Ordering::Acquire)` call.
+    /// A call to `check` that observes the signal produced by this `set`
+    /// establishes a happens-after relationship. Stores on this thread made
+    /// before this call will be visible to the owning thread after `check`
+    /// returns a status other than `Pending`.
     ///
     /// # Safety
     ///
@@ -136,7 +152,7 @@ impl Latch {
     ///
     ///   2. The latch has not been `set` since it was created or last `reset`,
     ///      calls to `set` do not race, and the latch will not be dropped or
-    ///      moved until after `check` returns something other than `Pending`.
+    ///      moved until after `check` returns a status other than `Pending`.
     #[inline(always)]
     pub unsafe fn set(latch: *const Latch, error: bool) {
         // First we store a reference to the semaphore (which is 'static) so
@@ -163,13 +179,16 @@ impl Latch {
         // In the event of a race with `wait`, this may cause `wait` to return.
         // Otherwise the other thread will sleep within `wait`.
         //
-        // SAFETY: The latch is still valid to access immutably, following the
-        // same logic as above.
+        // SAFETY: The latch must still be valid to dereference, up to the point
+        // where we begin this store, by the same reasoning as above.
         //
-        // NOTE: This store will mean `check` no longer returns `Pending`,
-        // invalidating the argument in Variant 2. The latch pointer therefore
-        // may become dangling after this line.
-        unsafe { (*latch).state.store(state, Ordering::Relaxed) };
+        // The `Release` store synchronizes with the `Acquire` load in `check`
+        // to supply the second half of the Variant 2 argument: the latch does
+        // not merely live *up to* this store, the store is also ordered
+        // *before* any later free. The owner may free the latch as soon as
+        // `check` reports non-`Pending`, and that check is ordered after this
+        // store, so there can be no data race.
+        unsafe { (*latch).state.store(state, Ordering::Release) };
         // Finally we try to signal the target thread on it's semaphore, just in
         // case it missed the notification and is currently waiting. This
         // guarantees that the other thread will make progress.
@@ -201,7 +220,7 @@ pub struct Semaphore {
 
 impl Semaphore {
     /// Creates a new signal.
-    pub const fn new() -> Self {
+    pub(crate) const fn new() -> Self {
         Semaphore {
             state: AtomicU32::new(LOCKED),
         }

src/lib.rs

@@ -178,7 +178,6 @@
 //! * *Block on.* Waits for a future to complete (outside of an async context).
 //! * *Broadcast.* Runs the same operation across all workers.
 //!
-//!
 
 #![no_std]
 
@@ -209,10 +208,27 @@ pub struct FnOnceMarker();
 #[doc(hidden)]
 pub struct FutureMarker();
 
+// -----------------------------------------------------------------------------
+// Trait sealing
+
+/// Seals [`Spawn`] and [`SpawnScoped`] so they cannot be implemented outside
+/// this crate.
+mod sealed {
+    use core::future::Future;
+
+    use crate::FnOnceMarker;
+    use crate::FutureMarker;
+    use crate::Worker;
+
+    pub trait Sealed<M> {}
+
+    impl<F: FnOnce(&Worker)> Sealed<FnOnceMarker> for F {}
+    impl<Fut: Future> Sealed<FutureMarker> for Fut {}
+}
+
 // -----------------------------------------------------------------------------
 // Top-level exports
 
-pub use latch::Latch;
 pub use scope::Scope;
 pub use scope::SpawnScoped;
 pub use thread_pool::Broadcast;
@@ -233,30 +249,45 @@ pub use thread_pool::spawn;
 pub use thread_pool::spawn_broadcast;
 pub use thread_pool::spawn_on;
 
+// -----------------------------------------------------------------------------
+// Internal primitives
+
+/// Low-level scheduling primitives for building custom executors on forte.
+///
+/// These are fully public but `#[doc(hidden)]`. You can use this API, but it's
+/// not advisable, and most of it is very unsafe.
+#[doc(hidden)]
+pub mod internals {
+    pub use crate::job::HeapJob;
+    pub use crate::job::JobQueue;
+    pub use crate::job::JobRef;
+    pub use crate::job::StackJob;
+    pub use crate::latch::Latch;
+    pub use crate::latch::Semaphore;
+    pub use crate::latch::Status;
+    pub use crate::thread_pool::Scheduler;
+}
+
 // -----------------------------------------------------------------------------
 // Platform Support
 
 // This exists to make it easy to swap out the basic parallelism primitives.
 // Currently there are no alternative implementations, but there may be in
 // future.
 mod platform {
-
+    pub use alloc::sync::Arc;
     pub use core::sync::atomic::AtomicBool;
     pub use core::sync::atomic::AtomicPtr;
     pub use core::sync::atomic::AtomicU32;
     pub use core::sync::atomic::AtomicUsize;
     pub use core::sync::atomic::Ordering;
     pub use core::sync::atomic::fence;
-
-    pub use alloc::sync::Arc;
+    use std::sync::LazyLock;
     pub use std::sync::Mutex;
     pub use std::thread::Builder as ThreadBuilder;
     pub use std::thread::JoinHandle;
-    pub use std::thread_local;
-
     pub use std::thread::available_parallelism;
-
-    use std::sync::LazyLock;
+    pub use std::thread_local;
     pub struct Lazy<T>(LazyLock<T>);
 
     impl<T> Lazy<T> {