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++][test] thread.condition.condvar/notify_all.pass.cpp has a flaky timing assumption #95944

Closed
StephanTLavavej opened this issue Jun 18, 2024 · 1 comment · Fixed by #97622
Assignees
Labels
libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. test-suite

Comments

@StephanTLavavej
Copy link
Member

After #91530 refactored some condvar tests to pass reliably, @Arup-Chauhan helped MSVC's STL by re-enabling the condvar tests that we had been skipping (microsoft/STL#4721). However, our CI system immediately encountered sporadic failures in thread.condition.condvar/notify_all.pass.cpp. #91530 didn't change that test, so this isn't new, we just didn't realize this had a distinct cause until the other tests were fixed.

The assertions we saw (on different test runs) were:

Assertion failed: test1 == 0, file D:\a\_work\1\s\llvm-project\libcxx\test\std\thread\thread.condition\thread.condition.condvar\notify_all.pass.cpp, line 35

Assertion failed: test2 == 0, file D:\a\_work\1\s\llvm-project\libcxx\test\std\thread\thread.condition\thread.condition.condvar\notify_all.pass.cpp, line 45

The test code is clearly using sleep_for 100ms as a synchronization primitive:

void f1()
{
std::unique_lock<std::mutex> lk(mut);
assert(test1 == 0);

void f2()
{
std::unique_lock<std::mutex> lk(mut);
assert(test2 == 0);

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));
{
std::unique_lock<std::mutex>lk(mut);
test1 = 1;
test2 = 1;
}

Under heavy load, it's entirely possible for the new threads t1 and t2 running f1 and f2 respectively to not get started before the main thread gets through its 100ms sleep, acquires mut, and sets test1 and test2 to 1. This causes f1 and f2 to assert when they discover this.

A deterministic mechanism needs to be used here.

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jun 18, 2024
@Arup-Chauhan
Copy link

@StephanTLavavej, thanks for the mention! I'll be keeping a close eye on the fix.

@EugeneZelenko, nice to meet you! I'd love to stay in the loop on this issue.

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. test-suite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants