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.
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:
The test code is clearly using
sleep_for100ms as a synchronization primitive:llvm-project/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
Lines 32 to 35 in 3eb4128
llvm-project/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
Lines 42 to 45 in 3eb4128
llvm-project/libcxx/test/std/thread/thread.condition/thread.condition.condvar/notify_all.pass.cpp
Lines 52 to 61 in 3eb4128
Under heavy load, it's entirely possible for the new threads
t1andt2runningf1andf2respectively to not get started before themainthread gets through its 100ms sleep, acquiresmut, and setstest1andtest2to1. This causesf1andf2to assert when they discover this.A deterministic mechanism needs to be used here.