diff --git a/Os/CMakeLists.txt b/Os/CMakeLists.txt index ccd34c43b87..90e992fe601 100644 --- a/Os/CMakeLists.txt +++ b/Os/CMakeLists.txt @@ -138,7 +138,29 @@ register_fprime_module( add_named_os_module(Console Fw_Logger) add_named_os_module("File;FileSystem;Directory" Utils_Hash Fw_DataStructures) add_named_os_module(Task ${CMAKE_THREAD_LIBS_INIT}) -add_named_os_module("Mutex;Condition") +# Mutex: Mutex.hpp is a thin public alias header (no Mutex.cpp); the interface and +# link-time delegate live in MutexInterface.hpp / DelegateMutex.{hpp,cpp}. Registered +# explicitly (rather than via add_named_os_module) because that helper assumes a +# matching .cpp per header. Condition.{cpp,hpp} stays bundled into Os_Mutex as +# before, preserving the module name that the Stub impl and REQUIRES_IMPLEMENTATIONS +# references depend on. +register_fprime_module( + "Os_Mutex" + REQUIRES_IMPLEMENTATIONS + "Os_Mutex" + SOURCES + "${CMAKE_CURRENT_LIST_DIR}/DelegateMutex.cpp" + "${CMAKE_CURRENT_LIST_DIR}/Condition.cpp" + HEADERS + "${CMAKE_CURRENT_LIST_DIR}/Mutex.hpp" + "${CMAKE_CURRENT_LIST_DIR}/MutexInterface.hpp" + "${CMAKE_CURRENT_LIST_DIR}/DelegateMutex.hpp" + "${CMAKE_CURRENT_LIST_DIR}/Condition.hpp" + DEPENDS + Fw_Time + Fw_Types +) +fprime_target_dependencies(Os PUBLIC "Os_Mutex") add_named_os_module(CountingSemaphore) add_named_os_module(Queue) add_named_os_module(Cpu) diff --git a/Os/DelegateMutex.cpp b/Os/DelegateMutex.cpp new file mode 100644 index 00000000000..0cbc9a93cb6 --- /dev/null +++ b/Os/DelegateMutex.cpp @@ -0,0 +1,39 @@ +// ====================================================================== +// \title Os/DelegateMutex.cpp +// \brief implementation of Os::DelegateMutex (link-time delegating Os::Mutex) +// ====================================================================== +#include +#include + +namespace Os { + +// ---------------------------------------------------------------------- +// DelegateMutex: link-time delegating implementation +// ---------------------------------------------------------------------- + +DelegateMutex::DelegateMutex() : m_delegate(*MutexInterface::getDelegate(m_handle_storage)) { + // m_handle_storage is placement-new storage populated by getDelegate(); do not value-initialize it (#5297) + FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); +} + +DelegateMutex::~DelegateMutex() { + FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); + m_delegate.~MutexInterface(); +} + +MutexHandle* DelegateMutex::getHandle() { + FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); + return this->m_delegate.getHandle(); +} + +DelegateMutex::Status DelegateMutex::take() { + FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); + return this->m_delegate.take(); +} + +DelegateMutex::Status DelegateMutex::release() { + FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); + return this->m_delegate.release(); +} + +} // namespace Os diff --git a/Os/DelegateMutex.hpp b/Os/DelegateMutex.hpp new file mode 100644 index 00000000000..0011a48506b --- /dev/null +++ b/Os/DelegateMutex.hpp @@ -0,0 +1,48 @@ +// ====================================================================== +// \title Os/DelegateMutex.hpp +// \brief Define the Os::DelegateMutex class +// ====================================================================== +#ifndef OS_DELEGATEMUTEX_HPP_ +#define OS_DELEGATEMUTEX_HPP_ + +#include "Os/MutexInterface.hpp" + +namespace Os { + +//! \brief Link-time delegating Mutex implementation. +//! +//! Stores an implementation-defined mutex handle in a byte array and forwards all operations to a +//! delegate constructed (via placement-new) by MutexInterface::getDelegate(). Which getDelegate() +//! is linked selects the concrete implementation at link time. This is the default binding of the +//! Os::Mutex alias; platforms may instead alias Os::Mutex directly to a concrete implementation for +//! compile-time selection (see config/OsDelegateMutex.hpp). +class DelegateMutex final : public MutexInterface { + public: + DelegateMutex(); //!< Constructor. Mutex is unlocked when created + ~DelegateMutex() final; //!< Destructor + + //! \brief copy constructor is forbidden + DelegateMutex(const DelegateMutex& other) = delete; + + //! \brief assignment operator is forbidden + DelegateMutex& operator=(const DelegateMutex& other) = delete; + + //! \brief return the underlying mutex handle (implementation specific) + //! \return internal mutex handle representation + MutexHandle* getHandle() override; + + Status take() override; //!< lock the mutex and get return status + Status release() override; //!< unlock the mutex and get return status + + private: + // This section is used to store the implementation-defined mutex handle. To Os::Mutex and fprime, this type is + // opaque and thus normal allocation cannot be done. Instead, we allow the implementor to store the handle in + // the byte-array here and set `m_delegate` to that address for storage. + // + alignas(FW_HANDLE_ALIGNMENT) MutexHandleStorage m_handle_storage; //!< Mutex handle storage + MutexInterface& m_delegate; //!< Delegate for the real implementation +}; + +} // namespace Os + +#endif // OS_DELEGATEMUTEX_HPP_ diff --git a/Os/Mutex.cpp b/Os/Mutex.cpp deleted file mode 100644 index d4deb6d970f..00000000000 --- a/Os/Mutex.cpp +++ /dev/null @@ -1,56 +0,0 @@ -// ====================================================================== -// \title Os/Mutex.cpp -// \brief common function implementation for Os::Mutex -// ====================================================================== -#include -#include - -namespace Os { - -Mutex::Mutex() : m_handle_storage(), m_delegate(*MutexInterface::getDelegate(m_handle_storage)) { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); -} - -Mutex::~Mutex() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - m_delegate.~MutexInterface(); -} - -MutexHandle* Mutex::getHandle() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - return this->m_delegate.getHandle(); -} - -Mutex::Status Mutex::take() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - return this->m_delegate.take(); -} - -Mutex::Status Mutex::release() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - return this->m_delegate.release(); -} - -void Mutex::lock() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - Mutex::Status status = this->take(); - FW_ASSERT(status == Mutex::Status::OP_OK, - static_cast(reinterpret_cast(this)), status); -} - -void Mutex::unLock() { - FW_ASSERT(&this->m_delegate == reinterpret_cast(&this->m_handle_storage[0])); - Mutex::Status status = this->release(); - FW_ASSERT(status == Mutex::Status::OP_OK, - static_cast(reinterpret_cast(this)), status); -} - -ScopeLock::ScopeLock(Mutex& mutex) : m_mutex(mutex) { - this->m_mutex.lock(); -} - -ScopeLock::~ScopeLock() { - this->m_mutex.unLock(); -} - -} // namespace Os diff --git a/Os/Mutex.hpp b/Os/Mutex.hpp index 4870be608e5..6c4a656e81f 100644 --- a/Os/Mutex.hpp +++ b/Os/Mutex.hpp @@ -1,105 +1,41 @@ // ====================================================================== // \title Os/Mutex.hpp -// \brief common definitions for Os::Mutex +// \brief public Os::Mutex interface and alias +// +// This header aggregates all definitions needed to use Os::Mutex: +// the interface, the configured alias, and the concrete delegate type. +// +// The includes below must appear in the order given: each depends on names +// defined by the one before it, so reordering them will fail to compile. +// The ordering constraints are: +// +// 1. config/OsDelegateMutex.hpp (CFG) defines the Os::Mutex type alias by +// forward-declaring a link-time delegate (e.g. DelegateMutex) or directly +// aliasing a concrete implementation (e.g. Va416x0Os::AtomicMutex::AtomicMutex). +// Must not include Os OSAL headers (they aren't yet defined). +// Should only be included in Os/MutexInterface.hpp. +// +// 2. Os/MutexInterface.hpp (IF) includes CFG first, then defines +// MutexHandle, MutexInterface, and ScopeLock. +// +// 3. OS_MUTEX_HEADER (IMPL) is defined by CFG and points to the concrete +// implementation header. If using delegation, this points to +// Os/DelegateMutex.hpp. If using compile-time selection, it points +// directly to a platform-specific implementation (e.g. AtomicMutex.hpp). +// +// MutexInterface.hpp must precede OS_MUTEX_HEADER here, +// and CFG must never include either of them (that would form a cycle). // ====================================================================== #ifndef Os_Mutex_hpp #define Os_Mutex_hpp -#include -#include +#include "Os/MutexInterface.hpp" -namespace Os { - -struct MutexHandle {}; - -class MutexInterface { - public: - enum Status { - OP_OK, //!< Operation was successful - ERROR_BUSY, //!< Mutex is busy - ERROR_DEADLOCK, //!< Deadlock condition detected - NOT_SUPPORTED, //!< Mutex does not support operation - ERROR_OTHER //!< All other errors - }; - - //! \brief default constructor - MutexInterface() = default; - - //! \brief default virtual destructor - virtual ~MutexInterface() = default; - - //! \brief copy constructor is forbidden - MutexInterface(const MutexInterface& other) = delete; - - //! \brief copy constructor is forbidden - MutexInterface(const MutexInterface* other) = delete; - - //! \brief assignment operator is forbidden - MutexInterface& operator=(const MutexInterface& other) = delete; - - //! \brief return the underlying mutex handle (implementation specific) - //! \return internal mutex handle representation - virtual MutexHandle* getHandle() = 0; - - //! \brief provide a pointer to a Mutex delegate object - static MutexInterface* getDelegate(MutexHandleStorage& aligned_new_memory); - - virtual Status take() = 0; //!< lock the mutex return status - virtual Status release() = 0; //!< unlock the mutex return status -}; - -class Mutex final : public MutexInterface { - public: - Mutex(); //!< Constructor. Mutex is unlocked when created - ~Mutex() final; //!< Destructor - - //! \brief return the underlying mutex handle (implementation specific) - //! \return internal mutex handle representation - MutexHandle* getHandle() override; - - Status take() override; //!< lock the mutex and get return status - Status release() override; //!< unlock the mutex and get return status - void lock(); //!< lock the mutex and assert success - void unLock(); //!< unlock the mutex and assert success - void unlock() { this->unLock(); } //!< alias for unLock to meet BasicLockable requirements - - private: - // This section is used to store the implementation-defined mutex handle. To Os::Mutex and fprime, this type is - // opaque and thus normal allocation cannot be done. Instead, we allow the implementor to store then handle in - // the byte-array here and set `handle` to that address for storage. - // - alignas(FW_HANDLE_ALIGNMENT) MutexHandleStorage m_handle_storage; //!< Mutex handle storage - MutexInterface& m_delegate; //!< Delegate for the real implementation -}; -//! \brief locks a mutex within the current scope -//! -//! The scope lock will lock the associated mutex immediately and will ensure the mutex is unlock when the scope lock -//! is destroyed. -//! -//! \warning it is unadvisable to dynamically allocate ScopeLock as this violates the implied usage. -class ScopeLock { - public: - //! \brief construct the scope lock - //! - //! Will lock the supplied mutex and will unlock the mutex when this object goes out of scope. - //! \param mutex - explicit ScopeLock(Mutex& mutex); - - //!\brief unlock the scoped mutex - ~ScopeLock(); - - //! \brief copy constructor is forbidden - ScopeLock(const ScopeLock& other) = delete; - - //! \brief copy constructor is forbidden - ScopeLock(const ScopeLock* other) = delete; - - //! \brief assignment operator is forbidden - ScopeLock& operator=(const ScopeLock& other) = delete; +// Validate that OS_MUTEX_HEADER was defined by config/OsDelegateMutex.hpp +#ifndef OS_MUTEX_HEADER +#error "OS_MUTEX_HEADER must be defined in config/OsDelegateMutex.hpp" +#endif - private: - Mutex& m_mutex; //!< Stores the mutex reference -}; -} // namespace Os +#include OS_MUTEX_HEADER -#endif +#endif // Os_Mutex_hpp diff --git a/Os/MutexInterface.hpp b/Os/MutexInterface.hpp new file mode 100644 index 00000000000..df3e4848550 --- /dev/null +++ b/Os/MutexInterface.hpp @@ -0,0 +1,114 @@ +// ====================================================================== +// \title Os/MutexInterface.hpp +// \brief Os::MutexHandle, Os::MutexInterface, and Os::ScopeLock definitions +// ====================================================================== +#ifndef OS_MUTEXINTERFACE_HPP_ +#define OS_MUTEXINTERFACE_HPP_ + +#include +#include +#include +#include "config/OsDelegateMutex.hpp" // defines Os::Mutex alias and OS_MUTEX_HEADER consumed by Os/Mutex.hpp; do not remove + +namespace Os { + +struct MutexHandle {}; + +class MutexInterface { + public: + enum Status { + OP_OK, //!< Operation was successful + ERROR_BUSY, //!< Mutex is busy + ERROR_DEADLOCK, //!< Deadlock condition detected + NOT_SUPPORTED, //!< Mutex does not support operation + ERROR_OTHER //!< All other errors + }; + + //! \brief default constructor + MutexInterface() = default; + + //! \brief default virtual destructor + virtual ~MutexInterface() = default; + + //! \brief copy constructor is forbidden + MutexInterface(const MutexInterface& other) = delete; + + //! \brief copy constructor is forbidden + MutexInterface(const MutexInterface* other) = delete; + + //! \brief assignment operator is forbidden + MutexInterface& operator=(const MutexInterface& other) = delete; + + //! \brief return the underlying mutex handle (implementation specific) + //! \return internal mutex handle representation + virtual MutexHandle* getHandle() = 0; + + //! \brief provide a pointer to a Mutex delegate object + static MutexInterface* getDelegate(MutexHandleStorage& aligned_new_memory); + + // ------------------------------------------------------------------ + // Mutex operations to be implemented by an OSAL implementation + // ------------------------------------------------------------------ + + virtual Status take() = 0; //!< lock the mutex return status + virtual Status release() = 0; //!< unlock the mutex return status + + // ------------------------------------------------------------------ + // Common (non-virtual) functions built on top of take()/release(). + // Defined inline on the interface (per fprime#5249) so they are available + // regardless of which implementation Os::Mutex is configured to be, and so + // that under compile-time selection the call site can devirtualize take()/ + // release() and inline the whole acquisition without relying on LTO. + // ------------------------------------------------------------------ + + //! \brief lock the mutex and assert success + void lock() { + const Status status = this->take(); + FW_ASSERT(status == Status::OP_OK, + static_cast(reinterpret_cast(this)), status); + } + + //! \brief unlock the mutex and assert success + void unLock() { + const Status status = this->release(); + FW_ASSERT(status == Status::OP_OK, + static_cast(reinterpret_cast(this)), status); + } + + //! \brief alias for unLock to meet BasicLockable requirements + void unlock() { this->unLock(); } +}; + +//! \brief locks a mutex within the current scope +//! +//! The scope lock will lock the associated mutex immediately and will ensure the mutex is unlock when the scope lock +//! is destroyed. +//! +//! \warning it is unadvisable to dynamically allocate ScopeLock as this violates the implied usage. +class ScopeLock { + public: + //! \brief construct the scope lock + //! + //! Will lock the supplied mutex and will unlock the mutex when this object goes out of scope. + //! \param mutex + explicit ScopeLock(MutexInterface& mutex) : m_mutex(mutex) { this->m_mutex.lock(); } + + //!\brief unlock the scoped mutex + ~ScopeLock() { this->m_mutex.unLock(); } + + //! \brief copy constructor is forbidden + ScopeLock(const ScopeLock& other) = delete; + + //! \brief copy constructor is forbidden + ScopeLock(const ScopeLock* other) = delete; + + //! \brief assignment operator is forbidden + ScopeLock& operator=(const ScopeLock& other) = delete; + + private: + MutexInterface& m_mutex; //!< Stores the mutex reference +}; + +} // namespace Os + +#endif // OS_MUTEXINTERFACE_HPP_ diff --git a/Os/docs/sdd.md b/Os/docs/sdd.md index 9f3a95d7244..782d55a91fb 100644 --- a/Os/docs/sdd.md +++ b/Os/docs/sdd.md @@ -153,7 +153,8 @@ For performance-critical services, the OSAL supports **compile-time selection** - More complex build configuration **Currently Supported Services:** -- **RawTime** +- **RawTime** (`config/OsDelegateRawTime.hpp`, `OS_RAW_TIME_HEADER`) +- **Mutex** (`config/OsDelegateMutex.hpp`, `OS_MUTEX_HEADER`) The configuration header mechanism allows projects to opt into compile-time selection while maintaining link-time selection as the default for backward compatibility. diff --git a/default/config/CMakeLists.txt b/default/config/CMakeLists.txt index 1d92d560897..4fba6d38efb 100644 --- a/default/config/CMakeLists.txt +++ b/default/config/CMakeLists.txt @@ -42,6 +42,7 @@ register_fprime_config( "${CMAKE_CURRENT_LIST_DIR}/IpCfg.hpp" "${CMAKE_CURRENT_LIST_DIR}/LocklessQueueCfg.hpp" "${CMAKE_CURRENT_LIST_DIR}/OsDelegateRawTime.hpp" + "${CMAKE_CURRENT_LIST_DIR}/OsDelegateMutex.hpp" "${CMAKE_CURRENT_LIST_DIR}/PassiveTextLoggerCfg.hpp" "${CMAKE_CURRENT_LIST_DIR}/PrmDbImplCfg.hpp" "${CMAKE_CURRENT_LIST_DIR}/PrmDbImplTesterCfg.hpp" diff --git a/default/config/OsDelegateMutex.hpp b/default/config/OsDelegateMutex.hpp new file mode 100644 index 00000000000..fa7f2f57ece --- /dev/null +++ b/default/config/OsDelegateMutex.hpp @@ -0,0 +1,57 @@ +// ====================================================================== +// \title config/OsDelegateMutex.hpp +// \brief configured selection of the Os::Mutex implementation +// +// This header configures how Os::Mutex resolves to a concrete implementation. +// Two mechanisms are available: +// +// 1. Link-time selection (default): Os::Mutex aliases to Os::DelegateMutex, +// which wraps a MutexInterface reference. At construction, DelegateMutex +// calls MutexInterface::getDelegate() to construct the platform-specific +// implementation (e.g., Stub::Mutex::StubMutex) via placement-new. The linker +// selects which getDelegate() based on which DefaultMutex.cpp is linked. +// Calls dispatch through the vtable at runtime. +// +// 2. Compile-time selection (performance optimization): Platforms may override +// this header to alias Os::Mutex directly to a concrete implementation +// (e.g., Va416x0Os::AtomicMutex::AtomicMutex). This eliminates the wrapper and +// virtual dispatch, enabling inlining (lock()/unLock()/ScopeLock are defined +// inline on MutexInterface, so acquisitions devirtualize without requiring LTO). +// +// WARNING: the aliased type MUST derive from Os::MutexInterface (lock()/unLock()/ +// ScopeLock are defined there). In addition, if the build links an Os_Mutex +// implementation module that also provides Os::ConditionVariable, that +// ConditionVariable must understand the aliased type's MutexHandle — e.g. +// Os/Posix/ConditionVariable.cpp reinterpret_casts the handle to PosixMutexHandle. +// Mixing a compile-time Mutex with a mismatched ConditionVariable implementation +// is undefined behavior with no build-time error. +// +// Example compile-time selection override: +// +// namespace Va416x0Os { namespace AtomicMutex { class AtomicMutex; } } +// namespace Os { using Mutex = Va416x0Os::AtomicMutex::AtomicMutex; } +// #define OS_MUTEX_HEADER "Va416x0/Os/AtomicMutex/AtomicMutex.hpp" +// +// IMPORTANT: CIRCULAR DEPENDENCY PREVENTION +// +// - This header MUST NOT include any Os OSAL headers (Os/*.hpp). +// - Only forward-declare types and define the Os::Mutex alias. +// - OS_MUTEX_HEADER should point to the implementation header, which will +// be included AFTER MutexInterface.hpp in Os/Mutex.hpp. +// - Violating this constraint will create circular dependencies. +// ====================================================================== +#ifndef CONFIG_OS_DELEGATEMUTEX_HPP +#define CONFIG_OS_DELEGATEMUTEX_HPP + +//!< Forward declaration of the link-time delegate +//!< Default: select the Mutex implementation at link time +namespace Os { + +class DelegateMutex; +using Mutex = DelegateMutex; + +} // namespace Os + +#define OS_MUTEX_HEADER + +#endif // CONFIG_OS_DELEGATEMUTEX_HPP diff --git a/docs/how-to/integrate/implement-osal.md b/docs/how-to/integrate/implement-osal.md index 9c128e22de7..bbcad11337f 100644 --- a/docs/how-to/integrate/implement-osal.md +++ b/docs/how-to/integrate/implement-osal.md @@ -65,7 +65,7 @@ Create `FprimeMyOs/Os/Mutex.hpp`: #ifndef MYOS_MUTEX_HPP #define MYOS_MUTEX_HPP -#include +#include // the interface + Status; Os/Mutex.hpp is only the alias/aggregation header #include // MyOs native mutex API namespace Os { @@ -98,7 +98,7 @@ class MyOsMutex : public MutexInterface { ``` > [!TIP] -> Look at each interface header in `Os/` (e.g., `Os/Mutex.hpp`, `Os/File.hpp`, `Os/Task.hpp`) to see the exact set of pure virtual methods that need to be implemented for each. Each interface also defines a `Status` enum — your implementation must return the appropriate [status values](../../../Os/docs/sdd.md#24-error-handling). +> Look at each interface header in `Os/` (e.g., `Os/MutexInterface.hpp`, `Os/File.hpp`, `Os/Task.hpp`) to see the exact set of pure virtual methods that need to be implemented for each. Each interface also defines a `Status` enum — your implementation must return the appropriate [status values](../../../Os/docs/sdd.md#24-error-handling). Note: for services that support compile-time selection, the pure-virtual contract lives in the `*Interface.hpp` header (e.g. `Os/MutexInterface.hpp`, `Os/RawTimeInterface.hpp`), while `Os/Mutex.hpp` / `Os/RawTime.hpp` are thin alias/aggregation headers. ### 2.2 — Implement the Methods