pw_sync: Updates the documentation and headers

Updates the pw_sync documentation to properly cover critical
section locks and briefly cover signaling.

Change-Id: I54497d2d98b47ce14f83f3b115c031d4023d376d
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/37564
Reviewed-by: Keir Mierle <keir@google.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
diff --git a/pw_sync/docs.rst b/pw_sync/docs.rst
index 1dfe23f..8af6fea 100644
--- a/pw_sync/docs.rst
+++ b/pw_sync/docs.rst
@@ -1,8 +1,413 @@
 .. _module-pw_sync:
 
--------
+=======
 pw_sync
--------
-This is a synchronization module for Pigweed. It is not ready for use, and
-is under construction.
+=======
+The ``pw_sync`` module contains utilities for synchronizing between threads
+and/or interrupts through signaling primitives and critical section lock
+primitives.
 
+.. contents::
+   :local:
+   :depth: 2
+
+.. Warning::
+  This module is still under construction, the API is not yet stable.
+
+.. Note::
+  The objects in this module do not have an Init() style public API which is
+  common in many RTOS C APIs. Instead, they rely on being able to invoke the
+  native initialization APIs for synchronization primitives during C++
+  construction.
+  In order to support global statically constructed synchronization without
+  constexpr constructors, the user and/or backend **MUST** ensure that any
+  initialization required in your environment is done prior to the creation
+  and/or initialization of the native synchronization primitives
+  (e.g. kernel initialization).
+
+--------------------------------
+Critical Section Lock Primitives
+--------------------------------
+The critical section lock primitives provided by this module comply with
+`BasicLockable <https://en.cppreference.com/w/cpp/named_req/BasicLockable>`_,
+`Lockable <https://en.cppreference.com/w/cpp/named_req/Lockable>`_, and where
+relevant
+`TimedLockable <https://en.cppreference.com/w/cpp/named_req/TimedLockable>`_ C++
+named requirements. This means that they are compatible with existing helpers in
+the STL's ``<mutex>`` thread support library. For example `std::lock_guard <https://en.cppreference.com/w/cpp/thread/lock_guard>`_
+and `std::unique_lock <https://en.cppreference.com/w/cpp/thread/unique_lock>`_ can be directly used.
+
+Mutex
+=====
+The Mutex is a synchronization primitive that can be used to protect shared data
+from being simultaneously accessed by multiple threads. It offers exclusive,
+non-recursive ownership semantics where priority inheritance is used to solve
+the classic priority-inversion problem.
+
+The Mutex's API is C++11 STL
+`std::timed_mutex <https://en.cppreference.com/w/cpp/thread/timed_mutex>`_ like,
+meaning it is a
+`BasicLockable <https://en.cppreference.com/w/cpp/named_req/BasicLockable>`_,
+`Lockable <https://en.cppreference.com/w/cpp/named_req/Lockable>`_, and
+`TimedLockable <https://en.cppreference.com/w/cpp/named_req/TimedLockable>`_.
+
+
+C++
+---
+.. cpp:class:: pw::sync::Mutex
+
+  .. cpp:function:: void lock()
+
+     Locks the mutex, blocking indefinitely. Failures are fatal.
+
+     **Precondition:** The lock isn't already held by this thread. Recursive
+     locking is undefined behavior.
+
+  .. cpp:function:: bool try_lock()
+
+     Attempts to lock the mutex in a non-blocking manner.
+     Returns true if the mutex was successfully acquired.
+
+     **Precondition:** The lock isn't already held by this thread. Recursive
+     locking is undefined behavior.
+
+  .. cpp:function:: bool try_lock_for(chrono::SystemClock::duration for_at_least)
+
+     Attempts to lock the mutex where, if needed, blocking for at least the
+     specified duration.
+     Returns true if the mutex was successfully acquired.
+
+     **Precondition:** The lock isn't already held by this thread. Recursive
+     locking is undefined behavior.
+
+  .. cpp:function:: bool try_lock_until(chrono::SystemClock::time_point until_at_least)
+
+     Attempts to lock the mutex where, if needed, blocking until at least the
+     specified time_point.
+     Returns true if the mutex was successfully acquired.
+
+     **Precondition:** The lock isn't already held by this thread. Recursive
+     locking is undefined behavior.
+
+  .. cpp:function:: void unlock()
+
+     Unlocks the mutex. Failures are fatal.
+
+     **Precondition:** The mutex is held by this thread.
+
+  +--------------------------------+----------+-------------+-------+
+  | *Safe to use in context*       | *Thread* | *Interrupt* | *NMI* |
+  +--------------------------------+----------+-------------+-------+
+  | ``Mutex::Mutex``               | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``Mutex::~Mutex``              | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``void Mutex::lock``           | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``bool Mutex::try_lock``       | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``bool Mutex::try_lock_for``   | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``bool Mutex::try_lock_until`` | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+  | ``void Mutex::unlock``         | ✔        |             |       |
+  +--------------------------------+----------+-------------+-------+
+
+
+Examples in C++
+^^^^^^^^^^^^^^^
+.. code-block:: cpp
+
+  #include "pw_chrono/system_clock.h"
+  #include "pw_sync/mutex.h"
+
+  pw::sync::Mutex mutex;
+
+  void ThreadSafeCriticalSection() {
+    mutex.lock();
+    NotThreadSafeCriticalSection();
+    mutex.unlock();
+  }
+
+  bool ThreadSafeCriticalSectionWithTimeout(
+      const SystemClock::duration timeout) {
+    if (!mutex.try_lock_for(timeout)) {
+      return false;
+    }
+    NotThreadSafeCriticalSection();
+    mutex.unlock();
+    return true;
+  }
+
+
+Alternatively you can use C++'s RAII helpers to ensure you always unlock.
+
+.. code-block:: cpp
+
+  #include <mutex>
+
+  #include "pw_chrono/system_clock.h"
+  #include "pw_sync/mutex.h"
+
+  pw::sync::Mutex mutex;
+
+  void ThreadSafeCriticalSection() {
+    std::lock_guard lock(mutex);
+    NotThreadSafeCriticalSection();
+  }
+
+  bool ThreadSafeCriticalSectionWithTimeout(
+      const SystemClock::duration timeout) {
+    std::unique_lock lock(mutex, std::defer_lock);
+    if (!lock.try_lock_for(timeout)) {
+      return false;
+    }
+    NotThreadSafeCriticalSection();
+    return true;
+  }
+
+
+
+C
+-
+The Mutex must be created in C++, however it can be passed into C using the
+``pw_sync_Mutex`` opaque struct alias.
+
+.. cpp:function:: void pw_sync_Mutex_Lock(pw_sync_Mutex* mutex)
+
+  Invokes the ``Mutex::lock`` member function on the given ``mutex``.
+
+.. cpp:function:: bool pw_sync_Mutex_TryLock(pw_sync_Mutex* mutex)
+
+  Invokes the ``Mutex::try_lock`` member function on the given ``mutex``.
+
+.. cpp:function:: bool pw_sync_Mutex_TryLockFor(pw_sync_Mutex* mutex, pw_chrono_SystemClock_Duration for_at_least)
+
+  Invokes the ``Mutex::try_lock_for`` member function on the given ``mutex``.
+
+.. cpp:function:: bool pw_sync_Mutex_TryLockUntil(pw_sync_Mutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least)
+
+  Invokes the ``Mutex::try_lock_until`` member function on the given ``mutex``.
+
+.. cpp:function:: void pw_sync_Mutex_Unlock(pw_sync_Mutex* mutex)
+
+  Invokes the ``Mutex::unlock`` member function on the given ``mutex``.
+
++-------------------------------------+----------+-------------+-------+
+| *Safe to use in context*            | *Thread* | *Interrupt* | *NMI* |
++-------------------------------------+----------+-------------+-------+
+| ``void pw_sync_Mutex_Lock``         | ✔        |             |       |
++-------------------------------------+----------+-------------+-------+
+| ``bool pw_sync_Mutex_TryLock``      | ✔        |             |       |
++-------------------------------------+----------+-------------+-------+
+| ``bool pw_sync_Mutex_TryLockFor``   | ✔        |             |       |
++-------------------------------------+----------+-------------+-------+
+| ``bool pw_sync_Mutex_TryLockUntil`` | ✔        |             |       |
++-------------------------------------+----------+-------------+-------+
+| ``void pw_sync_Mutex_Unlock``       | ✔        |             |       |
++-------------------------------------+----------+-------------+-------+
+
+
+Example in C
+^^^^^^^^^^^^
+.. code-block:: cpp
+
+  #include "pw_chrono/system_clock.h"
+  #include "pw_sync/mutex.h"
+
+  pw::sync::Mutex mutex;
+
+  extern pw_sync_Mutex mutex;  // This can only be created in C++.
+
+  void ThreadSafeCriticalSection(void) {
+    pw_sync_Mutex_Lock(&mutex);
+    NotThreadSafeCriticalSection();
+    pw_sync_Mutex_Unlock(&mutex);
+  }
+
+  bool ThreadSafeCriticalSectionWithTimeout(
+      const pw_chrono_SystemClock_Duration timeout) {
+    if (!pw_sync_Mutex_TryLockFor(&mutex, timeout)) {
+      return false;
+    }
+    NotThreadSafeCriticalSection();
+    pw_sync_Mutex_Unlock(&mutex);
+    return true;
+  }
+
+
+InterruptSpinLock
+=================
+The InterruptSpinLock is a synchronization primitive that can be used to protect
+shared data from being simultaneously accessed by multiple threads and/or
+interrupts as a targeted global lock, with the exception of Non-Maskable
+Interrupts (NMIs). It offers exclusive, non-recursive ownership semantics where
+IRQs up to a backend defined level of "NMIs" will be masked to solve
+priority-inversion.
+
+This InterruptSpinLock relies on built-in local interrupt masking to make it
+interrupt safe without requiring the caller to separately mask and unmask
+interrupts when using this primitive.
+
+Unlike global interrupt locks, this also works safely and efficiently on SMP
+systems. On systems which are not SMP, spinning is not required but some state
+may still be used to detect recursion.
+
+The InterruptSpinLock is a
+`BasicLockable <https://en.cppreference.com/w/cpp/named_req/BasicLockable>`_
+and
+`Lockable <https://en.cppreference.com/w/cpp/named_req/Lockable>`_.
+
+
+C++
+---
+.. cpp:class:: pw::sync::InterruptSpinLock
+
+  .. cpp:function:: void lock()
+
+      Locks the spinlock, blocking indefinitely. Failures are fatal.
+
+      **Precondition:** Recursive locking is undefined behavior.
+
+  .. cpp:function:: bool try_lock()
+
+      Attempts to lock the spinlock in a non-blocking manner.
+      Returns true if the spinlock was successfully acquired.
+
+      **Precondition:** Recursive locking is undefined behavior.
+
+  .. cpp:function:: void unlock()
+
+     Unlocks the mutex. Failures are fatal.
+
+     **Precondition:** The spinlock is held by the caller.
+
+  +-------------------------------------------+----------+-------------+-------+
+  | *Safe to use in context*                  | *Thread* | *Interrupt* | *NMI* |
+  +-------------------------------------------+----------+-------------+-------+
+  | ``InterruptSpinLock::InterruptSpinLock``  | ✔        | ✔           |       |
+  +-------------------------------------------+----------+-------------+-------+
+  | ``InterruptSpinLock::~InterruptSpinLock`` | ✔        | ✔           |       |
+  +-------------------------------------------+----------+-------------+-------+
+  | ``void InterruptSpinLock::lock``          | ✔        | ✔           |       |
+  +-------------------------------------------+----------+-------------+-------+
+  | ``bool InterruptSpinLock::try_lock``      | ✔        | ✔           |       |
+  +-------------------------------------------+----------+-------------+-------+
+  | ``void InterruptSpinLock::unlock``        | ✔        | ✔           |       |
+  +-------------------------------------------+----------+-------------+-------+
+
+
+Examples in C++
+^^^^^^^^^^^^^^^
+.. code-block:: cpp
+
+  #include "pw_sync/interrupt_spin_lock.h"
+
+  pw::sync::InterruptSpinLock interrupt_spin_lock;
+
+  void InterruptSafeCriticalSection() {
+    interrupt_spin_lock.lock();
+    NotThreadSafeCriticalSection();
+    interrupt_spin_lock.unlock();
+  }
+
+
+Alternatively you can use C++'s RAII helpers to ensure you always unlock.
+
+.. code-block:: cpp
+
+  #include <mutex>
+
+  #include "pw_sync/interrupt_spin_lock.h"
+
+  pw::sync::InterruptSpinLock interrupt_spin_lock;
+
+  void InterruptSafeCriticalSection() {
+    std::lock_guard lock(interrupt_spin_lock);
+    NotThreadSafeCriticalSection();
+  }
+
+
+C
+-
+The InterruptSpinLock must be created in C++, however it can be passed into C using the
+``pw_sync_InterruptSpinLock`` opaque struct alias.
+
+.. cpp:function:: void pw_sync_InterruptSpinLock_Lock(pw_sync_InterruptSpinLock* interrupt_spin_lock)
+
+  Invokes the ``InterruptSpinLock::lock`` member function on the given ``interrupt_spin_lock``.
+
+.. cpp:function:: bool pw_sync_InterruptSpinLock_TryLock(pw_sync_InterruptSpinLock* interrupt_spin_lock)
+
+  Invokes the ``InterruptSpinLock::try_lock`` member function on the given ``interrupt_spin_lock``.
+
+.. cpp:function:: void pw_sync_InterruptSpinLock_Unlock(pw_sync_InterruptSpinLock* interrupt_spin_lock)
+
+  Invokes the ``InterruptSpinLock::unlock`` member function on the given ``interrupt_spin_lock``.
+
++--------------------------------------------+----------+-------------+-------+
+| *Safe to use in context*                   | *Thread* | *Interrupt* | *NMI* |
++--------------------------------------------+----------+-------------+-------+
+| ``void pw_sync_InterruptSpinLock_Lock``    | ✔        | ✔           |       |
++--------------------------------------------+----------+-------------+-------+
+| ``bool pw_sync_InterruptSpinLock_TryLock`` | ✔        | ✔           |       |
++--------------------------------------------+----------+-------------+-------+
+| ``void pw_sync_InterruptSpinLock_Unlock``  | ✔        | ✔           |       |
++--------------------------------------------+----------+-------------+-------+
+
+
+Example in C
+^^^^^^^^^^^^
+.. code-block:: cpp
+
+  #include "pw_chrono/system_clock.h"
+  #include "pw_sync/interrupt_spin_lock.h"
+
+  pw::sync::InterruptSpinLock interrupt_spin_lock;
+
+  extern pw_sync_InterruptSpinLock interrupt_spin_lock;  // This can only be created in C++.
+
+  void InterruptSafeCriticalSection(void) {
+    pw_sync_InterruptSpinLock_Lock(&interrupt_spin_lock);
+    NotThreadSafeCriticalSection();
+    pw_sync_InterruptSpinLock_Unlock(&interrupt_spin_lock);
+  }
+
+
+--------------------
+Signaling Primitives
+--------------------
+
+CountingSemaphore
+=================
+The CountingSemaphore is a synchronization primitive that can be used for
+counting events and/or resource management where receiver(s) can block on
+acquire until notifier(s) signal by invoking release.
+
+Note that unlike Mutexes, priority inheritance is not used by semaphores meaning
+semaphores are subject to unbounded priority inversions. Due to this, Pigweed
+does not recommend semaphores for mutual exclusion.
+
+The CountingSemaphore is initialized to being empty or having no tokens.
+
+The entire API is thread safe, but only a subset is interrupt safe. None of it
+is NMI safe.
+
+.. Warning::
+  Releasing multiple tokens is often not natively supported, meaning you may
+  end up invoking the native kernel API many times, i.e. once per token you
+  are releasing!
+
+BinarySemaphore
+===============
+BinarySemaphore is a specialization of CountingSemaphore with an arbitrary token
+limit of 1. Note that that ``max()`` is >= 1, meaning it may be released up to
+``max()`` times but only acquired once for those N releases.
+
+Implementations of BinarySemaphore are typically more efficient than the
+default implementation of CountingSemaphore.
+
+The BinarySemaphore is initialized to being empty or having no tokens.
+
+The entire API is thread safe, but only a subset is interrupt safe. None of it
+is NMI safe.
diff --git a/pw_sync/public/pw_sync/binary_semaphore.h b/pw_sync/public/pw_sync/binary_semaphore.h
index ecb3e31..b7e1c99 100644
--- a/pw_sync/public/pw_sync/binary_semaphore.h
+++ b/pw_sync/public/pw_sync/binary_semaphore.h
@@ -25,7 +25,7 @@
 
 namespace pw::sync {
 
-// BinarySemaphore is a specialization of CountingSemaphore with arbitrary
+// BinarySemaphore is a specialization of CountingSemaphore with an arbitrary
 // token limit of 1. Note that that max() is >= 1, meaning it may be
 // released up to max() times but only acquired once for those N releases.
 // Implementations of BinarySemaphore are typically more efficient than the
@@ -33,10 +33,9 @@
 // but only a subset is IRQ safe.
 //
 // WARNING: In order to support global statically constructed BinarySemaphores,
-// the backend MUST ensure that any initialization required in your environment
-// prior to the creation and/or initialization of the native semaphore
-// (e.g. kernel initialization), is done before or during the invocation of the
-// global static C++ constructors.
+// the user and/or backend MUST ensure that any initialization required in your
+// environment is done prior to the creation and/or initialization of the native
+// synchronization primitives (e.g. kernel initialization).
 class BinarySemaphore {
  public:
   using native_handle_type = backend::NativeBinarySemaphoreHandle;
diff --git a/pw_sync/public/pw_sync/counting_semaphore.h b/pw_sync/public/pw_sync/counting_semaphore.h
index cf2e904..23fbb21 100644
--- a/pw_sync/public/pw_sync/counting_semaphore.h
+++ b/pw_sync/public/pw_sync/counting_semaphore.h
@@ -33,11 +33,10 @@
 // Pigweed does not recommend semaphores for mutual exclusion. The entire API is
 // thread safe but only a subset is IRQ safe.
 //
-// WARNING: In order to support global statically constructed
-// CountingSemaphores, the backend MUST ensure that any initialization required
-// in your environment prior to the creation and/or initialization of the native
-// semaphore (e.g. kernel initialization), is done before or during the
-// invocation of the global static C++ constructors.
+// WARNING: In order to support global statically constructed CountingSemaphores
+// the user and/or backend MUST ensure that any initialization required in your
+// environment is done prior to the creation and/or initialization of the native
+// synchronization primitives (e.g. kernel initialization).
 class CountingSemaphore {
  public:
   using native_handle_type = backend::NativeCountingSemaphoreHandle;
diff --git a/pw_sync/public/pw_sync/interrupt_spin_lock.h b/pw_sync/public/pw_sync/interrupt_spin_lock.h
index e42f60e..f03c8d8 100644
--- a/pw_sync/public/pw_sync/interrupt_spin_lock.h
+++ b/pw_sync/public/pw_sync/interrupt_spin_lock.h
@@ -25,7 +25,8 @@
 
 // The InterruptSpinLock is a synchronization primitive that can be used to
 // protect shared data from being simultaneously accessed by multiple threads
-// and/or interrupts as a targeted global lock (except for NMIs).
+// and/or interrupts as a targeted global lock, with the exception of
+// Non-Maskable Interrupts (NMIs).
 // It offers exclusive, non-recursive ownership semantics where IRQs up to a
 // backend defined level of "NMIs" will be masked to solve priority-inversion.
 //
@@ -34,7 +35,11 @@
 // unmask interrupts when using this primitive.
 //
 // Unlike global interrupt locks, this also works safely and efficiently on SMP
-// systems. This entire API is IRQ safe.
+// systems. On systems which are not SMP, spinning is not required and it's
+// possible that only interrupt masking occurs but some state may still be used
+// to detect recursion.
+//
+// This entire API is IRQ safe, but NOT NMI safe.
 //
 // Precondition: Code that holds a specific InterruptSpinLock must not try to
 // re-acquire it. However, it is okay to nest distinct spinlocks.
@@ -61,6 +66,9 @@
   bool try_lock();
 
   // Unlocks the spinlock. Failures are fatal.
+  //
+  // PRECONDITION:
+  //   The spinlock is held by the caller.
   void unlock();
 
   native_handle_type native_handle();
diff --git a/pw_sync/public/pw_sync/mutex.h b/pw_sync/public/pw_sync/mutex.h
index 1bc77d7..c451581 100644
--- a/pw_sync/public/pw_sync/mutex.h
+++ b/pw_sync/public/pw_sync/mutex.h
@@ -30,11 +30,10 @@
 // inheritance is used to solve the classic priority-inversion problem.
 // This is thread safe, but NOT IRQ safe.
 //
-// WARNING: In order to support global statically constructed Mutex, the backend
-// MUST ensure that any initialization required in your environment prior to the
-// creation and/or initialization of the native semaphore (e.g. kernel
-// initialization), is done before or during the invocation of the global static
-// C++ constructors.
+// WARNING: In order to support global statically constructed Mutexes, the user
+// and/or backend MUST ensure that any initialization required in your
+// environment is done prior to the creation and/or initialization of the native
+// synchronization primitives (e.g. kernel initialization).
 class Mutex {
  public:
   using native_handle_type = backend::NativeMutexHandle;
@@ -82,7 +81,7 @@
   // Unlocks the mutex. Failures are fatal.
   //
   // PRECONDITION:
-  //   The lock is held by this thread.
+  //   The mutex is held by this thread.
   void unlock();
 
   native_handle_type native_handle();