Skip to content

[libc++] Refactor tests for shared_mutex and shared_timed_mutex #100783

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

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

ldionne
Copy link
Member

@ldionne ldionne commented Jul 26, 2024

This makes the tests less flaky and also makes a few other refactorings like using traits instead of .compile.fail.cpp tests.

@ldionne ldionne requested a review from a team as a code owner July 26, 2024 17:47
@llvmbot llvmbot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Jul 26, 2024
@llvmbot
Copy link
Member

llvmbot commented Jul 26, 2024

@llvm/pr-subscribers-libcxx

Author: Louis Dionne (ldionne)

Changes

This makes the tests less flaky and also makes a few other refactorings like using traits instead of .compile.fail.cpp tests.


Patch is 64.54 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/100783.diff

18 Files Affected:

  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.compile.pass.cpp (+3-9)
  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.copy.compile.pass.cpp (+3-8)
  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.default.pass.cpp (+5-6)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp (+76-34)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp (+111-60)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp (+35-31)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp (+49-44)
  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/assign.compile.pass.cpp (+2-8)
  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/ctor.copy.compile.pass.cpp (+2-7)
  • (renamed) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/ctor.default.pass.cpp (+1-1)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/lock.pass.cpp (+82-40)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/lock_shared.pass.cpp (+115-78)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock.pass.cpp (+35-31)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_for.pass.cpp (+72-53)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared.pass.cpp (+49-43)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_for.pass.cpp (+96-63)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_shared_until.pass.cpp (+88-54)
  • (modified) libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.sharedtimedmutex.requirements/thread.sharedtimedmutex.class/try_lock_until.pass.cpp (+72-54)
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.verify.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.compile.pass.cpp
similarity index 75%
rename from libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.verify.cpp
rename to libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.compile.pass.cpp
index 34164aaa0413c..0d90bff928369 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.verify.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/assign.compile.pass.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
@@ -16,12 +16,6 @@
 // shared_mutex& operator=(const shared_mutex&) = delete;
 
 #include <shared_mutex>
+#include <type_traits>
 
-int main(int, char**)
-{
-    std::shared_mutex m0;
-    std::shared_mutex m1;
-    m1 = m0; // expected-error {{overload resolution selected deleted operator '='}}
-
-  return 0;
-}
+static_assert(!std::is_copy_assignable<std::shared_mutex>::value, "");
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.verify.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.copy.compile.pass.cpp
similarity index 76%
rename from libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.verify.cpp
rename to libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.copy.compile.pass.cpp
index 9b43198d0a37b..f9e1935f15b9e 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/copy.verify.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.copy.compile.pass.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
@@ -16,11 +16,6 @@
 // shared_mutex(const shared_mutex&) = delete;
 
 #include <shared_mutex>
+#include <type_traits>
 
-int main(int, char**)
-{
-    std::shared_mutex m0;
-    std::shared_mutex m1(m0); // expected-error {{call to deleted constructor of 'std::shared_mutex'}}
-
-  return 0;
-}
+static_assert(!std::is_copy_constructible<std::shared_mutex>::value, "");
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/default.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.default.pass.cpp
similarity index 87%
rename from libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/default.pass.cpp
rename to libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.default.pass.cpp
index 5504645bb31f9..c941f3a5ea277 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/default.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/ctor.default.pass.cpp
@@ -5,7 +5,7 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
@@ -19,10 +19,9 @@
 
 #include "test_macros.h"
 
-int main(int, char**)
-{
-    std::shared_mutex m;
-    (void)m;
+int main(int, char**) {
+  std::shared_mutex m;
+  (void)m;
 
-    return 0;
+  return 0;
 }
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp
index 122f2b0cddb79..724fb0728a979 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock.pass.cpp
@@ -5,63 +5,105 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
-// ALLOW_RETRIES: 2
-
 // <shared_mutex>
 
 // class shared_mutex;
 
 // void lock();
 
+#include <shared_mutex>
+#include <atomic>
 #include <cassert>
 #include <chrono>
-#include <cstdlib>
-#include <shared_mutex>
 #include <thread>
+#include <vector>
 
 #include "make_test_thread.h"
 #include "test_macros.h"
 
-std::shared_mutex m;
+int main(int, char**) {
+  // Exclusive-lock a mutex that is not locked yet. This should succeed.
+  {
+    std::shared_mutex m;
+    m.lock();
+    m.unlock();
+  }
 
-typedef std::chrono::system_clock Clock;
-typedef Clock::time_point time_point;
-typedef Clock::duration duration;
-typedef std::chrono::milliseconds ms;
-typedef std::chrono::nanoseconds ns;
+  // Exclusive-lock a mutex that is already locked exclusively. This should block until it is unlocked.
+  {
+    std::atomic<bool> ready(false);
+    std::shared_mutex m;
+    m.lock();
+    std::atomic<bool> is_locked_from_main(true);
 
-ms WaitTime = ms(250);
+    std::thread t = support::make_test_thread([&] {
+      ready = true;
+      m.lock();
+      assert(!is_locked_from_main);
+      m.unlock();
+    });
 
-// 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_IS_EXECUTED_IN_A_SLOW_ENVIRONMENT)
-ms Tolerance = ms(50);
-#else
-ms Tolerance = ms(50 * 5);
-#endif
+    while (!ready)
+      /* spin */;
 
-void f()
-{
-    time_point t0 = Clock::now();
-    m.lock();
-    time_point t1 = Clock::now();
+    // We would rather signal this after we unlock, but that would create a race condition.
+    // We instead signal it before we unlock, which means that it's technically possible for the thread
+    // to take the lock while we're still holding it and for the test to still pass.
+    is_locked_from_main = false;
     m.unlock();
-    ns d = t1 - t0 - WaitTime;
-    assert(d < Tolerance);  // within tolerance
-}
 
-int main(int, char**)
-{
-    m.lock();
-    std::thread t = support::make_test_thread(f);
-    std::this_thread::sleep_for(WaitTime);
-    m.unlock();
     t.join();
+  }
+
+  // Exclusive-lock a mutex that is already share-locked. This should block until it is unlocked.
+  {
+    std::atomic<bool> ready(false);
+    std::shared_mutex m;
+    m.lock_shared();
+    std::atomic<bool> is_locked_from_main(true);
+
+    std::thread t = support::make_test_thread([&] {
+      ready = true;
+      m.lock();
+      assert(!is_locked_from_main);
+      m.unlock();
+    });
+
+    while (!ready)
+      /* spin */;
+
+    // We would rather signal this after we unlock, but that would create a race condition.
+    // We instead signal it before we unlock, which means that it's technically possible for
+    // the thread to take the lock while we're still holding it and for the test to still pass.
+    is_locked_from_main = false;
+    m.unlock_shared();
+
+    t.join();
+  }
+
+  // Make sure that at most one thread can acquire the mutex concurrently.
+  {
+    std::atomic<int> counter = 0;
+    std::shared_mutex mutex;
+
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 10; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        mutex.lock();
+        counter++;
+        assert(counter == 1);
+        counter--;
+        mutex.unlock();
+      }));
+    }
+
+    for (auto& t : threads)
+      t.join();
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp
index 9df0d57c9621c..0d13ca6d27738 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/lock_shared.pass.cpp
@@ -5,87 +5,138 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
-// ALLOW_RETRIES: 2
-
 // <shared_mutex>
 
 // class shared_mutex;
 
 // void lock_shared();
 
-#include <cassert>
-#include <chrono>
-#include <cstdlib>
 #include <shared_mutex>
+#include <atomic>
+#include <cassert>
 #include <thread>
 #include <vector>
 
 #include "make_test_thread.h"
-#include "test_macros.h"
-
-std::shared_mutex m;
-
-typedef std::chrono::system_clock Clock;
-typedef Clock::time_point time_point;
-typedef Clock::duration duration;
-typedef std::chrono::milliseconds ms;
-typedef std::chrono::nanoseconds ns;
-
-ms WaitTime = ms(250);
-
-// 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_IS_EXECUTED_IN_A_SLOW_ENVIRONMENT)
-ms Tolerance = ms(50);
-#else
-ms Tolerance = ms(50 * 5);
-#endif
-
-void f()
-{
-    time_point t0 = Clock::now();
-    m.lock_shared();
-    time_point t1 = Clock::now();
-    m.unlock_shared();
-    ns d = t1 - t0 - WaitTime;
-    assert(d < Tolerance);  // within tolerance
-}
 
-void g()
-{
-    time_point t0 = Clock::now();
-    m.lock_shared();
-    time_point t1 = Clock::now();
-    m.unlock_shared();
-    ns d = t1 - t0;
-    assert(d < Tolerance);  // within tolerance
-}
+int main(int, char**) {
+  // Lock-shared a mutex that is not locked yet. This should succeed.
+  {
+    std::shared_mutex m;
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 5; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        m.lock_shared();
+        m.unlock_shared();
+      }));
+    }
 
+    for (auto& t : threads)
+      t.join();
+  }
 
-int main(int, char**)
-{
+  // Lock-shared a mutex that is already exclusively locked. This should block until it is unlocked.
+  {
+    std::atomic<int> ready(0);
+    std::shared_mutex m;
     m.lock();
-    std::vector<std::thread> v;
-    for (int i = 0; i < 5; ++i)
-        v.push_back(support::make_test_thread(f));
-    std::this_thread::sleep_for(WaitTime);
+    std::atomic<bool> is_locked_from_main(true);
+
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 5; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        ++ready;
+        while (ready < 5)
+          /* wait until all threads have been created */;
+
+        m.lock_shared();
+        assert(!is_locked_from_main);
+        m.unlock_shared();
+      }));
+    }
+
+    while (ready < 5)
+      /* wait until all threads have been created */;
+
+    // We would rather signal this after we unlock, but that would create a race condition.
+    // We instead signal it before we unlock, which means that it's technically possible for
+    // the thread to take the lock while we're still holding it and for the test to still pass.
+    is_locked_from_main = false;
     m.unlock();
-    for (auto& t : v)
-        t.join();
+
+    for (auto& t : threads)
+      t.join();
+  }
+
+  // Lock-shared a mutex that is already lock-shared. This should succeed.
+  {
+    std::atomic<int> ready(0);
+    std::shared_mutex m;
     m.lock_shared();
-    for (auto& t : v)
-        t = support::make_test_thread(g);
-    std::thread q = support::make_test_thread(f);
-    std::this_thread::sleep_for(WaitTime);
+
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 5; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        ++ready;
+        while (ready < 5)
+          /* wait until all threads have been created */;
+
+        m.lock_shared();
+        m.unlock_shared();
+      }));
+    }
+
+    while (ready < 5)
+      /* wait until all threads have been created */;
+
     m.unlock_shared();
-    for (auto& t : v)
-        t.join();
-    q.join();
+
+    for (auto& t : threads)
+      t.join();
+  }
+
+  // Create several threads that all acquire-shared the same mutex and make sure that each
+  // thread successfully acquires-shared the mutex.
+  //
+  // We record how many other threads were holding the mutex when it was acquired, which allows
+  // us to know whether the test was somewhat effective at causing multiple threads to lock at
+  // the same time.
+  {
+    std::shared_mutex mutex;
+    std::vector<std::thread> threads;
+    constexpr int n_threads           = 5;
+    std::atomic<int> holders          = 0;
+    int concurrent_holders[n_threads] = {};
+    std::atomic<bool> ready           = false;
+
+    for (int i = 0; i != n_threads; ++i) {
+      threads.push_back(support::make_test_thread([&, i] {
+        while (!ready) {
+          // spin
+        }
+
+        mutex.lock_shared();
+        ++holders;
+        concurrent_holders[i] = holders;
+
+        mutex.unlock_shared();
+        --holders;
+      }));
+    }
+
+    ready = true; // let the threads actually start shared-acquiring the mutex
+    for (auto& t : threads)
+      t.join();
+
+    // We can't guarantee that we'll ever have more than 1 concurrent holder so that's what
+    // we assert, however in principle we should often trigger more than 1 concurrent holder.
+    int max_concurrent_holders = *std::max_element(std::begin(concurrent_holders), std::end(concurrent_holders));
+    assert(max_concurrent_holders >= 1);
+  }
 
   return 0;
 }
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp
index f39b1ee29f7db..c62d5c8663712 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock.pass.cpp
@@ -5,56 +5,60 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
-// ALLOW_RETRIES: 2
-
 // <shared_mutex>
 
 // class shared_mutex;
 
 // bool try_lock();
 
+#include <shared_mutex>
+#include <atomic>
 #include <cassert>
 #include <chrono>
-#include <cstdlib>
-#include <shared_mutex>
 #include <thread>
 
 #include "make_test_thread.h"
