Merge pull request #77 from SLM-Audio/syl/thread-test-changes

Thread refactor

Author: MeijisIrlnd

include/mostly_harmless/utils/mostlyharmless_TaskThread.h

@@ -10,21 +10,74 @@
 #include <thread>
 
 namespace mostly_harmless::utils {
+    /***
+     * \brief Convenience wrapper around std::thread
+     *
+     * Contains some simple signalling mechanisms, and forms the basis for `Timer` - to use it, supply a lambda to the `action` field, then call perform (and remember to call `stop` to join the thread!)
+     */
     class TaskThread {
     public:
+        /**
+         * Destructor. Calls stop, which joins the internal thread, so will block if the thread is running!
+         */
         ~TaskThread() noexcept;
-        void perform();
-        void stop(bool join) noexcept;
-        void sleep();
-        void wake();
-        [[nodiscard]] bool isThreadRunning() const noexcept;
+
+        /**
+         * Performs the `action` lambda (if it's been set) on a std::thread, and sets isThreadRunning to false on scope exit.
+         */
+        auto perform() -> void;
+
+        /**
+         * Calls signalStop(), joins the thread, and resets the internal state for reuse.
+         */
+        auto stop() noexcept -> void;
+
+        /**
+         * For use in your `action` lambda, pauses the thread's execution until it's woken via wake().
+         * Internally just uses a `std::condition_variable` and an atomic "canWakeUp" bool - this call is equivalent to std::condition_variable::wait().
+         * Note that this doesn't actually do anything on its own unless you call `sleep` in your action - it's just there if you need it!
+         */
+        auto sleep() -> void;
+
+        /**
+         * Paired with sleep, sets the `canWakeUp` bool to true, and notifies the underlying condition variable. All ramblings from wake()` also apply here.
+         */
+        auto wake() -> void;
+
+        /**
+         * Sets an internal atomic bool that the thread should exit. Like sleep() and wake(), doesn't actually do anything on its own,
+         * pair this with usage of hasSignalledStop() in your custom thread action.
+         */
+        auto signalStop() -> void;
+
+        /**
+         * Retrieves the state of the atomic bool described in signalStop() - use this in your custom thread action to respond to cancellation requests if needed (again, does nothing on it's own!)
+         * @return The state of the `should_stop` bool.
+         */
+        [[nodiscard]] auto hasSignalledStop() const noexcept -> bool;
+
+        /**
+         * Gives a loose indication of whether the thread is still running or not - loose, because this will only get set *after* the user action has been run, not within.
+         * This means there may be a few cpu cycles discrepancy between when you *think* this should return true and when it *actually* returns true.
+         * TLDR, don't rely on it for anything timing critical!
+         * @return Whether the thread is running or not.
+         */
+        [[nodiscard]] auto isThreadRunning() const noexcept -> bool;
+
+        /**
+         *  A user-settable lambda to be invoked on the internal std::thread. Put your threaded logic here!!
+         */
         std::function<void(void)> action{ nullptr };
 
     private:
-        std::mutex m_mutex;
+        auto reset() -> void;
+        struct {
+            std::mutex mutex;
+            std::atomic<bool> canWakeUp{ false };
+            std::condition_variable conditionVariable;
+        } m_sleepState;
         std::atomic<bool> m_isThreadRunning{ false };
-        std::atomic<bool> m_canWakeUp{ false };
-        std::condition_variable m_conditionVariable;
+        std::atomic<bool> m_stop{ false };
         std::unique_ptr<std::thread> m_thread{ nullptr };
     };
 } // namespace mostly_harmless::utils

include/mostly_harmless/utils/mostlyharmless_Timer.h

