[libcxx-commits] [libcxx] varconst/fix flaky atomic wait (PR #70435)

Konstantin Varlamov via libcxx-commits libcxx-commits at lists.llvm.org
Fri Oct 27 02:21:38 PDT 2023


https://github.com/var-const created https://github.com/llvm/llvm-project/pull/70435

- [libc++] Make sure all `jthread` tests use `support::make_test_jthread`.
- clang-format
- [libc++] Fix flakiness in `atomic_notify_all.pass.cpp`


>From 7ae33c19e0d9f68b1d56be0ad5d99337f394a3e1 Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconsteq at gmail.com>
Date: Wed, 25 Oct 2023 17:01:28 -0700
Subject: [PATCH 1/3] [libc++] Make sure all `jthread` tests use
 `support::make_test_jthread`.

---
 .../thread.jthread/cons.func.token.pass.cpp   | 48 ++++++++++---------
 1 file changed, 26 insertions(+), 22 deletions(-)

diff --git a/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp b/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
index 2d029271d75ce84..fafd7818620aa7d 100644
--- a/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
+++ b/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
@@ -19,6 +19,7 @@
 #include <thread>
 #include <type_traits>
 
+#include "make_test_thread.h"
 #include "test_macros.h"
 
 template <class... Args>
@@ -46,7 +47,7 @@ int main(int, char**) {
   // Postconditions: get_id() != id() is true and ssource.stop_possible() is true
   // and *this represents the newly started thread.
   {
-    std::jthread jt{[] {}};
+    std::jthread jt = support::make_test_jthread([] {});
     assert(jt.get_stop_source().stop_possible());
     assert(jt.get_id() != std::jthread::id());
   }
@@ -56,12 +57,13 @@ int main(int, char**) {
   // if that expression is well-formed,
   {
     int result = 0;
-    std::jthread jt{[&result](std::stop_token st, int i) {
-                      assert(st.stop_possible());
-                      assert(!st.stop_requested());
-                      result += i;
-                    },
-                    5};
+    std::jthread jt = support::make_test_jthread(
+        [&result](std::stop_token st, int i) {
+          assert(st.stop_possible());
+          assert(!st.stop_requested());
+          result += i;
+        },
+        5);
     jt.join();
     assert(result == 5);
   }
@@ -70,7 +72,7 @@ int main(int, char**) {
   // invoke(auto(std::forward<F>(f)), auto(std::forward<Args>(args))...)
   {
     int result = 0;
-    std::jthread jt{[&result](int i) { result += i; }, 5};
+    std::jthread jt = support::make_test_jthread([&result](int i) { result += i; }, 5);
     jt.join();
     assert(result == 5);
   }
@@ -90,20 +92,22 @@ int main(int, char**) {
     auto mainThread = std::this_thread::get_id();
 
     TrackThread arg1;
-    std::jthread jt1{[mainThread](const TrackThread& arg) {
-                       assert(arg.threadId == mainThread);
-                       assert(arg.threadId != std::this_thread::get_id());
-                       assert(arg.copyConstructed);
-                     },
-                     arg1};
+    std::jthread jt1 = support::make_test_jthread(
+        [mainThread](const TrackThread& arg) {
+          assert(arg.threadId == mainThread);
+          assert(arg.threadId != std::this_thread::get_id());
+          assert(arg.copyConstructed);
+        },
+        arg1);
 
     TrackThread arg2;
-    std::jthread jt2{[mainThread](const TrackThread& arg) {
-                       assert(arg.threadId == mainThread);
-                       assert(arg.threadId != std::this_thread::get_id());
-                       assert(arg.moveConstructed);
-                     },
-                     std::move(arg2)};
+    std::jthread jt2 = support::make_test_jthread(
+        [mainThread](const TrackThread& arg) {
+          assert(arg.threadId == mainThread);
+          assert(arg.threadId != std::this_thread::get_id());
+          assert(arg.moveConstructed);
+        },
+        std::move(arg2));
   }
 
 #if !defined(TEST_HAS_NO_EXCEPTIONS)
@@ -120,7 +124,7 @@ int main(int, char**) {
     };
     ThrowOnCopyFunc f1;
     try {
-      std::jthread jt{f1};
+      std::jthread jt = support::make_test_jthread(f1);
       assert(false);
     } catch (const Exception& e) {
       assert(e.threadId == std::this_thread::get_id());
@@ -140,7 +144,7 @@ int main(int, char**) {
     };
 
     Arg arg(flag);
-    std::jthread jt(
+    std::jthread jt = support::make_test_jthread(
         [&flag](const auto&) {
           assert(flag == 5); // happens-after the copy-construction of arg
         },

>From 53c55bb909f3c3e2da4161b978d118b18714e21b Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconsteq at gmail.com>
Date: Wed, 25 Oct 2023 17:32:44 -0700
Subject: [PATCH 2/3] clang-format

---
 .../test/std/thread/thread.jthread/cons.func.token.pass.cpp   | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp b/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
index fafd7818620aa7d..6a445bd5af3208e 100644
--- a/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
+++ b/libcxx/test/std/thread/thread.jthread/cons.func.token.pass.cpp
@@ -56,7 +56,7 @@ int main(int, char**) {
   // invoke(auto(std::forward<F>(f)), get_stop_token(), auto(std::forward<Args>(args))...)
   // if that expression is well-formed,
   {
-    int result = 0;
+    int result      = 0;
     std::jthread jt = support::make_test_jthread(
         [&result](std::stop_token st, int i) {
           assert(st.stop_possible());
@@ -71,7 +71,7 @@ int main(int, char**) {
   // otherwise
   // invoke(auto(std::forward<F>(f)), auto(std::forward<Args>(args))...)
   {
-    int result = 0;
+    int result      = 0;
     std::jthread jt = support::make_test_jthread([&result](int i) { result += i; }, 5);
     jt.join();
     assert(result == 5);

>From 7fd483413828751e3f55cdd101f549b35f0eeb2c Mon Sep 17 00:00:00 2001
From: Konstantin Varlamov <varconsteq at gmail.com>
Date: Fri, 27 Oct 2023 02:19:42 -0700
Subject: [PATCH 3/3] [libc++] Fix flakiness in `atomic_notify_all.pass.cpp`

Avoid relying on sleeping for synchronization.
---
 .../atomic_notify_all.pass.cpp                     | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
index d22ea426057113b..b96691735a4ccbd 100644
--- a/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
+++ b/libcxx/test/std/atomics/atomics.types.operations/atomics.types.operations.wait/atomic_notify_all.pass.cpp
@@ -56,15 +56,21 @@ struct TestFn {
     {
       volatile A a(T(2));
       static_assert(noexcept(std::atomic_notify_all(&a)), "");
-      auto f = [&]() {
+
+      std::atomic<bool> is_ready[2] = {false, false};
+      auto f = [&](int index) {
         assert(std::atomic_load(&a) == T(2));
+        is_ready[index].store(true);
+
         std::atomic_wait(&a, T(2));
         assert(std::atomic_load(&a) == T(4));
       };
-      std::thread t1 = support::make_test_thread(f);
-      std::thread t2 = support::make_test_thread(f);
-      std::this_thread::sleep_for(std::chrono::milliseconds(100));
+      std::thread t1 = support::make_test_thread(f, /*index=*/0);
+      std::thread t2 = support::make_test_thread(f, /*index=*/1);
 
+      while (!is_ready[0] || !is_ready[1]) {
+        // Spin
+      }
       std::atomic_store(&a, T(4));
       std::atomic_notify_all(&a);
       t1.join();



More information about the libcxx-commits mailing list