Merge pull request #2755 from MRtrix3/dev_threading_fixes

dev threading fixes

core/algo/threaded_loop.h

@@ -20,6 +20,7 @@
 #include "algo/iterator.h"
 #include "algo/loop.h"
 #include "debug.h"
+#include "mutexprotected.h"
 #include "thread.h"
 #include <tuple>
 
@@ -316,37 +317,47 @@ template <class OuterLoopType> struct ThreadedLoopRunOuter {
       return;
     }
 
-    std::mutex mutex;
     ProgressBar::SwitchToMultiThreaded progress_functions;
 
     struct Shared {
+      Shared(const Shared &) = delete;
+      Shared(Shared &&) = delete;
+      Shared &operator=(const Shared &) = delete;
+      Shared &operator=(Shared &&) = delete;
+      ~Shared() = default;
       Iterator &iterator;
       decltype(outer_loop(iterator)) loop;
-      std::mutex &mutex;
       FORCE_INLINE bool next(Iterator &pos) {
-        std::lock_guard<std::mutex> lock(mutex);
         if (loop) {
           assign_pos_of(iterator, loop.axes).to(pos);
           ++loop;
           return true;
         } else
           return false;
       }
-    } shared = {iterator, outer_loop(iterator), mutex};
+    };
+
+    MutexProtected<Shared> shared = {iterator, outer_loop(iterator)};
 
     struct PerThread {
-      Shared &shared;
+      MutexProtected<Shared> &shared;
+      PerThread(const PerThread &) = default;
+      PerThread(PerThread &&) noexcept = default;
+      PerThread &operator=(const PerThread &) = delete;
+      PerThread &operator=(PerThread &&) = delete;
+      ~PerThread() = default;
       typename std::remove_reference<Functor>::type func;
       void execute() {
-        Iterator pos = shared.iterator;
-        while (shared.next(pos))
+        auto pos = shared.lock()->iterator;
+        while (shared.lock()->next(pos))
           func(pos);
       }
     } loop_thread = {shared, functor};
 
     auto threads = Thread::run(Thread::multi(loop_thread), "loop threads");
 
-    __manage_progress(&shared.loop, &threads);
+    auto *loop = &(shared.lock()->loop);
+    __manage_progress(loop, &threads);
     threads.wait();
   }
 

core/mutexprotected.h

@@ -0,0 +1,68 @@
+/* Copyright (c) 2008-2024 the MRtrix3 contributors.
+ *
+ * This Source Code Form is subject to the terms of the Mozilla Public
+ * License, v. 2.0. If a copy of the MPL was not distributed with this
+ * file, You can obtain one at http://mozilla.org/MPL/2.0/.
+ *
+ * Covered Software is provided under this License on an "as is"
+ * basis, without warranty of any kind, either expressed, implied, or
+ * statutory, including, without limitation, warranties that the
+ * Covered Software is free of defects, merchantable, fit for a
+ * particular purpose or non-infringing.
+ * See the Mozilla Public License v. 2.0 for more details.
+ *
+ * For more details, see http://www.mrtrix.org/.
+ */
+
+#ifndef MUTEXPROTECTED_H
+#define MUTEXPROTECTED_H
+
+#include <mutex>
+
+// This class is a wrapper around an object that provides a mutex-protected
+// interface to the object. The object is constructed in place, and the
+// constructor forwards its arguments to the constructor of the object.
+// The lock() method returns a Guard object that provides access to the
+// object. The Guard object locks the mutex in its constructor and unlocks
+// the mutex in its destructor. This ensures that the mutex is always
+// unlocked when the Guard object goes out of scope, even if an exception
+// is thrown.
+// Usage example:
+// MutexProtected<std::vector<int>> protectedVector;
+// {
+//   auto guard = protectedVector.lock();
+//   guard->push_back(42);
+// }
+
+template <typename Object> class MutexProtected {
+public:
+  // Constructor forwards arguments to the constructor of the object
+  template <typename... Args> MutexProtected(Args &&...args) : m_object{std::forward<Args>(args)...} {}
+
+  // Guard is not copy-constructible or copy-assignable
+  // NOLINTBEGIN(cppcoreguidelines-avoid-const-or-ref-data-members)
+  class Guard {
+  public:
+    Guard(MutexProtected &protectedObject) : m_protectedObject(protectedObject), m_lock(protectedObject.m_mutex) {}
+    Guard(const Guard &) = delete;
+    Guard &operator=(const Guard &) = delete;
+    Guard(Guard &&) noexcept = default;
+    Guard &operator=(Guard &&) noexcept = default;
+    ~Guard() = default;
+
+    Object &operator*() { return m_protectedObject.m_object; }
+    Object *operator->() { return &m_protectedObject.m_object; }
+
+  private:
+    MutexProtected &m_protectedObject;
+    std::lock_guard<std::mutex> m_lock;
+  };
+  // NOLINTEND(cppcoreguidelines-avoid-const-or-ref-data-members)
+
+  Guard lock() { return Guard(*this); }
+
+private:
+  std::mutex m_mutex;
+  Object m_object;
+};
+#endif // MUTEXPROTECTED_H

core/progressbar.h

@@ -217,6 +217,7 @@ template <class TextFunc> FORCE_INLINE void ProgressBar::update(TextFunc &&text_
   if (!show)
     return;
   double time = timer.elapsed();
+  const std::unique_lock<std::mutex> lock(mutex);
   if (increment && _multiplier) {
     if (++current_val >= next_percent) {
       set_text(text_func());
@@ -246,6 +247,7 @@ template <class TextFunc> FORCE_INLINE void ProgressBar::update(TextFunc &&text_
 FORCE_INLINE void ProgressBar::operator++() {
   if (!show)
     return;
+  const std::unique_lock<std::mutex> lock(mutex);
   if (_multiplier) {
     if (++current_val >= next_percent) {
       _value = std::round(current_val / _multiplier);