@@ -9,9 +9,9 @@
 namespace mostly_harmless::utils {
     class Timer final {
     public:
-        void run(int intervalMs);
-        void run(double frequency);
-        void stop(bool join);
+        auto run(int intervalMs) -> void;
+        auto run(double frequency) -> void;
+        auto stop() -> void;
         std::function<void(void)> action;
 
     private:

source/mostlyharmless_PluginBase.cpp

@@ -386,7 +386,7 @@ namespace mostly_harmless::internal {
 
     void PluginBase::guiDestroy() noexcept {
         MH_LOG("GUI: guiDestroy()");
-        m_guiDispatchThread.stop(true);
+        m_guiDispatchThread.stop();
         m_editor.reset();
     }
 

source/utils/mostlyharmless_TaskThread.cpp

@@ -1,52 +1,71 @@
 //
 // Created by Syl on 12/08/2024.
 //
+#include "mostly_harmless/utils/mostlyharmless_OnScopeExit.h"
+
+
 #include <mostly_harmless/utils/mostlyharmless_TaskThread.h>
 #include <cassert>
 #include <thread>
 namespace mostly_harmless::utils {
     TaskThread::~TaskThread() noexcept {
-        stop(true);
+        stop();
     }
 
-    void TaskThread::perform() {
+    auto TaskThread::perform() -> void {
         auto expected{ false };
         if (m_isThreadRunning.compare_exchange_strong(expected, true)) {
             auto actionWrapper = [this]() -> void {
+                OnScopeExit se{ [this]() -> void {
+                    m_isThreadRunning.store(false);
+                } };
                 action();
-                m_isThreadRunning = false;
             };
             m_thread = std::make_unique<std::thread>(std::move(actionWrapper));
         }
     }
 
-    void TaskThread::stop(bool join) noexcept {
-        m_isThreadRunning = false;
+    auto TaskThread::stop() noexcept -> void {
+        signalStop();
         if (!m_thread) {
             return;
         }
-        if (join) {
-            if (m_thread->joinable()) {
-                m_thread->join();
-            }
-            m_thread.reset();
+        if (m_thread->joinable()) {
+            m_thread->join();
         }
+        reset();
     }
 
-    void TaskThread::sleep() {
-        m_canWakeUp = false;
-        std::unique_lock<std::mutex> ul{ m_mutex };
-        m_conditionVariable.wait(ul, [this]() -> bool { return m_canWakeUp; });
+    auto TaskThread::sleep() -> void {
+        m_sleepState.canWakeUp = false;
+        std::unique_lock<std::mutex> ul{ m_sleepState.mutex };
+        m_sleepState.conditionVariable.wait(ul, [this]() -> bool { return m_sleepState.canWakeUp; });
     }
 
-    void TaskThread::wake() {
-        m_canWakeUp = true;
-        std::lock_guard<std::mutex> lock{ m_mutex };
-        m_conditionVariable.notify_one();
+    auto TaskThread::wake() -> void {
+        m_sleepState.canWakeUp = true;
+        std::lock_guard<std::mutex> lock{ m_sleepState.mutex };
+        m_sleepState.conditionVariable.notify_one();
     }
 
-    bool TaskThread::isThreadRunning() const noexcept {
+    auto TaskThread::isThreadRunning() const noexcept -> bool {
         return m_isThreadRunning;
     }
 
+    auto TaskThread::signalStop() -> void {
+        m_stop.store(true);
+    }
+
+    auto TaskThread::hasSignalledStop() const noexcept -> bool {
+        return m_stop;
+    }
+
+    auto TaskThread::reset() -> void {
+        m_thread.reset();
+        m_stop = false;
+    }
+
+
+
+
 } // namespace mostly_harmless::utils

source/utils/mostlyharmless_Timer.cpp

@@ -4,11 +4,11 @@
 #include <mostly_harmless/utils/mostlyharmless_Timer.h>
 #include <thread>
 namespace mostly_harmless::utils {
-    void Timer::run(int intervalMs) {
+    auto Timer::run(int intervalMs) -> void {
         if (!action || m_thread.isThreadRunning()) return;
         auto threadAction = [this, intervalMs]() -> void {
             auto startPoint = std::chrono::steady_clock::now();
-            while (m_thread.isThreadRunning()) {
+            while (!m_thread.hasSignalledStop()) {
                 std::this_thread::sleep_for(std::chrono::milliseconds(1));
                 const auto now = std::chrono::steady_clock::now();
                 const auto delta = std::chrono::duration_cast<std::chrono::milliseconds>(now - startPoint);
@@ -21,13 +21,13 @@ namespace mostly_harmless::utils {
         m_thread.perform();
     }
 
-    void Timer::run(double frequency) {
+    auto Timer::run(double frequency) -> void {
         const auto intervalMs = static_cast<int>(1.0 / frequency);
         run(intervalMs);
     }
 
-    void Timer::stop(bool join) {
-        m_thread.stop(join);
+    auto Timer::stop() -> void {
+        m_thread.stop();
     }