-#include "test_macros.h"
-
-std::shared_mutex m;
-
-typedef std::chrono::system_clock Clock;
-typedef Clock::time_point time_point;
-typedef Clock::duration duration;
-typedef std::chrono::milliseconds ms;
-typedef std::chrono::nanoseconds ns;
-
-void f()
-{
-    time_point t0 = Clock::now();
-    assert(!m.try_lock());
-    assert(!m.try_lock());
-    assert(!m.try_lock());
-    while(!m.try_lock())
-        ;
-    time_point t1 = Clock::now();
+
+int main(int, char**) {
+  // Try to exclusive-lock a mutex that is not locked yet. This should succeed.
+  {
+    std::shared_mutex m;
+    bool succeeded = m.try_lock();
+    assert(succeeded);
     m.unlock();
-    ns d = t1 - t0 - ms(250);
-    assert(d < ms(200));  // within 200ms
-}
+  }
 
-int main(int, char**)
-{
+  // Try to exclusive-lock a mutex that is already locked exclusively. This should fail.
+  {
+    std::shared_mutex m;
     m.lock();
-    std::thread t = support::make_test_thread(f);
-    std::this_thread::sleep_for(ms(250));
+
+    std::thread t = support::make_test_thread([&] {
+        bool succeeded = m.try_lock();
+        assert(!succeeded);
+    });
+    t.join();
+
     m.unlock();
+  }
+
+  // Try to exclusive-lock a mutex that is already share-locked. This should fail.
+  {
+    std::shared_mutex m;
+    m.lock_shared();
+
+    std::thread t = support::make_test_thread([&] {
+        bool succeeded = m.try_lock();
+        assert(!succeeded);
+    });
     t.join();
 
+    m.unlock_shared();
+  }
+
   return 0;
 }
diff --git a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp
index c091b06f47492..61069bebb72bd 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.mutex.requirements/thread.shared_mutex.requirements/thread.shared_mutex.class/try_lock_shared.pass.cpp
@@ -5,71 +5,76 @@
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
-//
+
 // UNSUPPORTED: no-threads
 // UNSUPPORTED: c++03, c++11, c++14
 
-// ALLOW_RETRIES: 2
-
 // <shared_mutex>
 
 // class shared_mutex;
 
 // bool try_lock_shared();
 
-#include <cassert>
-#include <chrono>
-#include <cstdlib>
 #include <shared_mutex>
+#include <cassert>
 #include <thread>
 #include <vector>
 
 #include "make_test_thread.h"
-#include "test_macros.h"
 
-std::shared_mutex m;
+int main(int, char**) {
+  // Try to lock-shared a mutex that is not locked yet. This should succeed.
+  {
+    std::shared_mutex m;
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 5; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        bool succeeded = m.try_lock_shared();
+        assert(succeeded);
+        m.unlock_shared();
+      }));
+    }
 
-typedef std::chrono::system_clock Clock;
-typedef Clock::time_point time_point;
-typedef Clock::duration duration;
-typedef std::chrono::milliseconds ms;
-typedef std::chrono::nanoseconds ns;
+    for (auto& t : threads)
+      t.join();
+  }
 
