From 7c4471f405a2ce97aebcb448b51de60bb9e0dbc7 Mon Sep 17 00:00:00 2001 From: luiscastela Date: Mon, 25 May 2026 17:33:58 +0100 Subject: [PATCH 1/2] Decouple lock types from LockStrategies header Move concrete lock type definitions out of LockStrategies.h and into a separate lock_types.h header. This allows platform integrations to provide ScopedCoreLock and ScopedECULock at build time. --- .../middleware/concurrency/LockStrategies.h | 51 +++---------------- libs/bsw/middleware/test/CMakeLists.txt | 3 ++ .../middleware/concurrency/lock_types.h | 37 ++++++++++++++ .../concurrency/ConcurrencyDefinitions.cpp | 10 ---- 4 files changed, 47 insertions(+), 54 deletions(-) create mode 100644 libs/bsw/middleware/test/include/middleware/concurrency/lock_types.h diff --git a/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h b/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h index 44ecc782f6b..47c67efa474 100644 --- a/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h +++ b/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h @@ -2,57 +2,20 @@ #pragma once -#include +// Platform integration provides ScopedCoreLock and ScopedECULock in the +// middleware::concurrency::integration namespace via lock_types.h. +// The concrete types will be provided at build time. +#include namespace middleware::concurrency { /** - * Suspend all interrupts. - * Platform-specific function that suspends all interrupts to ensure critical sections - * are protected. This is used in conjunction with lock strategies for thread-safe operations. - * The implementation must be provided for each platform integration. + * \brief Suspends all interrupts for the current core */ extern void suspendAllInterrupts(); -/** - * Scoped lock for single-core protection. - * RAII-style lock that protects critical sections within a single core by disabling - * interrupts or using other single-core synchronization mechanisms. The lock is acquired in the - * constructor and automatically released in the destructor, ensuring proper cleanup even in the - * presence of exceptions. - * The implementation must be provided for each platform integration. - */ -struct ScopedCoreLock -{ - /** Acquires the single-core lock. */ - ScopedCoreLock(); - - /** Releases the single-core lock. */ - ~ScopedCoreLock(); - - ScopedCoreLock(ScopedCoreLock const&) = delete; - ScopedCoreLock& operator=(ScopedCoreLock const&) = delete; -}; - -/** - * Scoped lock for ECU-wide (multi-core) protection. - * RAII-style lock that protects critical sections across multiple cores in an ECU by - * using hardware-supported spinlocks or other multi-core synchronization mechanisms. The lock is - * acquired in the constructor and automatically released in the destructor, ensuring proper - * cleanup even in the presence of exceptions. - * The implementation must be provided for each platform integration. - */ -struct ScopedECULock -{ - /** Acquires the ECU-wide lock using \p lock. */ - ScopedECULock(uint8_t volatile* lock); - - /** Releases the ECU-wide lock. */ - ~ScopedECULock(); - - ScopedECULock(ScopedECULock const&) = delete; - ScopedECULock& operator=(ScopedECULock const&) = delete; -}; +using ScopedCoreLock = ::middleware::concurrency::integration::ScopedCoreLock; +using ScopedECULock = ::middleware::concurrency::integration::ScopedECULock; } // namespace middleware::concurrency diff --git a/libs/bsw/middleware/test/CMakeLists.txt b/libs/bsw/middleware/test/CMakeLists.txt index 1c216c70b35..89aa38320a9 100644 --- a/libs/bsw/middleware/test/CMakeLists.txt +++ b/libs/bsw/middleware/test/CMakeLists.txt @@ -24,6 +24,9 @@ add_executable( target_include_directories(middlewareTest PUBLIC src) +target_include_directories(middleware + PUBLIC ${CMAKE_CURRENT_SOURCE_DIR}/include) + target_link_libraries(middlewareTest PRIVATE middleware gmock gtest_main) gtest_discover_tests(middlewareTest PROPERTIES LABELS "middlewareTest") diff --git a/libs/bsw/middleware/test/include/middleware/concurrency/lock_types.h b/libs/bsw/middleware/test/include/middleware/concurrency/lock_types.h new file mode 100644 index 00000000000..df408f81160 --- /dev/null +++ b/libs/bsw/middleware/test/include/middleware/concurrency/lock_types.h @@ -0,0 +1,37 @@ +#pragma once + +#include + +namespace middleware::concurrency::integration +{ + +/** + * No-op scope guard for single-core lock — test/host stub. + */ +struct ScopedCoreLock +{ + ScopedCoreLock() = default; + ~ScopedCoreLock() = default; + + ScopedCoreLock(ScopedCoreLock const&) = delete; + ScopedCoreLock& operator=(ScopedCoreLock const&) = delete; + ScopedCoreLock(ScopedCoreLock&&) = delete; + ScopedCoreLock& operator=(ScopedCoreLock&&) = delete; +}; + +/** + * No-op scope guard for ECU-wide lock — test/host stub. + */ +struct ScopedECULock +{ + explicit ScopedECULock(uint8_t volatile*) {} + + ~ScopedECULock() = default; + + ScopedECULock(ScopedECULock const&) = delete; + ScopedECULock& operator=(ScopedECULock const&) = delete; + ScopedECULock(ScopedECULock&&) = delete; + ScopedECULock& operator=(ScopedECULock&&) = delete; +}; + +} // namespace middleware::concurrency::integration diff --git a/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp b/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp index 4053de26592..8a73111b57d 100644 --- a/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp +++ b/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp @@ -1,16 +1,6 @@ -#include "middleware/concurrency/LockStrategies.h" - namespace middleware::concurrency { void suspendAllInterrupts() {} -ScopedCoreLock::ScopedCoreLock() {} - -ScopedCoreLock::~ScopedCoreLock() {} - -ScopedECULock::ScopedECULock(uint8_t volatile*) {} - -ScopedECULock::~ScopedECULock() {} - } // namespace middleware::concurrency From 1794d861df762bd2562c307ba83f14ccbdca2968 Mon Sep 17 00:00:00 2001 From: luiscastela Date: Wed, 27 May 2026 09:04:05 +0100 Subject: [PATCH 2/2] Remove suspendAllInterrupts from middleware The call to `suspendAllInterrupts()` in `ProxyBase` and `SkeletonBase` was redundant. After logging the cross-thread violation, both call sites unconditionally hit an assert, which is not recoverable. --- .../include/middleware/concurrency/LockStrategies.h | 5 ----- libs/bsw/middleware/src/middleware/core/ProxyBase.cpp | 2 -- libs/bsw/middleware/src/middleware/core/SkeletonBase.cpp | 2 -- libs/bsw/middleware/test/CMakeLists.txt | 1 - .../test/src/concurrency/ConcurrencyDefinitions.cpp | 6 ------ 5 files changed, 16 deletions(-) delete mode 100644 libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp diff --git a/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h b/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h index 47c67efa474..2b8e8fd885a 100644 --- a/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h +++ b/libs/bsw/middleware/interfaces/include/middleware/concurrency/LockStrategies.h @@ -10,11 +10,6 @@ namespace middleware::concurrency { -/** - * \brief Suspends all interrupts for the current core - */ -extern void suspendAllInterrupts(); - using ScopedCoreLock = ::middleware::concurrency::integration::ScopedCoreLock; using ScopedECULock = ::middleware::concurrency::integration::ScopedECULock; diff --git a/libs/bsw/middleware/src/middleware/core/ProxyBase.cpp b/libs/bsw/middleware/src/middleware/core/ProxyBase.cpp index e7ecc60875f..75bcc08dc96 100644 --- a/libs/bsw/middleware/src/middleware/core/ProxyBase.cpp +++ b/libs/bsw/middleware/src/middleware/core/ProxyBase.cpp @@ -139,8 +139,6 @@ void ProxyBase::checkCrossThreadError(uint32_t const initId) const auto const currentTaskId = ::middleware::os::getProcessId(); if (initId != currentTaskId) { - ::middleware::concurrency::suspendAllInterrupts(); - logger::logCrossThreadViolation( logger::LogLevel::Critical, logger::Error::ProxyCrossThreadViolation, diff --git a/libs/bsw/middleware/src/middleware/core/SkeletonBase.cpp b/libs/bsw/middleware/src/middleware/core/SkeletonBase.cpp index 2bce3ba1c1d..e68e9ffcfc8 100644 --- a/libs/bsw/middleware/src/middleware/core/SkeletonBase.cpp +++ b/libs/bsw/middleware/src/middleware/core/SkeletonBase.cpp @@ -169,8 +169,6 @@ void SkeletonBase::checkCrossThreadError(uint32_t const initId) const auto const currentTaskId = ::middleware::os::getProcessId(); if (initId != currentTaskId) { - ::middleware::concurrency::suspendAllInterrupts(); - logger::logCrossThreadViolation( logger::LogLevel::Critical, logger::Error::SkeletonCrossThreadViolation, diff --git a/libs/bsw/middleware/test/CMakeLists.txt b/libs/bsw/middleware/test/CMakeLists.txt index 89aa38320a9..dca67567f6e 100644 --- a/libs/bsw/middleware/test/CMakeLists.txt +++ b/libs/bsw/middleware/test/CMakeLists.txt @@ -1,6 +1,5 @@ add_executable( middlewareTest - src/concurrency/ConcurrencyDefinitions.cpp src/core/ClusterConfigurationTest.cpp src/core/ConnectionTest.cpp src/core/DbManipulatorTest.cpp diff --git a/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp b/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp deleted file mode 100644 index 8a73111b57d..00000000000 --- a/libs/bsw/middleware/test/src/concurrency/ConcurrencyDefinitions.cpp +++ /dev/null @@ -1,6 +0,0 @@ -namespace middleware::concurrency -{ - -void suspendAllInterrupts() {} - -} // namespace middleware::concurrency