[libcxx-commits] [PATCH] D79406: [libcxx testing] Make try_lock_shared_until.pass.cpp more reliable

[David Zarzycki via Phabricator via libcxx-commits]

davezarzycki created this revision.
davezarzycki added reviewers: ldionne, EricWF, howard.hinnant, mclow.lists.
davezarzycki added a project: libc++.
Herald added subscribers: broadwaylamb, jfb, dexonsmith.
Herald added a reviewer: libc++.

The challenge with testing timeouts is that slow and/or busy machines can cause tests to fail in unexpected ways. After this change, this test should be much more robust. The only remaining and tiny race that I can think of is preemption between `--countDown` and the try_lock being called.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D79406

Files:
  libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp


Index: libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp
===================================================================
--- libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp
+++ libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp
@@ -9,8 +9,6 @@
 // UNSUPPORTED: libcpp-has-no-threads
 // UNSUPPORTED: c++98, c++03, c++11
 
-// ALLOW_RETRIES: 2
-
 // shared_timed_mutex was introduced in macosx10.12
 // UNSUPPORTED: with_system_cxx_lib=macosx10.11
 // UNSUPPORTED: with_system_cxx_lib=macosx10.10
@@ -39,44 +37,42 @@
 typedef std::chrono::milliseconds ms;
 typedef std::chrono::nanoseconds ns;
 
-ms WaitTime = ms(250);
+ms SuccessWaitTime = ms(5000); // Some machines are busy or slow or both
+ms FailureWaitTime = ms(50);
 
-// Thread sanitizer causes more overhead and will sometimes cause this test
-// to fail. To prevent this we give Thread sanitizer more time to complete the
-// test.
-#if !defined(TEST_HAS_SANITIZERS)
-ms Tolerance = ms(50);
-#else
-ms Tolerance = ms(50 * 5);
-#endif
+// On busy or slow machines, there can be a significant delay between thread
+// creation and thread start, so we use an atomic variable to signal that the
+// thread is actually executing.
+static std::atomic<unsigned> countDown;
 
 void f1()
 {
+    --countDown;
     time_point t0 = Clock::now();
-    assert(m.try_lock_shared_until(Clock::now() + WaitTime + Tolerance) == true);
+    assert(m.try_lock_shared_until(Clock::now() + SuccessWaitTime) == true);
     time_point t1 = Clock::now();
     m.unlock_shared();
-    ns d = t1 - t0 - WaitTime;
-    assert(d < Tolerance);  // within 50ms
+    assert(t1 - t0 <= SuccessWaitTime);
 }
 
 void f2()
 {
     time_point t0 = Clock::now();
-    assert(m.try_lock_shared_until(Clock::now() + WaitTime) == false);
-    time_point t1 = Clock::now();
-    ns d = t1 - t0 - WaitTime;
-    assert(d < Tolerance);  // within tolerance
+    assert(m.try_lock_shared_until(Clock::now() + FailureWaitTime) == false);
+    assert(Clock::now() - t0 >= FailureWaitTime);
 }
 
 int main(int, char**)
 {
+    int threads = 5;
     {
+        countDown.store(threads);
         m.lock();
         std::vector<std::thread> v;
-        for (int i = 0; i < 5; ++i)
+        for (int i = 0; i < threads; ++i)
             v.push_back(std::thread(f1));
-        std::this_thread::sleep_for(WaitTime);
+        while (countDown > 0)
+            std::this_thread::yield();
         m.unlock();
         for (auto& t : v)
             t.join();
@@ -84,12 +80,11 @@
     {
         m.lock();
         std::vector<std::thread> v;
-        for (int i = 0; i < 5; ++i)
+        for (int i = 0; i < threads; ++i)
             v.push_back(std::thread(f2));
-        std::this_thread::sleep_for(WaitTime + Tolerance);
-        m.unlock();
         for (auto& t : v)
             t.join();
+        m.unlock();
     }
 
   return 0;


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D79406.262063.patch
Type: text/x-patch
Size: 3183 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/libcxx-commits/attachments/20200505/988e9d6b/attachment.bin>