diff options
author | Ewout van Bekkum <ewout@google.com> | 2021-04-06 16:15:22 -0700 |
---|---|---|
committer | CQ Bot Account <pigweed-scoped@luci-project-accounts.iam.gserviceaccount.com> | 2021-04-09 18:05:44 +0000 |
commit | 6f5b8fb1ff9a8b2e025863187c7ec7c46fa74929 (patch) | |
tree | 71bef6168f6f05a57f3536e447a33a90c8098e7e | |
parent | ecbabcc401c9c818008663424f25fd925b077ba6 (diff) | |
download | pigweed-6f5b8fb1ff9a8b2e025863187c7ec7c46fa74929.tar.gz |
pw_sync: split out pw::sync::TimedMutex from pw::sync::Mutex
Change-Id: I6a52123759045658cc21aaa3254279d95c23ed6e
Requires: pigweed-internal:11280
Reviewed-on: https://pigweed-review.googlesource.com/c/pigweed/pigweed/+/40083
Reviewed-by: Wyatt Hepler <hepler@google.com>
Commit-Queue: Auto-Submit <auto-submit@pigweed.google.com.iam.gserviceaccount.com>
Pigweed-Auto-Submit: Ewout van Bekkum <ewout@google.com>
36 files changed, 1013 insertions, 280 deletions
diff --git a/pw_sync/BUILD b/pw_sync/BUILD index b882e479a..f1c6620f3 100644 --- a/pw_sync/BUILD +++ b/pw_sync/BUILD @@ -26,6 +26,7 @@ licenses(["notice"]) # Apache License 2.0 PW_SYNC_BINARY_SEMAPHORE_BACKEND = "//pw_sync_stl:binary_semaphore" PW_SYNC_COUNTING_SEMAPHORE_BACKEND = "//pw_sync_stl:counting_semaphore" PW_SYNC_MUTEX_BACKEND = "//pw_sync_stl:mutex" +PW_SYNC_TIMED_MUTEX_BACKEND = "//pw_sync_stl:timed_mutex" PW_SYNC_INTERRUPT_SPIN_LOCK_BACKEND = "//pw_sync_stl:interrupt_spin_lock" pw_cc_library( @@ -101,7 +102,6 @@ pw_cc_library( ], deps = [ PW_SYNC_MUTEX_BACKEND + "_headers", - "//pw_chrono:system_clock", "//pw_preprocessor", ], ) @@ -122,6 +122,38 @@ pw_cc_library( ) pw_cc_library( + name = "timed_mutex_facade", + hdrs = [ + "public/pw_sync/timed_mutex.h", + ], + includes = ["public"], + srcs = [ + "timed_mutex.cc" + ], + deps = [ + PW_SYNC_TIMED_MUTEX_BACKEND + "_headers", + "//pw_chrono:system_clock", + ":mutex_facade", + "//pw_preprocessor", + ], +) + +pw_cc_library( + name = "timed_mutex", + deps = [ + ":timed_mutex_facade", + PW_SYNC_TIMED_MUTEX_BACKEND + "_headers", + ], +) + +pw_cc_library( + name = "timed_mutex_backend", + deps = [ + PW_SYNC_TIMED_MUTEX_BACKEND, + ], +) + +pw_cc_library( name = "interrupt_spin_lock_facade", hdrs = [ "public/pw_sync/interrupt_spin_lock.h", @@ -199,6 +231,19 @@ pw_cc_test( ) pw_cc_test( + name = "timed_mutex_facade_test", + srcs = [ + "timed_mutex_facade_test.cc", + "timed_mutex_facade_test_c.c", + ], + deps = [ + ":timed_mutex", + "//pw_preprocessor", + "//pw_unit_test", + ], +) + +pw_cc_test( name = "interrupt_spin_lock_facade_test", srcs = [ "interrupt_spin_lock_facade_test.cc", diff --git a/pw_sync/BUILD.gn b/pw_sync/BUILD.gn index eeaf60ebe..673f80b12 100644 --- a/pw_sync/BUILD.gn +++ b/pw_sync/BUILD.gn @@ -51,11 +51,20 @@ pw_facade("mutex") { backend = pw_sync_MUTEX_BACKEND public_configs = [ ":public_include_path" ] public = [ "public/pw_sync/mutex.h" ] + public_deps = [ "$dir_pw_preprocessor" ] + sources = [ "mutex.cc" ] +} + +pw_facade("timed_mutex") { + backend = pw_sync_TIMED_MUTEX_BACKEND + public_configs = [ ":public_include_path" ] + public = [ "public/pw_sync/timed_mutex.h" ] public_deps = [ + ":mutex", "$dir_pw_chrono:system_clock", "$dir_pw_preprocessor", ] - sources = [ "mutex.cc" ] + sources = [ "timed_mutex.cc" ] } pw_facade("interrupt_spin_lock") { @@ -76,6 +85,7 @@ pw_test_group("tests") { ":binary_semaphore_facade_test", ":counting_semaphore_facade_test", ":mutex_facade_test", + ":timed_mutex_facade_test", ":interrupt_spin_lock_facade_test", ] } @@ -119,6 +129,19 @@ pw_test("mutex_facade_test") { ] } +pw_test("timed_mutex_facade_test") { + enable_if = pw_sync_TIMED_MUTEX_BACKEND != "" + sources = [ + "timed_mutex_facade_test.cc", + "timed_mutex_facade_test_c.c", + ] + deps = [ + ":timed_mutex", + "$dir_pw_preprocessor", + pw_sync_TIMED_MUTEX_BACKEND, + ] +} + pw_test("interrupt_spin_lock_facade_test") { enable_if = pw_sync_INTERRUPT_SPIN_LOCK_BACKEND != "" sources = [ diff --git a/pw_sync/backend.gni b/pw_sync/backend.gni index 21091eb6a..8f6605a58 100644 --- a/pw_sync/backend.gni +++ b/pw_sync/backend.gni @@ -22,6 +22,9 @@ declare_args() { # Backend for the pw_sync module's mutex. pw_sync_MUTEX_BACKEND = "" + # Backend for the pw_sync module's timed mutex. + pw_sync_TIMED_MUTEX_BACKEND = "" + # Backend for the pw_sync module's interrupt spin lock. pw_sync_INTERRUPT_SPIN_LOCK_BACKEND = "" diff --git a/pw_sync/docs.rst b/pw_sync/docs.rst index aec5900e6..de5a125aa 100644 --- a/pw_sync/docs.rst +++ b/pw_sync/docs.rst @@ -45,15 +45,183 @@ 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::mutex <https://en.cppreference.com/w/cpp/thread/mutex>`_ like, +meaning it is a +`BasicLockable <https://en.cppreference.com/w/cpp/named_req/BasicLockable>`_ +and `Lockable <https://en.cppreference.com/w/cpp/named_req/Lockable>`_. + +.. list-table:: + + * - *Supported on* + - *Backend module* + * - FreeRTOS + - :ref:`module-pw_sync_freertos` + * - ThreadX + - :ref:`module-pw_sync_threadx` + * - embOS + - :ref:`module-pw_sync_embos` + * - STL + - :ref:`module-pw_sync_stl` + * - Baremetal + - Planned + * - Zephyr + - Planned + * - CMSIS-RTOS API v2 & RTX5 + - Planned + +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:: void unlock() + + Unlocks the mutex. Failures are fatal. + + **Precondition:** The mutex is held by this thread. + + + .. list-table:: + + * - *Safe to use in context* + - *Thread* + - *Interrupt* + - *NMI* + * - ``Mutex::Mutex`` + - ✔ + - + - + * - ``Mutex::~Mutex`` + - ✔ + - + - + * - ``void Mutex::lock`` + - ✔ + - + - + * - ``bool Mutex::try_lock`` + - ✔ + - + - + * - ``void Mutex::unlock`` + - ✔ + - + - + +Examples in C++ +^^^^^^^^^^^^^^^ +.. code-block:: cpp + + #include "pw_sync/mutex.h" + + pw::sync::Mutex mutex; + + void ThreadSafeCriticalSection() { + mutex.lock(); + NotThreadSafeCriticalSection(); + mutex.unlock(); + } + + +Alternatively you can use C++'s RAII helpers to ensure you always unlock. + +.. code-block:: cpp + + #include <mutex> + + #include "pw_sync/mutex.h" + + pw::sync::Mutex mutex; + + void ThreadSafeCriticalSection() { + std::lock_guard lock(mutex); + NotThreadSafeCriticalSection(); + } + + +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:: void pw_sync_Mutex_Unlock(pw_sync_Mutex* mutex) + + Invokes the ``Mutex::unlock`` member function on the given ``mutex``. + +.. list-table:: + + * - *Safe to use in context* + - *Thread* + - *Interrupt* + - *NMI* + * - ``void pw_sync_Mutex_Lock`` + - ✔ + - + - + * - ``bool pw_sync_Mutex_TryLock`` + - ✔ + - + - + * - ``void pw_sync_Mutex_Unlock`` + - ✔ + - + - + +Example in C +^^^^^^^^^^^^ +.. code-block:: cpp + + #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); + } + +TimedMutex +========== +The TimedMutex is an extension of the Mutex which offers timeout and deadline +based semantics. + +The TimedMutex'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>`_. -.. Warning:: - This interface will likely be split between a Mutex and TimedMutex to ease - portability for baremetal support. +Note that the ``TimedMutex`` is a derived ``Mutex`` class, meaning that +a ``TimedMutex`` can be used by someone who needs the basic ``Mutex``. This is +in stark contrast to the C++ STL's +`std::timed_mutex <https://en.cppreference.com/w/cpp/thread/timed_mutex>`_. + .. list-table:: @@ -67,8 +235,6 @@ meaning it is a - :ref:`module-pw_sync_embos` * - STL - :ref:`module-pw_sync_stl` - * - Baremetal - - Planned * - Zephyr - Planned * - CMSIS-RTOS API v2 & RTX5 @@ -76,7 +242,7 @@ meaning it is a C++ --- -.. cpp:class:: pw::sync::Mutex +.. cpp:class:: pw::sync::TimedMutex .. cpp:function:: void lock() @@ -124,31 +290,31 @@ C++ - *Thread* - *Interrupt* - *NMI* - * - ``Mutex::Mutex`` + * - ``TimedMutex::TimedMutex`` - ✔ - - - * - ``Mutex::~Mutex`` + * - ``TimedMutex::~TimedMutex`` - ✔ - - - * - ``void Mutex::lock`` + * - ``void TimedMutex::lock`` - ✔ - - - * - ``bool Mutex::try_lock`` + * - ``bool TimedMutex::try_lock`` - ✔ - - - * - ``bool Mutex::try_lock_for`` + * - ``bool TimedMutex::try_lock_for`` - ✔ - - - * - ``bool Mutex::try_lock_until`` + * - ``bool TimedMutex::try_lock_until`` - ✔ - - - * - ``void Mutex::unlock`` + * - ``void TimedMutex::unlock`` - ✔ - - @@ -158,15 +324,9 @@ Examples in C++ .. code-block:: cpp #include "pw_chrono/system_clock.h" - #include "pw_sync/mutex.h" + #include "pw_sync/timed_mutex.h" - pw::sync::Mutex mutex; - - void ThreadSafeCriticalSection() { - mutex.lock(); - NotThreadSafeCriticalSection(); - mutex.unlock(); - } + pw::sync::TimedMutex mutex; bool ThreadSafeCriticalSectionWithTimeout( const SystemClock::duration timeout) { @@ -186,14 +346,9 @@ Alternatively you can use C++'s RAII helpers to ensure you always unlock. #include <mutex> #include "pw_chrono/system_clock.h" - #include "pw_sync/mutex.h" + #include "pw_sync/timed_mutex.h" - pw::sync::Mutex mutex; - - void ThreadSafeCriticalSection() { - std::lock_guard lock(mutex); - NotThreadSafeCriticalSection(); - } + pw::sync::TimedMutex mutex; bool ThreadSafeCriticalSectionWithTimeout( const SystemClock::duration timeout) { @@ -209,28 +364,28 @@ Alternatively you can use C++'s RAII helpers to ensure you always unlock. C - -The Mutex must be created in C++, however it can be passed into C using the -``pw_sync_Mutex`` opaque struct alias. +The TimedMutex must be created in C++, however it can be passed into C using the +``pw_sync_TimedMutex`` opaque struct alias. -.. cpp:function:: void pw_sync_Mutex_Lock(pw_sync_Mutex* mutex) +.. cpp:function:: void pw_sync_TimedMutex_Lock(pw_sync_TimedMutex* mutex) - Invokes the ``Mutex::lock`` member function on the given ``mutex``. + Invokes the ``TimedMutex::lock`` member function on the given ``mutex``. -.. cpp:function:: bool pw_sync_Mutex_TryLock(pw_sync_Mutex* mutex) +.. cpp:function:: bool pw_sync_TimedMutex_TryLock(pw_sync_TimedMutex* mutex) - Invokes the ``Mutex::try_lock`` member function on the given ``mutex``. + Invokes the ``TimedMutex::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) +.. cpp:function:: bool pw_sync_TimedMutex_TryLockFor(pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_Duration for_at_least) - Invokes the ``Mutex::try_lock_for`` member function on the given ``mutex``. + Invokes the ``TimedMutex::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) +.. cpp:function:: bool pw_sync_TimedMutex_TryLockUntil(pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least) - Invokes the ``Mutex::try_lock_until`` member function on the given ``mutex``. + Invokes the ``TimedMutex::try_lock_until`` member function on the given ``mutex``. -.. cpp:function:: void pw_sync_Mutex_Unlock(pw_sync_Mutex* mutex) +.. cpp:function:: void pw_sync_TimedMutex_Unlock(pw_sync_TimedMutex* mutex) - Invokes the ``Mutex::unlock`` member function on the given ``mutex``. + Invokes the ``TimedMutex::unlock`` member function on the given ``mutex``. .. list-table:: @@ -238,23 +393,23 @@ The Mutex must be created in C++, however it can be passed into C using the - *Thread* - *Interrupt* - *NMI* - * - ``void pw_sync_Mutex_Lock`` + * - ``void pw_sync_TimedMutex_Lock`` - ✔ - - - * - ``bool pw_sync_Mutex_TryLock`` + * - ``bool pw_sync_TimedMutex_TryLock`` - ✔ - - - * - ``bool pw_sync_Mutex_TryLockFor`` + * - ``bool pw_sync_TimedMutex_TryLockFor`` - ✔ - - - * - ``bool pw_sync_Mutex_TryLockUntil`` + * - ``bool pw_sync_TimedMutex_TryLockUntil`` - ✔ - - - * - ``void pw_sync_Mutex_Unlock`` + * - ``void pw_sync_TimedMutex_Unlock`` - ✔ - - @@ -264,25 +419,19 @@ Example in C .. code-block:: cpp #include "pw_chrono/system_clock.h" - #include "pw_sync/mutex.h" + #include "pw_sync/timed_mutex.h" - pw::sync::Mutex mutex; - - extern pw_sync_Mutex mutex; // This can only be created in C++. + pw::sync::TimedMutex mutex; - void ThreadSafeCriticalSection(void) { - pw_sync_Mutex_Lock(&mutex); - NotThreadSafeCriticalSection(); - pw_sync_Mutex_Unlock(&mutex); - } + extern pw_sync_TimedMutex mutex; // This can only be created in C++. bool ThreadSafeCriticalSectionWithTimeout( const pw_chrono_SystemClock_Duration timeout) { - if (!pw_sync_Mutex_TryLockFor(&mutex, timeout)) { + if (!pw_sync_TimedMutex_TryLockFor(&mutex, timeout)) { return false; } NotThreadSafeCriticalSection(); - pw_sync_Mutex_Unlock(&mutex); + pw_sync_TimedMutex_Unlock(&mutex); return true; } diff --git a/pw_sync/mutex.cc b/pw_sync/mutex.cc index 416182ed9..1932c5acc 100644 --- a/pw_sync/mutex.cc +++ b/pw_sync/mutex.cc @@ -14,22 +14,10 @@ #include "pw_sync/mutex.h" -using pw::chrono::SystemClock; - extern "C" void pw_sync_Mutex_Lock(pw_sync_Mutex* mutex) { mutex->lock(); } extern "C" bool pw_sync_Mutex_TryLock(pw_sync_Mutex* mutex) { return mutex->try_lock(); } -extern "C" bool pw_sync_Mutex_TryLockFor( - pw_sync_Mutex* mutex, pw_chrono_SystemClock_Duration for_at_least) { - return mutex->try_lock_for(SystemClock::duration(for_at_least.ticks)); -} - -extern "C" bool pw_sync_Mutex_TryLockUntil( - pw_sync_Mutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least) { - return mutex->try_lock_until(SystemClock::time_point( - SystemClock::duration(until_at_least.duration_since_epoch.ticks))); -} extern "C" void pw_sync_Mutex_Unlock(pw_sync_Mutex* mutex) { mutex->unlock(); } diff --git a/pw_sync/mutex_facade_test.cc b/pw_sync/mutex_facade_test.cc index ca47cfe03..0fc720322 100644 --- a/pw_sync/mutex_facade_test.cc +++ b/pw_sync/mutex_facade_test.cc @@ -15,12 +15,8 @@ #include <chrono> #include "gtest/gtest.h" -#include "pw_chrono/system_clock.h" #include "pw_sync/mutex.h" -using pw::chrono::SystemClock; -using namespace std::chrono_literals; - namespace pw::sync { namespace { @@ -29,22 +25,10 @@ extern "C" { // Functions defined in mutex_facade_test_c.c which call the API from C. void pw_sync_Mutex_CallLock(pw_sync_Mutex* mutex); bool pw_sync_Mutex_CallTryLock(pw_sync_Mutex* mutex); -bool pw_sync_Mutex_CallTryLockFor(pw_sync_Mutex* mutex, - pw_chrono_SystemClock_Duration for_at_least); -bool pw_sync_Mutex_CallTryLockUntil( - pw_sync_Mutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least); void pw_sync_Mutex_CallUnlock(pw_sync_Mutex* mutex); } // extern "C" -// We can't control the SystemClock's period configuration, so just in case -// duration cannot be accurately expressed in integer ticks, round the -// duration up. -constexpr SystemClock::duration kRoundedArbitraryDuration = - SystemClock::for_at_least(42ms); -constexpr pw_chrono_SystemClock_Duration kRoundedArbitraryDurationInC = - PW_SYSTEM_CLOCK_MS(42); - // TODO(pwbug/291): Add real concurrency tests once we have pw::thread. TEST(Mutex, LockUnlock) { @@ -71,39 +55,6 @@ TEST(Mutex, TryLockUnlock) { mutex.unlock(); } -TEST(Mutex, TryLockUnlockFor) { - pw::sync::Mutex mutex; - - SystemClock::time_point before = SystemClock::now(); - ASSERT_TRUE(mutex.try_lock_for(kRoundedArbitraryDuration)); - SystemClock::duration time_elapsed = SystemClock::now() - before; - EXPECT_LT(time_elapsed, kRoundedArbitraryDuration); - - // TODO(pwbug/291): Ensure it blocks fails to lock when already held. - // before = SystemClock::now(); - // EXPECT_FALSE(mutex.try_lock_for(kRoundedArbitraryDuration)); - // time_elapsed = SystemClock::now() - before; - /// EXPECT_GE(time_elapsed, kRoundedArbitraryDuration); - - mutex.unlock(); -} - -TEST(Mutex, TryLockUnlockUntil) { - pw::sync::Mutex mutex; - - const SystemClock::time_point deadline = - SystemClock::now() + kRoundedArbitraryDuration; - ASSERT_TRUE(mutex.try_lock_until(deadline)); - EXPECT_LT(SystemClock::now(), deadline); - - // TODO(pwbug/291): Ensure it blocks fails to lock when already held. - // EXPECT_FALSE( - // mutex.try_lock_until(SystemClock::now() + kRoundedArbitraryDuration)); - // EXPECT_GE(SystemClock::now(), deadline); - - mutex.unlock(); -} - TEST(Mutex, LockUnlockInC) { pw::sync::Mutex mutex; pw_sync_Mutex_CallLock(&mutex); @@ -118,44 +69,5 @@ TEST(Mutex, TryLockUnlockInC) { pw_sync_Mutex_CallUnlock(&mutex); } -TEST(Mutex, TryLockUnlockForInC) { - pw::sync::Mutex mutex; - - pw_chrono_SystemClock_TimePoint before = pw_chrono_SystemClock_Now(); - ASSERT_TRUE( - pw_sync_Mutex_CallTryLockFor(&mutex, kRoundedArbitraryDurationInC)); - pw_chrono_SystemClock_Duration time_elapsed = - pw_chrono_SystemClock_TimeElapsed(before, pw_chrono_SystemClock_Now()); - EXPECT_LT(time_elapsed.ticks, kRoundedArbitraryDurationInC.ticks); - - // TODO(pwbug/291): Ensure it blocks fails to lock when already held. - // before = pw_chrono_SystemClock_Now(); - // EXPECT_FALSE( - // pw_sync_Mutex_CallTryLockFor(&mutex, kRoundedArbitraryDurationInC)); - // time_elapsed = - // pw_chrono_SystemClock_TimeElapsed(before, pw_chrono_SystemClock_Now()); - // EXPECT_GE(time_elapsed.ticks, kRoundedArbitraryDurationInC.ticks); - - pw_sync_Mutex_CallUnlock(&mutex); -} - -TEST(Mutex, TryLockUnlockUntilInC) { - pw::sync::Mutex mutex; - pw_chrono_SystemClock_TimePoint deadline; - deadline.duration_since_epoch.ticks = - pw_chrono_SystemClock_Now().duration_since_epoch.ticks + - kRoundedArbitraryDurationInC.ticks; - ASSERT_TRUE(pw_sync_Mutex_CallTryLockUntil(&mutex, deadline)); - EXPECT_LT(pw_chrono_SystemClock_Now().duration_since_epoch.ticks, - deadline.duration_since_epoch.ticks); - - // TODO(pwbug/291): Ensure it blocks fails to lock when already held. - // EXPECT_FALSE(pw_sync_Mutex_CallTryLockUntil(&mutex, deadline)); - // EXPECT_GE(pw_chrono_SystemClock_Now().duration_since_epoch.ticks, - // deadline.duration_since_epoch.ticks); - - mutex.unlock(); -} - } // namespace } // namespace pw::sync diff --git a/pw_sync/mutex_facade_test_c.c b/pw_sync/mutex_facade_test_c.c index 5b1e631e9..5196d511e 100644 --- a/pw_sync/mutex_facade_test_c.c +++ b/pw_sync/mutex_facade_test_c.c @@ -25,16 +25,6 @@ bool pw_sync_Mutex_CallTryLock(pw_sync_Mutex* mutex) { return pw_sync_Mutex_TryLock(mutex); } -bool pw_sync_Mutex_CallTryLockFor(pw_sync_Mutex* mutex, - pw_chrono_SystemClock_Duration for_at_least) { - return pw_sync_Mutex_TryLockFor(mutex, for_at_least); -} - -bool pw_sync_Mutex_CallTryLockUntil( - pw_sync_Mutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least) { - return pw_sync_Mutex_TryLockUntil(mutex, until_at_least); -} - void pw_sync_Mutex_CallUnlock(pw_sync_Mutex* mutex) { pw_sync_Mutex_Unlock(mutex); } diff --git a/pw_sync/public/pw_sync/mutex.h b/pw_sync/public/pw_sync/mutex.h index c45158178..ef4f22014 100644 --- a/pw_sync/public/pw_sync/mutex.h +++ b/pw_sync/public/pw_sync/mutex.h @@ -15,7 +15,6 @@ #include <stdbool.h> -#include "pw_chrono/system_clock.h" #include "pw_preprocessor/util.h" #ifdef __cplusplus @@ -60,24 +59,6 @@ class Mutex { // undefined behavior. bool try_lock(); - // 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. - bool try_lock_for(chrono::SystemClock::duration for_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. - bool try_lock_until(chrono::SystemClock::time_point until_at_least); - // Unlocks the mutex. Failures are fatal. // // PRECONDITION: @@ -107,10 +88,6 @@ PW_EXTERN_C_START void pw_sync_Mutex_Lock(pw_sync_Mutex* mutex); bool pw_sync_Mutex_TryLock(pw_sync_Mutex* mutex); -bool pw_sync_Mutex_TryLockFor(pw_sync_Mutex* mutex, - pw_chrono_SystemClock_Duration for_at_least); -bool pw_sync_Mutex_TryLockUntil(pw_sync_Mutex* mutex, - pw_chrono_SystemClock_TimePoint until_at_least); void pw_sync_Mutex_Unlock(pw_sync_Mutex* mutex); PW_EXTERN_C_END diff --git a/pw_sync/public/pw_sync/timed_mutex.h b/pw_sync/public/pw_sync/timed_mutex.h new file mode 100644 index 000000000..7774c8fad --- /dev/null +++ b/pw_sync/public/pw_sync/timed_mutex.h @@ -0,0 +1,87 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include <stdbool.h> + +#include "pw_chrono/system_clock.h" +#include "pw_preprocessor/util.h" +#include "pw_sync/mutex.h" + +#ifdef __cplusplus + +namespace pw::sync { + +// The TimedMutex is a synchronization primitive that can be used to protect +// shared data from being simultaneously accessed by multiple threads with +// timeouts and deadlines, extending the Mutex. +// It offers exclusive, non-recursive ownership semantics where priority +// 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 TimedMutexes, 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 TimedMutex : public Mutex { + public: + TimedMutex() = default; + ~TimedMutex() = default; + TimedMutex(const TimedMutex&) = delete; + TimedMutex(TimedMutex&&) = delete; + TimedMutex& operator=(const TimedMutex&) = delete; + TimedMutex& operator=(TimedMutex&&) = delete; + + // 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. + bool try_lock_for(chrono::SystemClock::duration for_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. + bool try_lock_until(chrono::SystemClock::time_point until_at_least); +}; + +} // namespace pw::sync + +#include "pw_sync_backend/timed_mutex_inline.h" + +using pw_sync_TimedMutex = pw::sync::TimedMutex; + +#else // !defined(__cplusplus) + +typedef struct pw_sync_TimedMutex pw_sync_TimedMutex; + +#endif // __cplusplus + +PW_EXTERN_C_START + +void pw_sync_TimedMutex_Lock(pw_sync_TimedMutex* mutex); +bool pw_sync_TimedMutex_TryLock(pw_sync_TimedMutex* mutex); +bool pw_sync_TimedMutex_TryLockFor(pw_sync_TimedMutex* mutex, + pw_chrono_SystemClock_Duration for_at_least); +bool pw_sync_TimedMutex_TryLockUntil( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least); +void pw_sync_TimedMutex_Unlock(pw_sync_TimedMutex* mutex); + +PW_EXTERN_C_END diff --git a/pw_sync/timed_mutex.cc b/pw_sync/timed_mutex.cc new file mode 100644 index 000000000..512d4c4c5 --- /dev/null +++ b/pw_sync/timed_mutex.cc @@ -0,0 +1,40 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +#include "pw_sync/timed_mutex.h" + +using pw::chrono::SystemClock; + +extern "C" void pw_sync_TimedMutex_Lock(pw_sync_TimedMutex* mutex) { + mutex->lock(); +} + +extern "C" bool pw_sync_TimedMutex_TryLock(pw_sync_TimedMutex* mutex) { + return mutex->try_lock(); +} + +extern "C" bool pw_sync_TimedMutex_TryLockFor( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_Duration for_at_least) { + return mutex->try_lock_for(SystemClock::duration(for_at_least.ticks)); +} + +extern "C" bool pw_sync_TimedMutex_TryLockUntil( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least) { + return mutex->try_lock_until(SystemClock::time_point( + SystemClock::duration(until_at_least.duration_since_epoch.ticks))); +} + +extern "C" void pw_sync_TimedMutex_Unlock(pw_sync_TimedMutex* mutex) { + mutex->unlock(); +} diff --git a/pw_sync/timed_mutex_facade_test.cc b/pw_sync/timed_mutex_facade_test.cc new file mode 100644 index 000000000..7b81d990e --- /dev/null +++ b/pw_sync/timed_mutex_facade_test.cc @@ -0,0 +1,162 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +#include <chrono> + +#include "gtest/gtest.h" +#include "pw_chrono/system_clock.h" +#include "pw_sync/timed_mutex.h" + +using pw::chrono::SystemClock; +using namespace std::chrono_literals; + +namespace pw::sync { +namespace { + +extern "C" { + +// Functions defined in mutex_facade_test_c.c which call the API from C. +void pw_sync_TimedMutex_CallLock(pw_sync_TimedMutex* mutex); +bool pw_sync_TimedMutex_CallTryLock(pw_sync_TimedMutex* mutex); +bool pw_sync_TimedMutex_CallTryLockFor( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_Duration for_at_least); +bool pw_sync_TimedMutex_CallTryLockUntil( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least); +void pw_sync_TimedMutex_CallUnlock(pw_sync_TimedMutex* mutex); + +} // extern "C" + +// We can't control the SystemClock's period configuration, so just in case +// duration cannot be accurately expressed in integer ticks, round the +// duration up. +constexpr SystemClock::duration kRoundedArbitraryDuration = + SystemClock::for_at_least(42ms); +constexpr pw_chrono_SystemClock_Duration kRoundedArbitraryDurationInC = + PW_SYSTEM_CLOCK_MS(42); + +// TODO(pwbug/291): Add real concurrency tests once we have pw::thread. + +TEST(TimedMutex, LockUnlock) { + pw::sync::TimedMutex mutex; + mutex.lock(); + // TODO(pwbug/291): Ensure it fails to lock when already held. + // EXPECT_FALSE(mutex.try_lock()); + mutex.unlock(); +} + +TimedMutex static_mutex; +TEST(TimedMutex, LockUnlockStatic) { + static_mutex.lock(); + // TODO(pwbug/291): Ensure it fails to lock when already held. + // EXPECT_FALSE(static_mutex.try_lock()); + static_mutex.unlock(); +} + +TEST(TimedMutex, TryLockUnlock) { + pw::sync::TimedMutex mutex; + ASSERT_TRUE(mutex.try_lock()); + // TODO(pwbug/291): Ensure it fails to lock when already held. + // EXPECT_FALSE(mutex.try_lock()); + mutex.unlock(); +} + +TEST(TimedMutex, TryLockUnlockFor) { + pw::sync::TimedMutex mutex; + + SystemClock::time_point before = SystemClock::now(); + ASSERT_TRUE(mutex.try_lock_for(kRoundedArbitraryDuration)); + SystemClock::duration time_elapsed = SystemClock::now() - before; + EXPECT_LT(time_elapsed, kRoundedArbitraryDuration); + + // TODO(pwbug/291): Ensure it blocks fails to lock when already held. + // before = SystemClock::now(); + // EXPECT_FALSE(mutex.try_lock_for(kRoundedArbitraryDuration)); + // time_elapsed = SystemClock::now() - before; + /// EXPECT_GE(time_elapsed, kRoundedArbitraryDuration); + + mutex.unlock(); +} + +TEST(TimedMutex, TryLockUnlockUntil) { + pw::sync::TimedMutex mutex; + + const SystemClock::time_point deadline = + SystemClock::now() + kRoundedArbitraryDuration; + ASSERT_TRUE(mutex.try_lock_until(deadline)); + EXPECT_LT(SystemClock::now(), deadline); + + // TODO(pwbug/291): Ensure it blocks fails to lock when already held. + // EXPECT_FALSE( + // mutex.try_lock_until(SystemClock::now() + kRoundedArbitraryDuration)); + // EXPECT_GE(SystemClock::now(), deadline); + + mutex.unlock(); +} + +TEST(TimedMutex, LockUnlockInC) { + pw::sync::TimedMutex mutex; + pw_sync_TimedMutex_CallLock(&mutex); + pw_sync_TimedMutex_CallUnlock(&mutex); +} + +TEST(TimedMutex, TryLockUnlockInC) { + pw::sync::TimedMutex mutex; + ASSERT_TRUE(pw_sync_TimedMutex_CallTryLock(&mutex)); + // TODO(pwbug/291): Ensure it fails to lock when already held. + // EXPECT_FALSE(pw_sync_TimedMutex_CallTryLock(&mutex)); + pw_sync_TimedMutex_CallUnlock(&mutex); +} + +TEST(TimedMutex, TryLockUnlockForInC) { + pw::sync::TimedMutex mutex; + + pw_chrono_SystemClock_TimePoint before = pw_chrono_SystemClock_Now(); + ASSERT_TRUE( + pw_sync_TimedMutex_CallTryLockFor(&mutex, kRoundedArbitraryDurationInC)); + pw_chrono_SystemClock_Duration time_elapsed = + pw_chrono_SystemClock_TimeElapsed(before, pw_chrono_SystemClock_Now()); + EXPECT_LT(time_elapsed.ticks, kRoundedArbitraryDurationInC.ticks); + + // TODO(pwbug/291): Ensure it blocks fails to lock when already held. + // before = pw_chrono_SystemClock_Now(); + // EXPECT_FALSE( + // pw_sync_TimedMutex_CallTryLockFor(&mutex, + // kRoundedArbitraryDurationInC)); + // time_elapsed = + // pw_chrono_SystemClock_TimeElapsed(before, pw_chrono_SystemClock_Now()); + // EXPECT_GE(time_elapsed.ticks, kRoundedArbitraryDurationInC.ticks); + + pw_sync_TimedMutex_CallUnlock(&mutex); +} + +TEST(TimedMutex, TryLockUnlockUntilInC) { + pw::sync::TimedMutex mutex; + pw_chrono_SystemClock_TimePoint deadline; + deadline.duration_since_epoch.ticks = + pw_chrono_SystemClock_Now().duration_since_epoch.ticks + + kRoundedArbitraryDurationInC.ticks; + ASSERT_TRUE(pw_sync_TimedMutex_CallTryLockUntil(&mutex, deadline)); + EXPECT_LT(pw_chrono_SystemClock_Now().duration_since_epoch.ticks, + deadline.duration_since_epoch.ticks); + + // TODO(pwbug/291): Ensure it blocks fails to lock when already held. + // EXPECT_FALSE(pw_sync_TimedMutex_CallTryLockUntil(&mutex, deadline)); + // EXPECT_GE(pw_chrono_SystemClock_Now().duration_since_epoch.ticks, + // deadline.duration_since_epoch.ticks); + + mutex.unlock(); +} + +} // namespace +} // namespace pw::sync diff --git a/pw_sync/timed_mutex_facade_test_c.c b/pw_sync/timed_mutex_facade_test_c.c new file mode 100644 index 000000000..50f5dcbc7 --- /dev/null +++ b/pw_sync/timed_mutex_facade_test_c.c @@ -0,0 +1,42 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. + +// These tests call the pw_sync module mutex API from C. The return values are +// checked in the main C++ tests. + +#include <stdbool.h> + +#include "pw_sync/timed_mutex.h" + +void pw_sync_TimedMutex_CallLock(pw_sync_TimedMutex* mutex) { + pw_sync_TimedMutex_Lock(mutex); +} + +bool pw_sync_TimedMutex_CallTryLock(pw_sync_TimedMutex* mutex) { + return pw_sync_TimedMutex_TryLock(mutex); +} + +bool pw_sync_TimedMutex_CallTryLockFor( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_Duration for_at_least) { + return pw_sync_TimedMutex_TryLockFor(mutex, for_at_least); +} + +bool pw_sync_TimedMutex_CallTryLockUntil( + pw_sync_TimedMutex* mutex, pw_chrono_SystemClock_TimePoint until_at_least) { + return pw_sync_TimedMutex_TryLockUntil(mutex, until_at_least); +} + +void pw_sync_TimedMutex_CallUnlock(pw_sync_TimedMutex* mutex) { + pw_sync_TimedMutex_Unlock(mutex); +} diff --git a/pw_sync_embos/BUILD b/pw_sync_embos/BUILD index 1adc21ca5..da66e9e92 100644 --- a/pw_sync_embos/BUILD +++ b/pw_sync_embos/BUILD @@ -101,23 +101,50 @@ pw_cc_library( deps = [ # TODO(pwbug/317): This should depend on embOS but our third parties # currently do not have Bazel support. - "//pw_chrono:system_clock", - "//pw_chrono_embos:system_clock_headers", + "//pw_sync:mutex_facade", ], ) pw_cc_library( name = "mutex", + deps = [ + ":mutex_headers", + "//pw_sync:mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex_headers", + hdrs = [ + "public/pw_sync_embos/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ], + includes = [ + "public", + "public_overrides", + ], + deps = [ + # TODO(pwbug/317): This should depend on embOS but our third parties + # currently do not have Bazel support. + "//pw_chrono:system_clock", + "//pw_sync:timed_mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex", srcs = [ - "mutex.cc", + "timed_mutex.cc", ], deps = [ - ":mutex_headers", + ":timed_mutex_headers", "//pw_interrupt:context", - "//pw_sync:mutex_facade", + "//pw_sync:timed_mutex_facade", + "//pw_chrono_embos:system_clock_headers", ], ) + pw_cc_library( name = "interrupt_spin_lock_headers", hdrs = [ diff --git a/pw_sync_embos/BUILD.gn b/pw_sync_embos/BUILD.gn index 8d9a0f4d7..043ae729c 100644 --- a/pw_sync_embos/BUILD.gn +++ b/pw_sync_embos/BUILD.gn @@ -98,13 +98,33 @@ pw_source_set("mutex") { ] public_deps = [ "$dir_pw_assert", + "$dir_pw_interrupt:context", + "$dir_pw_sync:mutex.facade", + "$dir_pw_third_party/embos", + ] +} + +# This target provides the backend for pw::sync::TimedMutex. +pw_source_set("timed_mutex") { + public_configs = [ + ":public_include_path", + ":backend_config", + ] + public = [ + "public/pw_sync_embos/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ] + public_deps = [ "$dir_pw_chrono:system_clock", + "$dir_pw_sync:timed_mutex.facade", + ] + sources = [ "timed_mutex.cc" ] + deps = [ + "$dir_pw_assert", "$dir_pw_chrono_embos:system_clock", "$dir_pw_interrupt:context", "$dir_pw_third_party/embos", ] - sources = [ "mutex.cc" ] - deps = [ "$dir_pw_sync:mutex.facade" ] assert( pw_chrono_SYSTEM_CLOCK_BACKEND == "" || pw_chrono_SYSTEM_CLOCK_BACKEND == "$dir_pw_chrono_embos:system_clock", diff --git a/pw_sync_embos/public/pw_sync_embos/mutex_inline.h b/pw_sync_embos/public/pw_sync_embos/mutex_inline.h index a8dd26ec4..9f8c14745 100644 --- a/pw_sync_embos/public/pw_sync_embos/mutex_inline.h +++ b/pw_sync_embos/public/pw_sync_embos/mutex_inline.h @@ -15,8 +15,6 @@ #include "RTOS.h" #include "pw_assert/light.h" -#include "pw_chrono/system_clock.h" -#include "pw_chrono_embos/system_clock_constants.h" #include "pw_interrupt/context.h" #include "pw_sync/mutex.h" @@ -37,13 +35,6 @@ inline bool Mutex::try_lock() { return OS_Request(&native_type_) != 0; } -inline bool Mutex::try_lock_until( - chrono::SystemClock::time_point until_at_least) { - // Note that if this deadline is in the future, it will get rounded up by - // one whole tick due to how try_lock_for is implemented. - return try_lock_for(until_at_least - chrono::SystemClock::now()); -} - inline void Mutex::unlock() { PW_ASSERT(!interrupt::InInterruptContext()); OS_Unuse(&native_type_); diff --git a/pw_sync_embos/public/pw_sync_embos/timed_mutex_inline.h b/pw_sync_embos/public/pw_sync_embos/timed_mutex_inline.h new file mode 100644 index 000000000..2ea7bff86 --- /dev/null +++ b/pw_sync_embos/public/pw_sync_embos/timed_mutex_inline.h @@ -0,0 +1,28 @@ +// Copyright 2021 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_chrono/system_clock.h" +#include "pw_sync/timed_mutex.h" + +namespace pw::sync { + +inline bool TimedMutex::try_lock_until( + chrono::SystemClock::time_point until_at_least) { + // Note that if this deadline is in the future, it will get rounded up by + // one whole tick due to how try_lock_for is implemented. + return try_lock_for(until_at_least - chrono::SystemClock::now()); +} + +} // namespace pw::sync diff --git a/pw_sync_embos/public_overrides/pw_sync_backend/timed_mutex_inline.h b/pw_sync_embos/public_overrides/pw_sync_backend/timed_mutex_inline.h new file mode 100644 index 000000000..467eb9fec --- /dev/null +++ b/pw_sync_embos/public_overrides/pw_sync_backend/timed_mutex_inline.h @@ -0,0 +1,16 @@ +// Copyright 2021 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_sync_embos/timed_mutex_inline.h" diff --git a/pw_sync_embos/mutex.cc b/pw_sync_embos/timed_mutex.cc index fb33d9f89..c08bef473 100644 --- a/pw_sync_embos/mutex.cc +++ b/pw_sync_embos/timed_mutex.cc @@ -12,7 +12,7 @@ // License for the specific language governing permissions and limitations under // the License. -#include "pw_sync/mutex.h" +#include "pw_sync/timed_mutex.h" #include <algorithm> @@ -26,7 +26,7 @@ using pw::chrono::SystemClock; namespace pw::sync { -bool Mutex::try_lock_for(SystemClock::duration for_at_least) { +bool TimedMutex::try_lock_for(SystemClock::duration for_at_least) { PW_DCHECK(!interrupt::InInterruptContext()); // Use non-blocking try_lock for negative and zero length durations. @@ -40,7 +40,7 @@ bool Mutex::try_lock_for(SystemClock::duration for_at_least) { pw::chrono::embos::kMaxTimeout - SystemClock::duration(1); while (for_at_least > kMaxTimeoutMinusOne) { const int lock_count = OS_UseTimed( - &native_type_, static_cast<OS_TIME>(kMaxTimeoutMinusOne.count())); + &native_handle(), static_cast<OS_TIME>(kMaxTimeoutMinusOne.count())); if (lock_count != 0) { PW_CHECK_UINT_EQ(1, lock_count, "Recursive locking is not permitted"); return true; @@ -48,7 +48,7 @@ bool Mutex::try_lock_for(SystemClock::duration for_at_least) { for_at_least -= kMaxTimeoutMinusOne; } const int lock_count = OS_UseTimed( - &native_type_, static_cast<OS_TIME>(for_at_least.count() + 1)); + &native_handle(), static_cast<OS_TIME>(for_at_least.count() + 1)); PW_CHECK_UINT_LE(1, lock_count, "Recursive locking is not permitted"); return lock_count == 1; } diff --git a/pw_sync_freertos/BUILD b/pw_sync_freertos/BUILD index f82c95a64..7ad275953 100644 --- a/pw_sync_freertos/BUILD +++ b/pw_sync_freertos/BUILD @@ -100,20 +100,46 @@ pw_cc_library( deps = [ # TODO: This should depend on FreeRTOS but our third parties currently # do not have Bazel support. + "//pw_sync:mutex_facade", + ], +) + +pw_cc_library( + name = "mutex", + deps = [ + ":mutex_headers", + "//pw_sync:mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex_headers", + hdrs = [ + "public/pw_sync_freertos/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ], + includes = [ + "public", + "public_overrides", + ], + deps = [ + # TODO: This should depend on FreeRTOS but our third parties currently + # do not have Bazel support. "//pw_chrono:system_clock", "//pw_chrono_freertos:system_clock_headers", + "//pw_sync:timed_mutex_facade", ], ) pw_cc_library( - name = "mutex", + name = "timed_mutex", srcs = [ - "mutex.cc", + "timed_mutex.cc", ], deps = [ - ":mutex_headers", + ":timed_mutex_headers", "//pw_interrupt:context", - "//pw_sync:mutex_facade", + "//pw_sync:timed_mutex_facade", ], ) diff --git a/pw_sync_freertos/BUILD.gn b/pw_sync_freertos/BUILD.gn index 20782e324..425ef4a16 100644 --- a/pw_sync_freertos/BUILD.gn +++ b/pw_sync_freertos/BUILD.gn @@ -98,13 +98,33 @@ pw_source_set("mutex") { ] public_deps = [ "$dir_pw_assert", + "$dir_pw_interrupt:context", + "$dir_pw_sync:mutex.facade", + "$dir_pw_third_party/freertos", + ] +} + +# This target provides the backend for pw::sync::TimedMutex. +pw_source_set("timed_mutex") { + public_configs = [ + ":public_include_path", + ":backend_config", + ] + public = [ + "public/pw_sync_freertos/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ] + public_deps = [ "$dir_pw_chrono:system_clock", + "$dir_pw_sync:timed_mutex.facade", + ] + sources = [ "timed_mutex.cc" ] + deps = [ + "$dir_pw_assert", "$dir_pw_chrono_freertos:system_clock", "$dir_pw_interrupt:context", "$dir_pw_third_party/freertos", ] - sources = [ "mutex.cc" ] - deps = [ "$dir_pw_sync:mutex.facade" ] assert(pw_chrono_SYSTEM_CLOCK_BACKEND == "" || pw_chrono_SYSTEM_CLOCK_BACKEND == "$dir_pw_chrono_freertos:system_clock", diff --git a/pw_sync_freertos/public/pw_sync_freertos/mutex_inline.h b/pw_sync_freertos/public/pw_sync_freertos/mutex_inline.h index 534e9ce11..92f7c07d6 100644 --- a/pw_sync_freertos/public/pw_sync_freertos/mutex_inline.h +++ b/pw_sync_freertos/public/pw_sync_freertos/mutex_inline.h @@ -15,13 +15,19 @@ #include "FreeRTOS.h" #include "pw_assert/light.h" -#include "pw_chrono/system_clock.h" -#include "pw_chrono_freertos/system_clock_constants.h" #include "pw_interrupt/context.h" #include "pw_sync/mutex.h" #include "semphr.h" namespace pw::sync { +namespace backend { + +static_assert(configUSE_MUTEXES != 0, "FreeRTOS mutexes aren't enabled."); + +static_assert(configSUPPORT_STATIC_ALLOCATION != 0, + "FreeRTOS static allocations are required for this backend."); + +} // namespace backend inline Mutex::Mutex() : native_type_() { const SemaphoreHandle_t handle = xSemaphoreCreateMutexStatic(&native_type_); @@ -51,13 +57,6 @@ inline bool Mutex::try_lock() { return xSemaphoreTake(&native_type_, 0) == pdTRUE; } -inline bool Mutex::try_lock_until( - chrono::SystemClock::time_point until_at_least) { - // Note that if this deadline is in the future, it will get rounded up by - // one whole tick due to how try_lock_for is implemented. - return try_lock_for(until_at_least - chrono::SystemClock::now()); -} - inline void Mutex::unlock() { PW_ASSERT(!interrupt::InInterruptContext()); // Unlocking only fails if it was not locked first. diff --git a/pw_sync_freertos/public/pw_sync_freertos/timed_mutex_inline.h b/pw_sync_freertos/public/pw_sync_freertos/timed_mutex_inline.h new file mode 100644 index 000000000..bc114b9e3 --- /dev/null +++ b/pw_sync_freertos/public/pw_sync_freertos/timed_mutex_inline.h @@ -0,0 +1,28 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_chrono/system_clock.h" +#include "pw_sync/timed_mutex.h" + +namespace pw::sync { + +inline bool TimedMutex::try_lock_until( + chrono::SystemClock::time_point until_at_least) { + // Note that if this deadline is in the future, it will get rounded up by + // one whole tick due to how try_lock_for is implemented. + return try_lock_for(until_at_least - chrono::SystemClock::now()); +} + +} // namespace pw::sync diff --git a/pw_sync_freertos/public_overrides/pw_sync_backend/timed_mutex_inline.h b/pw_sync_freertos/public_overrides/pw_sync_backend/timed_mutex_inline.h new file mode 100644 index 000000000..e83372313 --- /dev/null +++ b/pw_sync_freertos/public_overrides/pw_sync_backend/timed_mutex_inline.h @@ -0,0 +1,16 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_sync_freertos/timed_mutex_inline.h" diff --git a/pw_sync_freertos/mutex.cc b/pw_sync_freertos/timed_mutex.cc index 7cdfef5df..0143b84dd 100644 --- a/pw_sync_freertos/mutex.cc +++ b/pw_sync_freertos/timed_mutex.cc @@ -12,7 +12,7 @@ // License for the specific language governing permissions and limitations under // the License. -#include "pw_sync/mutex.h" +#include "pw_sync/timed_mutex.h" #include <algorithm> @@ -26,16 +26,8 @@ using pw::chrono::SystemClock; namespace pw::sync { -namespace { -static_assert(configUSE_MUTEXES != 0, "FreeRTOS mutexes aren't enabled."); - -static_assert(configSUPPORT_STATIC_ALLOCATION != 0, - "FreeRTOS static allocations are required for this backend."); - -} // namespace - -bool Mutex::try_lock_for(SystemClock::duration for_at_least) { +bool TimedMutex::try_lock_for(SystemClock::duration for_at_least) { PW_DCHECK(!interrupt::InInterruptContext()); // Use non-blocking try_acquire for negative and zero length durations. @@ -48,14 +40,14 @@ bool Mutex::try_lock_for(SystemClock::duration for_at_least) { constexpr SystemClock::duration kMaxTimeoutMinusOne = pw::chrono::freertos::kMaxTimeout - SystemClock::duration(1); while (for_at_least > kMaxTimeoutMinusOne) { - if (xSemaphoreTake(&native_type_, + if (xSemaphoreTake(&native_handle(), static_cast<TickType_t>(kMaxTimeoutMinusOne.count())) == pdTRUE) { return true; } for_at_least -= kMaxTimeoutMinusOne; } - return xSemaphoreTake(&native_type_, + return xSemaphoreTake(&native_handle(), static_cast<TickType_t>(for_at_least.count() + 1)) == pdTRUE; } diff --git a/pw_sync_stl/BUILD b/pw_sync_stl/BUILD index 9ee8784df..c2f39a752 100644 --- a/pw_sync_stl/BUILD +++ b/pw_sync_stl/BUILD @@ -92,7 +92,7 @@ pw_cc_library( "public_overrides", ], deps = [ - "//pw_chrono:system_clock", + "//pw_sync:mutex_facade", ], ) @@ -100,12 +100,35 @@ pw_cc_library( name = "mutex", deps = [ ":mutex_headers", - "//pw_chrono:system_clock", "//pw_sync:mutex_facade", ], ) pw_cc_library( + name = "timed_mutex_headers", + hdrs = [ + "public/pw_sync_stl/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ], + includes = [ + "public", + "public_overrides", + ], + deps = [ + "//pw_chrono:system_clock", + "//pw_sync:timed_mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex", + deps = [ + ":timed_mutex_headers", + "//pw_sync:timed_mutex_facade", + ], +) + +pw_cc_library( name = "interrupt_spin_lock_headers", hdrs = [ "public/pw_sync_stl/interrupt_spin_lock_inline.h", diff --git a/pw_sync_stl/BUILD.gn b/pw_sync_stl/BUILD.gn index 2e71713f9..0b7e56b29 100644 --- a/pw_sync_stl/BUILD.gn +++ b/pw_sync_stl/BUILD.gn @@ -90,14 +90,25 @@ pw_source_set("mutex_backend") { "public_overrides/pw_sync_backend/mutex_inline.h", "public_overrides/pw_sync_backend/mutex_native.h", ] - deps = [ - "$dir_pw_chrono:system_clock", - "$dir_pw_sync:mutex.facade", + public_deps = [ "$dir_pw_sync:mutex.facade" ] +} + +# This target provides the backend for pw::sync::TimedMutex. +pw_source_set("timed_mutex_backend") { + public_configs = [ + ":public_include_path", + ":backend_config", + ] + public = [ + "$dir_pw_sync:timed_mutex.facade", + "public/pw_sync_stl/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", ] + public_deps = [ "$dir_pw_chrono:system_clock" ] assert( pw_chrono_SYSTEM_CLOCK_BACKEND == "" || pw_chrono_SYSTEM_CLOCK_BACKEND == "$dir_pw_chrono_stl:system_clock", - "The STL pw::sync::Mutex backend only works with the STL " + + "The STL pw::sync::TimedMutex backend only works with the STL " + "pw::chrono::SystemClock backend.") } diff --git a/pw_sync_stl/public/pw_sync_stl/mutex_inline.h b/pw_sync_stl/public/pw_sync_stl/mutex_inline.h index b4459eba0..88548c4eb 100644 --- a/pw_sync_stl/public/pw_sync_stl/mutex_inline.h +++ b/pw_sync_stl/public/pw_sync_stl/mutex_inline.h @@ -13,7 +13,6 @@ // the License. #pragma once -#include "pw_chrono/system_clock.h" #include "pw_sync/mutex.h" namespace pw::sync { @@ -26,15 +25,6 @@ inline void Mutex::lock() { native_type_.lock(); } inline bool Mutex::try_lock() { return native_type_.try_lock(); } -inline bool Mutex::try_lock_for(chrono::SystemClock::duration for_at_least) { - return native_type_.try_lock_for(for_at_least); -} - -inline bool Mutex::try_lock_until( - chrono::SystemClock::time_point until_at_least) { - return native_type_.try_lock_until(until_at_least); -} - inline void Mutex::unlock() { native_type_.unlock(); } inline Mutex::native_handle_type Mutex::native_handle() { return native_type_; } diff --git a/pw_sync_stl/public/pw_sync_stl/timed_mutex_inline.h b/pw_sync_stl/public/pw_sync_stl/timed_mutex_inline.h new file mode 100644 index 000000000..5bfd3409d --- /dev/null +++ b/pw_sync_stl/public/pw_sync_stl/timed_mutex_inline.h @@ -0,0 +1,31 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_chrono/system_clock.h" +#include "pw_sync/mutex.h" + +namespace pw::sync { + +inline bool TimedMutex::try_lock_for( + chrono::SystemClock::duration for_at_least) { + return native_handle().try_lock_for(for_at_least); +} + +inline bool TimedMutex::try_lock_until( + chrono::SystemClock::time_point until_at_least) { + return native_handle().try_lock_until(until_at_least); +} + +} // namespace pw::sync diff --git a/pw_sync_stl/public_overrides/pw_sync_backend/timed_mutex_inline.h b/pw_sync_stl/public_overrides/pw_sync_backend/timed_mutex_inline.h new file mode 100644 index 000000000..e8ddcc13e --- /dev/null +++ b/pw_sync_stl/public_overrides/pw_sync_backend/timed_mutex_inline.h @@ -0,0 +1,16 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_sync_stl/timed_mutex_inline.h" diff --git a/pw_sync_threadx/BUILD b/pw_sync_threadx/BUILD index 837ed0f01..b9129f9b0 100644 --- a/pw_sync_threadx/BUILD +++ b/pw_sync_threadx/BUILD @@ -100,20 +100,46 @@ pw_cc_library( deps = [ # TODO: This should depend on ThreadX but our third parties currently # do not have Bazel support. - "//pw_chrono:system_clock", + "//pw_sync:mutex_facade", ], ) pw_cc_library( name = "mutex", + deps = [ + ":mutex_headers", + "//pw_sync:mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex_headers", + hdrs = [ + "public/pw_sync_threadx/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", + ], + includes = [ + "public", + "public_overrides", + ], + deps = [ + # TODO: This should depend on ThreadX but our third parties currently + # do not have Bazel support. + "//pw_chrono:system_clock", + "//pw_sync:timed_mutex_facade", + ], +) + +pw_cc_library( + name = "timed_mutex", srcs = [ - "mutex.cc", + "timed_mutex.cc", ], deps = [ - ":mutex_headers", + ":timed_mutex_headers", "//pw_chrono_threadx:system_clock_headers", "//pw_interrupt:context", - "//pw_sync:mutex_facade", + "//pw_sync:timed_mutex_facade", ], ) diff --git a/pw_sync_threadx/BUILD.gn b/pw_sync_threadx/BUILD.gn index 41d5b320e..b02d7575e 100644 --- a/pw_sync_threadx/BUILD.gn +++ b/pw_sync_threadx/BUILD.gn @@ -29,6 +29,26 @@ config("backend_config") { visibility = [ ":*" ] } +# This target provides the backend for pw::sync::Mutex. +pw_source_set("mutex") { + public_configs = [ + ":public_include_path", + ":backend_config", + ] + public = [ + "public/pw_sync_threadx/mutex_inline.h", + "public/pw_sync_threadx/mutex_native.h", + "public_overrides/pw_sync_backend/mutex_inline.h", + "public_overrides/pw_sync_backend/mutex_native.h", + ] + public_deps = [ + "$dir_pw_assert", + "$dir_pw_interrupt:context", + "$dir_pw_sync:mutex.facade", + "$dir_pw_third_party/threadx", + ] +} + if (pw_chrono_SYSTEM_CLOCK_BACKEND != "") { # This target provides the backend for pw::sync::BinarySemaphore. pw_source_set("binary_semaphore") { @@ -91,27 +111,25 @@ if (pw_chrono_SYSTEM_CLOCK_BACKEND != "") { "the ThreadX pw::chrono::SystemClock backend.") } - # This target provides the backend for pw::sync::Mutex. - pw_source_set("mutex") { + # This target provides the backend for pw::sync::TimedMutex. + pw_source_set("timed_mutex") { public_configs = [ ":public_include_path", ":backend_config", ] public = [ - "public/pw_sync_threadx/mutex_inline.h", - "public/pw_sync_threadx/mutex_native.h", - "public_overrides/pw_sync_backend/mutex_inline.h", - "public_overrides/pw_sync_backend/mutex_native.h", + "public/pw_sync_threadx/timed_mutex_inline.h", + "public_overrides/pw_sync_backend/timed_mutex_inline.h", ] public_deps = [ - "$dir_pw_assert", "$dir_pw_chrono:system_clock", - "$dir_pw_interrupt:context", - "$dir_pw_third_party/threadx", + "$dir_pw_sync:timed_mutex.facade", ] - sources = [ "mutex.cc" ] + sources = [ "timed_mutex.cc" ] deps = [ - "$dir_pw_sync:mutex.facade", + "$dir_pw_assert", + "$dir_pw_interrupt:context", + "$dir_pw_third_party/threadx", pw_chrono_SYSTEM_CLOCK_BACKEND, ] assert(pw_sync_OVERRIDE_SYSTEM_CLOCK_BACKEND_CHECK || diff --git a/pw_sync_threadx/public/pw_sync_threadx/mutex_inline.h b/pw_sync_threadx/public/pw_sync_threadx/mutex_inline.h index af537bf59..564b24e16 100644 --- a/pw_sync_threadx/public/pw_sync_threadx/mutex_inline.h +++ b/pw_sync_threadx/public/pw_sync_threadx/mutex_inline.h @@ -14,7 +14,6 @@ #pragma once #include "pw_assert/light.h" -#include "pw_chrono/system_clock.h" #include "pw_interrupt/context.h" #include "pw_sync/mutex.h" #include "tx_api.h" @@ -53,13 +52,6 @@ inline bool Mutex::try_lock() { return true; } -inline bool Mutex::try_lock_until( - chrono::SystemClock::time_point until_at_least) { - // Note that if this deadline is in the future, it will get rounded up by - // one whole tick due to how try_lock_for is implemented. - return try_lock_for(until_at_least - chrono::SystemClock::now()); -} - inline void Mutex::unlock() { // Enforce the pw::sync::Mutex IRQ contract. PW_ASSERT(!interrupt::InInterruptContext()); diff --git a/pw_sync_threadx/public/pw_sync_threadx/timed_mutex_inline.h b/pw_sync_threadx/public/pw_sync_threadx/timed_mutex_inline.h new file mode 100644 index 000000000..8644046fd --- /dev/null +++ b/pw_sync_threadx/public/pw_sync_threadx/timed_mutex_inline.h @@ -0,0 +1,28 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_chrono/system_clock.h" +#include "pw_sync/timed_mutex.h" + +namespace pw::sync { + +inline bool Mutex::try_lock_until( + chrono::SystemClock::time_point until_at_least) { + // Note that if this deadline is in the future, it will get rounded up by + // one whole tick due to how try_lock_for is implemented. + return try_lock_for(until_at_least - chrono::SystemClock::now()); +} + +} // namespace pw::sync diff --git a/pw_sync_threadx/public_overrides/pw_sync_backend/timed_mutex_inline.h b/pw_sync_threadx/public_overrides/pw_sync_backend/timed_mutex_inline.h new file mode 100644 index 000000000..8bacb62ab --- /dev/null +++ b/pw_sync_threadx/public_overrides/pw_sync_backend/timed_mutex_inline.h @@ -0,0 +1,16 @@ +// Copyright 2020 The Pigweed Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); you may not +// use this file except in compliance with the License. You may obtain a copy of +// the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +// WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +// License for the specific language governing permissions and limitations under +// the License. +#pragma once + +#include "pw_sync_threadx/timed_mutex_inline.h" diff --git a/pw_sync_threadx/mutex.cc b/pw_sync_threadx/timed_mutex.cc index ce05399ef..92b14eda2 100644 --- a/pw_sync_threadx/mutex.cc +++ b/pw_sync_threadx/timed_mutex.cc @@ -12,7 +12,7 @@ // License for the specific language governing permissions and limitations under // the License. -#include "pw_sync/mutex.h" +#include "pw_sync/timed_mutex.h" #include <algorithm> @@ -26,8 +26,8 @@ using pw::chrono::SystemClock; namespace pw::sync { -bool Mutex::try_lock_for(SystemClock::duration for_at_least) { - // Enforce the pw::sync::Mutex IRQ contract. +bool TimedMutex::try_lock_for(SystemClock::duration for_at_least) { + // Enforce the pw::sync::TimedMutex IRQ contract. PW_DCHECK(!interrupt::InInterruptContext()); // Use non-blocking try_lock for negative or zero length durations. diff --git a/targets/host/target_toolchains.gni b/targets/host/target_toolchains.gni index 7f3411ad9..6ff3789e6 100644 --- a/targets/host/target_toolchains.gni +++ b/targets/host/target_toolchains.gni @@ -87,6 +87,7 @@ _win_incompatible_config = { pw_sync_COUNTING_SEMAPHORE_BACKEND = "$dir_pw_sync_stl:counting_semaphore_backend" pw_sync_MUTEX_BACKEND = "$dir_pw_sync_stl:mutex_backend" + pw_sync_TIMED_MUTEX_BACKEND = "$dir_pw_sync_stl:timed_mutex_backend" # Configure backends for pw_thread's facades. pw_thread_ID_BACKEND = "$dir_pw_thread_stl:id" |