[libcxx-commits] [PATCH] D143816: [libc++] Fixes a flaky test.

[Mark de Wever via Phabricator via libcxx-commits]

This revision was automatically updated to reflect the committed changes.
Mordante marked an inline comment as done.
Closed by commit rG261d21f8c375: [libc++] Fixes a flaky test. (authored by Mordante).

Changed prior to commit:
  https://reviews.llvm.org/D143816?vs=497726&id=498409#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D143816/new/

https://reviews.llvm.org/D143816

Files:
  libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp


Index: libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp
===================================================================
--- libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp
+++ libcxx/test/std/thread/thread.condition/notify_all_at_thread_exit_lwg3343.pass.cpp
@@ -8,11 +8,15 @@
 //
 // UNSUPPORTED: no-threads
 
-// ALLOW_RETRIES: 3
-
 // notify_all_at_thread_exit(...) requires move semantics to transfer the unique_lock.
 // UNSUPPORTED: c++03
 
+// The fix of LWG3343 is done in the dylib. That means Apple backdeployment
+// targets remain broken. Due to the nature of the test, testing on a broken
+// system does not guarantee that the test fails, so the test can't use XFAIL.
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx10.{{.+}}
+// UNSUPPORTED: use_system_cxx_lib && target={{.+}}-apple-macosx11.{{.+}}
+
 // This is a regression test for LWG3343.
 //
 // <condition_variable>
@@ -29,6 +33,8 @@
 #include <mutex>
 #include <thread>
 
+int condition_variable_lock_skipped_counter = 0;
+
 union X {
     X() : cv_() {}
     ~X() {}
@@ -46,11 +52,13 @@
 
     for (int i = 0; i < N; ++i) {
         std::thread t = support::make_test_thread([&] {
-            // Signal thread completion
+            // Emulate work being done.
+            std::this_thread::sleep_for(std::chrono::milliseconds(1));
+
+            // Signal thread completion.
             std::unique_lock<std::mutex> lk(m);
             --threads_active;
             std::notify_all_at_thread_exit(x.cv_, std::move(lk));
-            std::this_thread::sleep_for(std::chrono::milliseconds(1));
         });
         t.detach();
     }
@@ -62,6 +70,15 @@
     // unlocks `m`.
     {
         std::unique_lock<std::mutex> lk(m);
+        // Due to OS scheduling the workers might have terminated when this
+        // code is reached. In that case the wait will not sleep and the call
+        // to notify_all_at_thread_exit has no effect; the condition variable
+        // will not be used here.
+        //
+        // Keep track of how often that happens, if too often the test needs
+        // to be improved.
+        if(threads_active == 0)
+            ++condition_variable_lock_skipped_counter;
         x.cv_.wait(lk, [&]() { return threads_active == 0; });
     }
 
@@ -92,5 +109,9 @@
         test();
     }
 
+    // The threshold is arbitrary, it just makes sure the notification is
+    // tested a reasonable number of times.
+    assert(condition_variable_lock_skipped_counter < 250);
+
     return 0;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D143816.498409.patch
Type: text/x-patch
Size: 2591 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20230217/1d98b1af/attachment.bin>