[1.0.4][r] fix potential multithreading issues

CHANGELOG.md

@@ -1,6 +1,15 @@
 # Changelog
 All notable changes to project will be documented in this file.
 
+## [1.0.4] - 2026-04-06
+### Fixed
+- Fixed potential race-conditions in dispatchers
+- Replaced copy with move operations in hsm 
+
+## [1.0.3] - 2026-04-06
+### Fixed
+- Fixed unused parameter compiler warnings/errors in generated code
+
 ## [1.0.2] - 2024-05-31
 ### Fixed
 - fixed handling of <xi:include> tags during code generation (transition targets were getting double prefix which was breaking the build)

CMakeLists.txt

@@ -3,7 +3,7 @@
 cmake_minimum_required(VERSION 3.16)
 project(hsmcpp)
 
-set(PROJECT_VERSION "1.0.2")
+set(PROJECT_VERSION "1.0.4")
 set(PROJECT_DESCRIPTION "C++ library for hierarchical state machines / finite state machines. Provides a code-free visual approach for defining state machine logic using GUI editors with automatic code and diagram generation. Check out https://hsmcpp.readthedocs.io for detailed documentation.")
 set(CMAKE_VERBOSE_MAKEFILE OFF)
 

README.md

@@ -1,5 +1,5 @@
 [![MIT License](https://img.shields.io/badge/license-MIT-blue.svg)](https://github.com/igor-krechetov/hsmcpp/blob/main/LICENSE)
-[![Changelog](https://img.shields.io/badge/changelog-v1.0.2-green.svg)](https://github.com/igor-krechetov/hsmcpp/blob/main/CHANGELOG.md)
+[![Changelog](https://img.shields.io/badge/changelog-v1.0.4-green.svg)](https://github.com/igor-krechetov/hsmcpp/blob/main/CHANGELOG.md)
 [![Documentation Status](https://readthedocs.org/projects/hsmcpp/badge/?version=latest)](https://hsmcpp.readthedocs.io/en/latest/?badge=latest)
 
 # Releases

include/hsmcpp/hsm.hpp

@@ -7,6 +7,7 @@
 #include <list>
 #include <memory>
 #include <string>
+#include <utility>
 
 #include "HsmTypes.hpp"
 #include "variant.hpp"
@@ -734,7 +735,7 @@ void HierarchicalStateMachine::registerState(const StateID_t state,
         }
     }
 
-    registerState(state, funcStateChanged, funcEntering, funcExiting);
+    registerState(state, std::move(funcStateChanged), std::move(funcEntering), std::move(funcExiting));
 }
 
 template <class HsmHandlerClass>
@@ -762,7 +763,7 @@ void HierarchicalStateMachine::registerFinalState(const StateID_t state,
         }
     }
 
-    registerFinalState(state, event, funcStateChanged, funcEntering, funcExiting);
+    registerFinalState(state, event, std::move(funcStateChanged), std::move(funcEntering), std::move(funcExiting));
 }
 
 template <class HsmHandlerClass>
@@ -780,7 +781,7 @@ void HierarchicalStateMachine::registerHistory(const StateID_t parent,
         }
     }
 
-    registerHistory(parent, historyState, type, defaultTarget, funcTransitionCallback);
+    registerHistory(parent, historyState, type, defaultTarget, std::move(funcTransitionCallback));
 }
 
 template <class HsmHandlerClass>
@@ -797,7 +798,7 @@ bool HierarchicalStateMachine::registerSubstateEntryPoint(const StateID_t parent
         condition = std::bind(conditionCallback, handler, std::placeholders::_1);
     }
 
-    return registerSubstateEntryPoint(parent, substate, onEvent, condition, expectedConditionValue);
+    return registerSubstateEntryPoint(parent, substate, onEvent, std::move(condition), expectedConditionValue);
 }
 
 template <typename... Args>
@@ -832,7 +833,7 @@ void HierarchicalStateMachine::registerTransition(const StateID_t fromState,
         }
     }
 
-    registerTransition(fromState, toState, onEvent, funcTransitionCallback, funcConditionCallback, expectedConditionValue);
+    registerTransition(fromState, toState, onEvent, std::move(funcTransitionCallback), std::move(funcConditionCallback), expectedConditionValue);
 }
 
 template <class HsmHandlerClass>
@@ -856,7 +857,7 @@ void HierarchicalStateMachine::registerSelfTransition(const StateID_t state,
         }
     }
 
