Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] Fix sporadic test failure in condition_variable notify_all test #97622

Merged
merged 3 commits into from
Jul 12, 2024

Conversation

huixie90
Copy link
Contributor

@huixie90 huixie90 commented Jul 3, 2024

fixes #95944

@huixie90 huixie90 requested a review from a team as a code owner July 3, 2024 19:13
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jul 3, 2024

@llvm/pr-subscribers-libcxx

Author: Hui (huixie90)

Changes

Full diff: https://github.com/llvm/llvm-project/pull/97622.diff

1 Files Affected:

  • (modified) libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp (+8-1)
diff --git a/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp b/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
index e0d587dbca0e9..995e4c9f72f8b 100644
--- a/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
+++ b/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
@@ -14,6 +14,7 @@
 
 // void notify_all();
 
+#include <atomic>
 #include <condition_variable>
 #include <mutex>
 #include <thread>
@@ -29,10 +30,13 @@ int test0 = 0;
 int test1 = 0;
 int test2 = 0;
 
+std::atomic<int> ready_count = 0;
+
 void f1()
 {
     std::unique_lock<std::mutex> lk(mut);
     assert(test1 == 0);
+    ready_count.fetch_add(1);
     while (test1 == 0)
         cv.wait(lk);
     assert(test1 == 1);
@@ -43,6 +47,7 @@ void f2()
 {
     std::unique_lock<std::mutex> lk(mut);
     assert(test2 == 0);
+    ready_count.fetch_add(1);
     while (test2 == 0)
         cv.wait(lk);
     assert(test2 == 1);
@@ -53,7 +58,9 @@ int main(int, char**)
 {
     std::thread t1 = support::make_test_thread(f1);
     std::thread t2 = support::make_test_thread(f2);
-    std::this_thread::sleep_for(std::chrono::milliseconds(100));
+    while (ready_count.load() != 2) {
+        std::this_thread::sleep_for(std::chrono::milliseconds(100));
+    }
     {
         std::unique_lock<std::mutex>lk(mut);
         test1 = 1;

Copy link

github-actions bot commented Jul 3, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@ldionne ldionne changed the title [libc++][test] fix sporiadic test failure in condition_variable notify_all test [libc++] Fix sporadic test failure in condition_variable notify_all test Jul 3, 2024
void f1()
{
std::unique_lock<std::mutex> lk(mut);
assert(test1 == 0);
ready_count.fetch_add(1);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Slight preference for ready_count += 1 since that's easier to read.

@huixie90 huixie90 merged commit 840e857 into llvm:main Jul 12, 2024
55 checks passed
@Arup-Chauhan
Copy link

Hi @huixie90 and @ldionne, I refactored some of these test cases for microsoft/STL.
I would appreciate if you could share some quick insights on what you did to fix this issue.

Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[libc++][test] thread.condition.condvar/notify_all.pass.cpp has a flaky timing assumption
4 participants