From 852462f4e2da4bdfa7f21d1b5478dad0ff5a9ed9 Mon Sep 17 00:00:00 2001
From: Angus Gratton <angus@espressif.com>
Date: Thu, 19 Dec 2019 11:01:06 +1100
Subject: [PATCH] test: Fix possible race in pthread C++ test

Also use TEST_ASSERT_EQUAL to get better debugging

Debugging intermittent UT failures on S2 release config

In the old version, the 300ms delay in between the two kinds of test
was supposed to keep the tasks in lockstep so it didn't matter that
global_sp was protected by two muxes.

However it seems like sometimes they could get out of sync -
I think because of a race in the sleep_until test. If the
second counter ticks over at that exact moment sleeping starts,
then the task doesn't sleep and will immediately keep running
 for the next iteration, possibly racing the other tasks.
---
 components/pthread/test/test_pthread_cxx.cpp | 67 +++++++++++---------
 1 file changed, 36 insertions(+), 31 deletions(-)

diff --git a/components/pthread/test/test_pthread_cxx.cpp b/components/pthread/test/test_pthread_cxx.cpp
index 074b9ba9c..69fe3a241 100644
--- a/components/pthread/test/test_pthread_cxx.cpp
+++ b/components/pthread/test/test_pthread_cxx.cpp
@@ -12,47 +12,50 @@
 #include "esp_log.h"
 const static char *TAG = "pthread_test";
 
-static std::shared_ptr<int> global_sp;
 static std::mutex           mtx;
+static std::shared_ptr<int> global_sp_mtx; // protected by mux
+
 static std::recursive_mutex recur_mtx;
+static std::shared_ptr<int> global_sp_recur_mtx; // protected by recursive mux
 
 static void thread_do_nothing() {}
 
 static void thread_main()
 {
-    int i = 0;
     std::cout << "thread_main CXX " << std::hex <<  std::this_thread::get_id() << std::endl;
-    std::chrono::milliseconds dur = std::chrono::milliseconds(300);
+    std::chrono::milliseconds dur = std::chrono::milliseconds(10);
 
-    while (i < 3) {
-        int old_val, new_val;
+    for (int i = 0; i < 10; i++) {
+        for (int j = 0; j < 10; j++) {
+            int old_val, new_val;
 
-        // mux test
-        mtx.lock();
-        old_val = *global_sp;
-        std::this_thread::yield();
-        (*global_sp)++;
-        std::this_thread::yield();
-        new_val = *global_sp;
-        mtx.unlock();
-        std::cout << "thread " << std::hex << std::this_thread::get_id() << ": " << i++ << " val= " << *global_sp << std::endl;
-        TEST_ASSERT_TRUE(new_val == old_val + 1);
+            // mux test
+            mtx.lock();
+            old_val = *global_sp_mtx;
+            std::this_thread::yield();
+            (*global_sp_mtx)++;
+            std::this_thread::yield();
+            new_val = *global_sp_mtx;
+            mtx.unlock();
+            std::cout << "thread " << std::hex << std::this_thread::get_id() << ": nrec " << i << " val= " << *global_sp_mtx << std::endl;
+            TEST_ASSERT_EQUAL(old_val + 1, new_val);
 
-        // sleep_for test
-        std::this_thread::sleep_for(dur);
+            // sleep_for test
+            std::this_thread::sleep_for(dur);
 
-        // recursive mux test
-        recur_mtx.lock();
-        recur_mtx.lock();
-        old_val = *global_sp;
-        std::this_thread::yield();
-        (*global_sp)++;
-        std::this_thread::yield();
-        new_val = *global_sp;
-        recur_mtx.unlock();
-        recur_mtx.unlock();
-        std::cout << "thread " << std::hex << std::this_thread::get_id() << ": " << i++ << " val= " << *global_sp << std::endl;
-        TEST_ASSERT_TRUE(new_val == old_val + 1);
+            // recursive mux test
+            recur_mtx.lock();
+            recur_mtx.lock();
+            old_val = *global_sp_recur_mtx;
+            std::this_thread::yield();
+            (*global_sp_recur_mtx)++;
+            std::this_thread::yield();
+            new_val = *global_sp_recur_mtx;
+            recur_mtx.unlock();
+            recur_mtx.unlock();
+            std::cout << "thread " << std::hex << std::this_thread::get_id() << ": rec " << i << " val= " << *global_sp_recur_mtx << std::endl;
+            TEST_ASSERT_EQUAL(old_val + 1, new_val);
+        }
 
         // sleep_until test
         using std::chrono::system_clock;
@@ -65,7 +68,8 @@ static void thread_main()
 
 TEST_CASE("pthread C++", "[pthread]")
 {
-    global_sp.reset(new int(1));
+    global_sp_mtx.reset(new int(1));
+    global_sp_recur_mtx.reset(new int(-1000));
 
     std::thread t1(thread_do_nothing);
     t1.join();
@@ -86,7 +90,8 @@ TEST_CASE("pthread C++", "[pthread]")
         t4.join();
     }
 
-    global_sp.reset(); // avoid reported leak
+    global_sp_mtx.reset(); // avoid reported leak
+    global_sp_recur_mtx.reset();
 }
 
 static void task_test_sandbox()