From 92e1d84fc73358120f303e22a05b261db948e849 Mon Sep 17 00:00:00 2001
From: Lysann Tranvouez <lysann.tranvouez@gmail.com>
Date: Fri, 21 Nov 2025 13:16:01 +0100
Subject: [PATCH] fix tests (oops) and also make them more robust

---
 tests/test_threads.cpp | 54 +++++++++++++++++++++++++++++++++---------
 1 file changed, 43 insertions(+), 11 deletions(-)

diff --git a/tests/test_threads.cpp b/tests/test_threads.cpp
index 11af631..f6d109f 100644
--- a/tests/test_threads.cpp
+++ b/tests/test_threads.cpp
@@ -1,5 +1,6 @@
 // Copyright (c) Lysann Tranvouez.  All rights reserved.
 
+#include <algorithm>
 #include <catch2/catch_test_macros.hpp>
 
 #include <mach_detours.h>
@@ -29,33 +30,62 @@ static const char* localFunctionDetour()
 }
 static const char* (*realLocalFunction)() = localFunction;
 
+struct threadData
+{
+    std::atomic<int> loopCounter;
+    bool cancel = false;
+};
+
 static void* threadFunc(void* pUserArg)
 {
-    const bool* pCancel = static_cast<const bool*>(pUserArg);
-    while (!*pCancel) {
+    auto& data = *static_cast<threadData*>(pUserArg);
+    while (!data.cancel) {
         localFunction();
         localFunction();
         localFunction();
         localFunction();
         localFunction();
+        ++data.loopCounter;
     }
     return nullptr;
 }
 
+template<size_t numThreads>
+bool resetLoopCountersAndWaitForLoop(std::array<threadData, numThreads>& threadDatas, std::optional<std::chrono::seconds> timeout = std::nullopt)
+{
+    for (auto i = 0; i < numThreads; i++) {
+        threadDatas[i].loopCounter = 0;
+    }
+
+    const auto endTime = timeout ? std::make_optional(std::chrono::steady_clock::now() + *timeout) : std::nullopt;
+    while (true) {
+        // we check for >= 2 to be sure we did a full loop - we don't want to execute just the line incrementing the counter!
+        const bool anyLooped = std::ranges::any_of(threadDatas, [](const threadData& data){ return data.loopCounter >= 2; });
+        if (anyLooped) {
+            return true;
+        }
+        if (endTime && std::chrono::steady_clock::now() > *endTime) {
+            return false;
+        }
+        usleep( 100 );
+    }
+}
+
 TEST_CASE( "Handling other threads", "[attach][local][threads]" )
 {
-    bool cancelThreads = false;
+    using namespace std::chrono_literals;
 
     constexpr auto numThreads = 150;
     pthread_t threads[numThreads];
+    std::array<threadData, numThreads> threadDatas{};
     for (auto i = 0; i < numThreads; i++) {
-        pthread_create(&threads[i], nullptr, threadFunc, (void*)&cancelThreads);
+        pthread_create(&threads[i], nullptr, threadFunc, (void*)&threadDatas[i]);
     }
 
     SECTION( "running threads modify a value" )
     {
         const int saved_b = g_b;
-        usleep( 1000 ); // let the threads update g_b a bit
+        resetLoopCountersAndWaitForLoop(threadDatas);
         CHECK( saved_b != g_b );
     }
 
@@ -79,13 +109,13 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" )
         }
 
         int saved_b = g_b;
-        usleep( 1000 ); // let any running thread update g_b a bit
+        resetLoopCountersAndWaitForLoop(threadDatas, 1s);
         CHECK( saved_b == g_b );
 
         CHECK( detour_transaction_commit() == err_none );
 
         saved_b = g_b;
-        usleep( 1000 ); // let the threads update g_b a bit
+        resetLoopCountersAndWaitForLoop(threadDatas);
         CHECK( saved_b != g_b );
     }
 
@@ -95,7 +125,7 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" )
         CHECK( detour_transaction_abort() == err_none );
 
         const int saved_b = g_b;
-        usleep( 1000 ); // let the threads update g_b a bit
+        resetLoopCountersAndWaitForLoop(threadDatas);
         CHECK( saved_b != g_b );
     }
 
@@ -108,15 +138,17 @@ TEST_CASE( "Handling other threads", "[attach][local][threads]" )
         CHECK( detour_attach_and_commit(reinterpret_cast<detour_func_t*>(&realLocalFunction), reinterpret_cast<detour_func_t>(localFunctionDetour)) == err_none );
 
         const int saved_b = g_b;
-        usleep( 1000 ); // let the threads update g_b a bit
-        CHECK( saved_b != g_b );
+        resetLoopCountersAndWaitForLoop(threadDatas);
+        CHECK( saved_b == g_b );
 
         // clean up
         CHECK( detour_transaction_begin_managed() == err_none );
         CHECK( detour_detach_and_commit(reinterpret_cast<detour_func_t*>(&realLocalFunction), reinterpret_cast<detour_func_t>(localFunctionDetour)) == err_none );
     }
 
-    cancelThreads = true;
+    for (auto i = 0; i < numThreads; i++) {
+        threadDatas[i].cancel = true;
+    }
     for (auto i = 0; i < numThreads; i++) {
         pthread_join(threads[i], nullptr);
     }