-    registerSelfTransition(state, onEvent, type, funcTransitionCallback, funcConditionCallback, expectedConditionValue);
+    registerSelfTransition(state, onEvent, type, std::move(funcTransitionCallback), std::move(funcConditionCallback), expectedConditionValue);
 }
 
 template <typename... Args>

include/hsmcpp/os/common/UniqueLock.hpp

@@ -3,6 +3,8 @@
 #ifndef HSMCPP_OS_COMMON_UNIQUELOCK_HPP
 #define HSMCPP_OS_COMMON_UNIQUELOCK_HPP
 
+#include <atomic>
+
 namespace hsmcpp
 {
 
@@ -24,7 +26,13 @@ class UniqueLock
 
     inline bool owns_lock() const noexcept
     {
-        return mOwnsLock;
+        const bool isLocked = const_cast<std::atomic_flag&>(mOwnsLock).test_and_set(std::memory_order_acquire);
+
+        if (false == isLocked) {
+            const_cast<std::atomic_flag&>(mOwnsLock).clear(std::memory_order_release);
+        }
+
+        return isLocked;
     }
 
     inline explicit operator bool() const noexcept
@@ -44,7 +52,7 @@ class UniqueLock
 
 private:
     Mutex* mSync = nullptr;
-    bool mOwnsLock = false;
+    mutable std::atomic_flag mOwnsLock = ATOMIC_FLAG_INIT;
 };
 
 } // namespace hsmcpp

scripts/local/validate.sh

@@ -1,11 +1,5 @@
 #!/bin/sh
 
-# python3 ./scripts/local/validate_metadata.py $1 ./
-# if [ $? != 0 ]
-# then
-#     exit 1
-# fi
-
 mkdir ./build
 cd ./build
 
@@ -67,8 +61,7 @@ then
           -DHSMBUILD_CLANGTIDY=OFF \
           ..
 
-    cppcheck --addon=../scripts/sca/cppcheck/misra.json \
-         --enable=warning,performance,portability,information \
+    cppcheck --enable=warning,performance,portability,information \
          --inline-suppr \
          --xml \
          --suppressions-list=../scripts/sca/cppcheck/cppcheck_suppress.txt --project=./compile_commands.json \

src/HsmEventDispatcherBase.cpp

@@ -207,6 +207,7 @@ void HsmEventDispatcherBase::stopTimer(const TimerID_t timerID) {
 }
 
 bool HsmEventDispatcherBase::isTimerRunning(const TimerID_t timerID) {
+    LockGuard lck(mHandlersSync);
     return (mActiveTimers.find(timerID) != mActiveTimers.end());
 }
 

src/HsmEventDispatcherQt.cpp

@@ -95,9 +95,12 @@ void HsmEventDispatcherQt::startTimerImpl(const TimerID_t timerID, const unsigne
                               SC2INT(timerID),
                               intervalMs,
                               BOOL2INT(isSingleShot));
+    mRunningTimersSync.lock();
     auto it = mNativeTimerHandlers.find(timerID);
+    const bool timerDoesntExist = (mNativeTimerHandlers.end() == it);
+    mRunningTimersSync.unlock();
 
