From 86ca7dc89659a556dcd1b6f954ed8f2799e23e2f Mon Sep 17 00:00:00 2001
From: "Nathaniel J. McClatchey, PhD" <nmcclatchey@users.noreply.github.com>
Date: Sun, 15 Sep 2019 13:29:51 -0700
Subject: [PATCH] Fix fatal bug: Threads must copy arguments. (#59)

Threads were taking arguments by reference, rather than creating copies. This has been altered to the correct behavior, which should fix several fatal (and subtle) errors.
Adds a regression test that will automatically detect similar failures (with high probability).

Recommend immediate update and recompilation of all projects using this library.
---
 mingw.thread.h  | 12 ++++++++----
 tests/tests.cpp | 27 +++++++++++++++++++++++++++
 2 files changed, 35 insertions(+), 4 deletions(-)

diff --git a/mingw.thread.h b/mingw.thread.h
index caf333e..b0fff64 100644
--- a/mingw.thread.h
+++ b/mingw.thread.h
@@ -72,18 +72,22 @@ namespace detail
     template<class Func, typename... Args>
     class ThreadFuncCall
     {
-        typedef std::tuple<Args...> Tuple;
-        Func mFunc;
+        using Tuple = std::tuple<typename std::decay<Args>::type...>;
+        typename std::decay<Func>::type mFunc;
         Tuple mArgs;
 
         template <std::size_t... S>
         void callFunc(detail::IntSeq<S...>)
         {
-            detail::invoke(std::forward<Func>(mFunc), std::get<S>(std::forward<Tuple>(mArgs)) ...);
+//  Note: Only called once (per thread)
+            detail::invoke(std::move(mFunc), std::move(std::get<S>(mArgs)) ...);
         }
     public:
         ThreadFuncCall(Func&& aFunc, Args&&... aArgs)
-        :mFunc(std::forward<Func>(aFunc)), mArgs(std::forward<Args>(aArgs)...){}
+          : mFunc(std::forward<Func>(aFunc)),
+            mArgs(std::forward<Args>(aArgs)...)
+        {
+        }
 
         void callFunc()
         {
diff --git a/tests/tests.cpp b/tests/tests.cpp
index b4cb7ac..2e47add 100644
--- a/tests/tests.cpp
+++ b/tests/tests.cpp
@@ -312,6 +312,33 @@ int main()
         std::hash<thread::id> hasher;
         std::cout << "Hash:\t" << hasher(this_thread::get_id()) << "\n";
     }
+
+//  Regression test: Thread must copy any argument that is passed by value.
+    {
+        std::vector<std::thread> loop_threads;
+        std::atomic<int> i_vals_touched [4];// { 0, 0, 0, 0 };
+        for (int i = 0; i < 4; ++i)
+            i_vals_touched[i].store(0, std::memory_order_relaxed);
+        for (int i = 0; i < 4; ++i)
+        {
+            loop_threads.push_back(std::thread([&](int c)
+                {
+                    log("For-loop test thread got value: %i", c);
+                    i_vals_touched[c].fetch_add(1, std::memory_order_relaxed);
+                }, i));
+        }
+        for (std::thread & thr : loop_threads)
+            thr.join();
+        for (int i = 0; i < 4; ++i)
+        {
+            if (i_vals_touched[i] != 1)
+            {
+                log("FATAL: Threads are not copying arguments!");
+                return 1;
+            }
+        }
+    }
+
     std::thread t([](TestMove&& a, const char* b, int c) mutable
     {
         try