+  // Try to lock-shared a mutex that is already exclusively locked. This should fail.
+  {
+    std::shared_mutex m;
+    m.lock();
 
-// 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_IS_EXECUTED_IN_A_SLOW_ENVIRONMENT)
-ms Tolerance = ms(200);
-#else
-ms Tolerance = ms(200 * 5);
-#endif
-
-void f()
-{
-    time_point t0 = Clock::now();
-    assert(!m.try_lock_shared());
-    assert(!m.try_lock_shared());
-    assert(!m.try_lock_shared());
-    while(!m.try_lock_shared())
-        std::this_thread::yield();
-    time_point t1 = Clock::now();
-    m.unlock_shared();
-    ns d = t1 - t0 - ms(250);
-    assert(d < Tolerance);  // within tolerance
-}
+    std::vector<std::thread> threads;
+    for (int i = 0; i != 5; ++i) {
+      threads.push_back(support::make_test_thread([&] {
+        bool succeeded = m.try_lock_shared();
+        assert(!succeeded);
+      }));
+    }
...
[truncated]

Copy link

github-actions bot commented Jul 26, 2024

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

This makes the tests less flaky and also makes a few other refactorings
like using traits instead of .compile.fail.cpp tests.
@ldionne ldionne force-pushed the review/refactor-shared-mutex-tests branch from 5e84ef6 to 00c1f8c Compare July 30, 2024 14:33
@ldionne ldionne merged commit 29ef92b into llvm:main Jul 31, 2024
55 checks passed
@ldionne ldionne deleted the review/refactor-shared-mutex-tests branch July 31, 2024 13:27
@llvm-ci
Copy link
Collaborator

llvm-ci commented Jul 31, 2024

LLVM Buildbot has detected a new failure on builder sanitizer-x86_64-linux running on sanitizer-buildbot2 while building libcxx at step 2 "annotate".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/66/builds/2357

Here is the relevant piece of the build log for the reference:

Step 2 (annotate) failure: 'python ../sanitizer_buildbot/sanitizers/zorg/buildbot/builders/sanitizers/buildbot_selector.py' (failure)
...
[373/378] Generating MSAN_INST_GTEST.gtest-all.cc.x86_64.o
[374/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[375/378] Generating Msan-x86_64-with-call-Test
[376/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[377/378] Generating Msan-x86_64-Test
[377/378] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 4493 of 10163 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60
FAIL: SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp (2949 of 4493)
******************** TEST 'SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=leak  -m32 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m32 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
RUN: at line 5: env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1      /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp 2>&1 | FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
+ env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp:68:24: error: CHECK_MAY_RETURN_1: expected string not found in input
// CHECK_MAY_RETURN_1: allocating 512 times
                       ^
<stdin>:52:44: note: scanning from here
Some of the malloc calls returned non-null: 256
                                           ^
<stdin>:53:13: note: possible intended match here
==670267==LeakSanitizer: soft rss limit unexhausted (220Mb vs 210Mb)
            ^

Input file: <stdin>
Check file: /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           47:  [256] 
           48:  [320] 
           49:  [384] 
           50:  [448] 
           51: Some of the malloc calls returned null: 256 
           52: Some of the malloc calls returned non-null: 256 
check:68'0                                                X~~~~ error: no match found
           53: ==670267==LeakSanitizer: soft rss limit unexhausted (220Mb vs 210Mb) 
Step 11 (test compiler-rt debug) failure: test compiler-rt debug (failure)
...
[373/378] Generating MSAN_INST_GTEST.gtest-all.cc.x86_64.o
[374/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64-with-call.o
[375/378] Generating Msan-x86_64-with-call-Test
[376/378] Generating MSAN_INST_TEST_OBJECTS.msan_test.cpp.x86_64.o
[377/378] Generating Msan-x86_64-Test
[377/378] Running compiler_rt regression tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/discovery.py:276: warning: input '/b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/rtsan/X86_64LinuxConfig' contained no tests
llvm-lit: /b/sanitizer-x86_64-linux/build/llvm-project/llvm/utils/lit/lit/main.py:72: note: The test suite configuration requested an individual test timeout of 0 seconds but a timeout of 900 seconds was requested on the command line. Forcing timeout to be 900 seconds.
-- Testing: 4493 of 10163 tests, 88 workers --
Testing:  0.. 10.. 20.. 30.. 40.. 50.. 60
FAIL: SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp (2949 of 4493)
******************** TEST 'SanitizerCommon-lsan-i386-Linux :: Linux/soft_rss_limit_mb_test.cpp' FAILED ********************
Exit Code: 1

Command Output (stderr):
--
RUN: at line 2: /b/sanitizer-x86_64-linux/build/build_default/./bin/clang  --driver-mode=g++ -gline-tables-only -fsanitize=leak  -m32 -funwind-tables  -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ /b/sanitizer-x86_64-linux/build/build_default/./bin/clang --driver-mode=g++ -gline-tables-only -fsanitize=leak -m32 -funwind-tables -I/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test -ldl -O2 /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -o /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
RUN: at line 5: env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1      /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp 2>&1 | FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
+ env LSAN_OPTIONS=soft_rss_limit_mb=220:quarantine_size=1:allocator_may_return_null=1 /b/sanitizer-x86_64-linux/build/build_default/runtimes/runtimes-bins/compiler-rt/test/sanitizer_common/lsan-i386-Linux/Linux/Output/soft_rss_limit_mb_test.cpp.tmp
+ FileCheck /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp -check-prefix=CHECK_MAY_RETURN_1
/b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp:68:24: error: CHECK_MAY_RETURN_1: expected string not found in input
// CHECK_MAY_RETURN_1: allocating 512 times
                       ^
<stdin>:52:44: note: scanning from here
Some of the malloc calls returned non-null: 256
                                           ^
<stdin>:53:13: note: possible intended match here
==670267==LeakSanitizer: soft rss limit unexhausted (220Mb vs 210Mb)
            ^

Input file: <stdin>
Check file: /b/sanitizer-x86_64-linux/build/llvm-project/compiler-rt/test/sanitizer_common/TestCases/Linux/soft_rss_limit_mb_test.cpp

-dump-input=help explains the following input dump.

Input was:
<<<<<<
            .
            .
            .
           47:  [256] 
           48:  [320] 
           49:  [384] 
           50:  [448] 
           51: Some of the malloc calls returned null: 256 
           52: Some of the malloc calls returned non-null: 256 
check:68'0                                                X~~~~ error: no match found
           53: ==670267==LeakSanitizer: soft rss limit unexhausted (220Mb vs 210Mb) 

CaseyCarter added a commit to CaseyCarter/STL that referenced this pull request Aug 24, 2024
Upstream has cleaned many of these up to remove timing assumptions in llvm/llvm-project#100783, llvm/llvm-project#102151, and llvm/llvm-project#104852. I reverified that our list of `ALLOW_RETRIES` tests is still current.
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.

3 participants