-    if (mNativeTimerHandlers.end() == it) {
+    if (timerDoesntExist) {
         auto funcCreateTimer = [=]() {
             QTimer* newTimer = new QTimer(this);
 

src/HsmEventDispatcherSTD.cpp

@@ -82,7 +82,6 @@ void HsmEventDispatcherSTD::join() {
 
 void HsmEventDispatcherSTD::startTimerImpl(const TimerID_t timerID, const unsigned int intervalMs, const bool isSingleShot) {
     HSM_TRACE_CALL_ARGS("timerID=%d, intervalMs=%d, isSingleShot=%d", SC2INT(timerID), intervalMs, BOOL2INT(isSingleShot));
-    auto it = mRunningTimers.end();
 
     // lazy initialization of timers thread
     if (false == mTimersThread.joinable()) {
@@ -91,7 +90,7 @@ void HsmEventDispatcherSTD::startTimerImpl(const TimerID_t timerID, const unsign
 
     {
         CriticalSection cs(mRunningTimersSync);
-        it = mRunningTimers.find(timerID);
+        auto it = mRunningTimers.find(timerID);
 
         // new timer
         if (mRunningTimers.end() == it) {
@@ -144,6 +143,8 @@ void HsmEventDispatcherSTD::doDispatching() {
                 // cppcheck-suppress misra-c2012-15.5
                 mEmitEvent.wait(lck, [=]() {
                     // cppcheck-suppress misra-c2012-15.5 ; false-positive. "return" statement belongs to lambda function
+                    // NOTE: it's assumed that calling empty() is thread-safe. Even if due to a race condition we get
+                    //       wrong value it will only cause a small delay in event processing, but won't cause any critical issues
                     return (false == mPendingEvents.empty()) || (false == mEnqueuedEvents.empty()) || (true == mStopDispatcher);
                 });
                 HSM_TRACE_DEBUG("woke up. pending events=%lu", mPendingEvents.size());
@@ -168,6 +169,9 @@ void HsmEventDispatcherSTD::handleTimers() {
     while (false == mStopDispatcher) {
         mRunningTimersSync.lock();
 
+        // NOTE: it's assumed that calling empty() is thread-safe. Even if due to
+        //       a race condition we get wrong value it will only cause a small delay in timer processing,
+        //       but won't cause any critical issues
         if (false == mRunningTimers.empty()) {
             auto itTimeout = mRunningTimers.begin();
 

src/HsmImpl.cpp

@@ -1490,7 +1490,7 @@ bool HierarchicalStateMachine::Impl::processFinalStateTransition(const PendingEv
 
                 {
                     HSM_SYNC_EVENTS_QUEUE();
-                    mPendingEvents.push_front(finalStateEvent);
+                    mPendingEvents.push_front(std::move(finalStateEvent));
                 }
             }
         }

src/os/common/UniqueLock.cpp

@@ -7,8 +7,8 @@
 namespace hsmcpp {
 
 UniqueLock::UniqueLock(Mutex& sync)
-// NOTE: false-positive. thinks that ':' is arithmetic operation
-// cppcheck-suppress misra-c2012-10.4
+    // NOTE: false-positive. thinks that ':' is arithmetic operation
+    // cppcheck-suppress misra-c2012-10.4
     : mSync(&sync) {
     lock();
 }
@@ -18,45 +18,64 @@ UniqueLock::~UniqueLock() {
 }
 
 UniqueLock::UniqueLock(UniqueLock&& src) noexcept
-    : mSync(src.mSync)
-    , mOwnsLock(src.mOwnsLock) {
+    : mSync(src.mSync) {
+    if (true == src.mOwnsLock.test_and_set(std::memory_order_acquire)) {
+        mOwnsLock.test_and_set(std::memory_order_release);
+    } else {
+        mOwnsLock.clear(std::memory_order_release);
+    }
+
     src.mSync = nullptr;
-    src.mOwnsLock = false;
+    src.mOwnsLock.clear(std::memory_order_release);
 }
 
 UniqueLock& UniqueLock::operator=(UniqueLock&& src) noexcept {
-    if (mOwnsLock) {
-        unlock();
-    }
+    if (this != &src) {
+        if (nullptr != mSync) {
+            if (true == mOwnsLock.test_and_set(std::memory_order_acquire)) {
+                mSync->unlock();
+            } else {
+                mOwnsLock.clear(std::memory_order_release);
+            }
+        }
 
-    mSync = src.mSync;
-    mOwnsLock = src.mOwnsLock;
+        mSync = src.mSync;
 
-    src.mSync = nullptr;
-    src.mOwnsLock = false;
+        if (true == src.mOwnsLock.test_and_set(std::memory_order_acquire)) {
+            mOwnsLock.test_and_set(std::memory_order_release);
+        } else {
+            mOwnsLock.clear(std::memory_order_release);
+        }
+
+        src.mSync = nullptr;
+        src.mOwnsLock.clear(std::memory_order_release);
+    }
 
     return *this;
 }
 
 void UniqueLock::lock() {
-    if ((nullptr != mSync) && (false == mOwnsLock)) {
+    if ((nullptr != mSync) && (false == mOwnsLock.test_and_set(std::memory_order_acq_rel))) {
         mSync->lock();
-        mOwnsLock = true;
     }
 }
 
 void UniqueLock::unlock() {
-    if ((nullptr != mSync) && (true == mOwnsLock)) {
-        mSync->unlock();
-        mOwnsLock = false;
+    if (nullptr != mSync) {
+        if (true == mOwnsLock.test_and_set(std::memory_order_acquire)) {
+            mSync->unlock();
+            mOwnsLock.clear(std::memory_order_release);
+        } else {
+            mOwnsLock.clear(std::memory_order_release);
+        }
     }
 }
 
 Mutex* UniqueLock::release() noexcept {
     Mutex* res = mSync;
 
     mSync = nullptr;
-    mOwnsLock = false;
+    mOwnsLock.clear(std::memory_order_release);
 
     return res;
 }