[libcxx-commits] [libcxx] [libc++] Refactor flaky tests for std::shared_lock (PR #91779)
via libcxx-commits
libcxx-commits at lists.llvm.org
Fri May 10 10:46:49 PDT 2024
llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-libcxx
Author: Louis Dionne (ldionne)
<details>
<summary>Changes</summary>
This makes the tests non-flaky.
---
Full diff: https://github.com/llvm/llvm-project/pull/91779.diff
3 Files Affected:
- (modified) libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp (+65-67)
- (modified) libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp (+82-57)
- (modified) libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp (+93-41)
``````````diff
diff --git a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp
index 4940041bcf965..ece330134f2cd 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.cons/mutex.pass.cpp
@@ -5,10 +5,9 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-//
+
// UNSUPPORTED: no-threads
// UNSUPPORTED: c++03, c++11
-// ALLOW_RETRIES: 2
// <shared_mutex>
@@ -19,9 +18,8 @@
// template<class _Mutex> shared_lock(shared_lock<_Mutex>)
// -> shared_lock<_Mutex>; // C++17
+#include <atomic>
#include <cassert>
-#include <chrono>
-#include <cstdlib>
#include <shared_mutex>
#include <thread>
#include <vector>
@@ -29,77 +27,77 @@
#include "make_test_thread.h"
#include "test_macros.h"
-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
+struct Monitor {
+ bool lock_shared_called = false;
+ bool unlock_shared_called = false;
+};
-std::shared_timed_mutex m;
+struct TrackedMutex {
+ Monitor* monitor = nullptr;
-void f()
-{
- time_point t0 = Clock::now();
- time_point t1;
- {
- std::shared_lock<std::shared_timed_mutex> ul(m);
- t1 = Clock::now();
- }
- ns d = t1 - t0 - WaitTime;
- assert(d < Tolerance); // within tolerance
-}
+ void lock_shared() {
+ if (monitor != nullptr)
+ monitor->lock_shared_called = true;
+ }
+ void unlock_shared() {
+ if (monitor != nullptr)
+ monitor->unlock_shared_called = true;
+ }
+};
-void g()
-{
- time_point t0 = Clock::now();
- time_point t1;
- {
- std::shared_lock<std::shared_timed_mutex> ul(m);
- t1 = Clock::now();
- }
- ns d = t1 - t0;
- assert(d < Tolerance); // within tolerance
-}
+template <class Mutex>
+void test() {
+ // Basic sanity test
+ {
+ Mutex mutex;
+ std::vector<std::thread> threads;
+ std::atomic<bool> ready(false);
+ for (int i = 0; i != 5; ++i) {
+ threads.push_back(support::make_test_thread([&] {
+ while (!ready) {
+ // spin
+ }
-int main(int, char**)
-{
- std::vector<std::thread> v;
- {
- m.lock();
- for (int i = 0; i < 5; ++i)
- v.push_back(support::make_test_thread(f));
- std::this_thread::sleep_for(WaitTime);
- m.unlock();
- for (auto& t : v)
- t.join();
- }
- {
- 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);
- m.unlock_shared();
- for (auto& t : v)
- t.join();
- q.join();
+ std::shared_lock<Mutex> lock(mutex);
+ assert(lock.owns_lock());
+ }));
}
+ ready = true;
+ for (auto& t : threads)
+ t.join();
+ }
+
+ // Test CTAD
+ {
+#if TEST_STD_VER >= 17
+ Mutex mutex;
+ std::shared_lock lock(mutex);
+ static_assert(std::is_same<decltype(lock), std::shared_lock<Mutex>>::value);
+#endif
+ }
+}
+
+int main(int, char**) {
#if TEST_STD_VER >= 17
- std::shared_lock sl(m);
- static_assert((std::is_same<decltype(sl), std::shared_lock<decltype(m)>>::value), "" );
+ test<std::shared_mutex>();
#endif
+ test<std::shared_timed_mutex>();
+ test<TrackedMutex>();
+
+ // Use shared_lock with a dummy mutex class that tracks whether each
+ // operation has been called or not.
+ {
+ Monitor monitor;
+ TrackedMutex mutex{&monitor};
+
+ std::shared_lock<TrackedMutex> lock(mutex);
+ assert(monitor.lock_shared_called);
+ assert(lock.owns_lock());
+
+ lock.unlock();
+ assert(monitor.unlock_shared_called);
+ }
return 0;
}
diff --git a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp
index edb7c42356ace..0dd385e56f52f 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/lock.pass.cpp
@@ -5,10 +5,9 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-//
+
// UNSUPPORTED: no-threads
// UNSUPPORTED: c++03, c++11
-// ALLOW_RETRIES: 2
// <shared_mutex>
@@ -16,10 +15,8 @@
// void lock();
+#include <atomic>
#include <cassert>
-#include <chrono>
-#include <cstdlib>
-#include <mutex>
#include <shared_mutex>
#include <system_error>
#include <thread>
@@ -28,71 +25,99 @@
#include "make_test_thread.h"
#include "test_macros.h"
-std::shared_timed_mutex m;
+struct Monitor {
+ bool lock_shared_called = false;
+ bool unlock_shared_called = false;
+};
-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;
+struct TrackedMutex {
+ Monitor* monitor = nullptr;
-ms WaitTime = ms(250);
+ void lock_shared() {
+ if (monitor != nullptr)
+ monitor->lock_shared_called = true;
+ }
+ void unlock_shared() {
+ if (monitor != nullptr)
+ monitor->unlock_shared_called = true;
+ }
+};
-// 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(25);
-#else
-ms Tolerance = ms(25 * 5);
-#endif
+template <class Mutex>
+void test() {
+ // Basic sanity test
+ {
+ Mutex mutex;
+ std::vector<std::thread> threads;
+ std::atomic<bool> ready(false);
+ for (int i = 0; i != 5; ++i) {
+ threads.push_back(support::make_test_thread([&] {
+ while (!ready) {
+ // spin
+ }
+ std::shared_lock<Mutex> lock(mutex, std::defer_lock);
+ lock.lock();
+ assert(lock.owns_lock());
+ }));
+ }
+
+ ready = true;
+ for (auto& t : threads)
+ t.join();
+ }
-void f()
-{
- std::shared_lock<std::shared_timed_mutex> lk(m, std::defer_lock);
- time_point t0 = Clock::now();
- lk.lock();
- time_point t1 = Clock::now();
- assert(lk.owns_lock() == true);
- ns d = t1 - t0 - WaitTime;
- assert(d < Tolerance); // within tolerance
+ // Try locking the same shared_lock again in the same thread. This should throw an exception.
+ {
+ Mutex mutex;
+ std::shared_lock<Mutex> lock(mutex, std::defer_lock);
+ lock.lock();
+ assert(lock.owns_lock());
#ifndef TEST_HAS_NO_EXCEPTIONS
- try
- {
- lk.lock();
- assert(false);
- }
- catch (std::system_error& e)
- {
- assert(e.code().value() == EDEADLK);
+ try {
+ lock.lock();
+ assert(false);
+ } catch (std::system_error const& e) {
+ assert(e.code() == std::errc::resource_deadlock_would_occur);
}
#endif
- lk.unlock();
- lk.release();
+ }
+
+ // Try locking a shared_lock that isn't associated to any mutex. This should throw an exception.
+ {
+ std::shared_lock<Mutex> lock; // no associated mutex
#ifndef TEST_HAS_NO_EXCEPTIONS
- try
- {
- lk.lock();
- assert(false);
- }
- catch (std::system_error& e)
- {
- assert(e.code().value() == EPERM);
+ try {
+ lock.lock();
+ assert(false);
+ } catch (std::system_error const& e) {
+ assert(e.code() == std::errc::operation_not_permitted);
}
#endif
+ }
}
-int main(int, char**)
-{
- 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);
- m.unlock();
- for (auto& t : v)
- t.join();
+int main(int, char**) {
+#if TEST_STD_VER >= 17
+ test<std::shared_mutex>();
+#endif
+ test<std::shared_timed_mutex>();
+ test<TrackedMutex>();
+
+ // Use shared_lock with a dummy mutex class that tracks whether each
+ // operation has been called or not.
+ {
+ Monitor monitor;
+ TrackedMutex mutex{&monitor};
+
+ std::shared_lock<TrackedMutex> lock(mutex, std::defer_lock);
+ lock.lock();
+ assert(monitor.lock_shared_called);
+ assert(lock.owns_lock());
+
+ lock.unlock();
+ assert(monitor.unlock_shared_called);
+ }
return 0;
}
diff --git a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp
index 0e707fcf2d50a..daa5ee8b56b8b 100644
--- a/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp
+++ b/libcxx/test/std/thread/thread.mutex/thread.lock/thread.lock.shared/thread.lock.shared.locking/try_lock.pass.cpp
@@ -5,11 +5,9 @@
// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
//
//===----------------------------------------------------------------------===//
-//
+
// UNSUPPORTED: no-threads
// UNSUPPORTED: c++03, c++11
-//
-// ALLOW_RETRIES: 2
// <shared_mutex>
@@ -17,60 +15,114 @@
// bool try_lock();
+#include <atomic>
#include <cassert>
-#include <mutex>
#include <shared_mutex>
#include <system_error>
+#include <thread>
+#include <vector>
+#include "make_test_thread.h"
#include "test_macros.h"
-bool try_lock_called = false;
+struct Monitor {
+ bool try_lock_shared_called = false;
+ bool unlock_shared_called = false;
+};
-struct mutex
-{
- bool try_lock_shared()
- {
- try_lock_called = !try_lock_called;
- return try_lock_called;
- }
- void unlock_shared() {}
+struct TrackedMutex {
+ Monitor* monitor = nullptr;
+
+ bool try_lock_shared() {
+ if (monitor != nullptr)
+ monitor->try_lock_shared_called = true;
+ return true;
+ }
+ void unlock_shared() {
+ if (monitor != nullptr)
+ monitor->unlock_shared_called = true;
+ }
};
-mutex m;
+template <class Mutex>
+void test() {
+ // Basic sanity test
+ {
+ Mutex mutex;
+ std::vector<std::thread> threads;
+ std::atomic<bool> ready(false);
+ for (int i = 0; i != 5; ++i) {
+ threads.push_back(support::make_test_thread([&] {
+ while (!ready) {
+ // spin
+ }
-int main(int, char**)
-{
- std::shared_lock<mutex> lk(m, std::defer_lock);
- assert(lk.try_lock() == true);
- assert(try_lock_called == true);
- assert(lk.owns_lock() == true);
-#ifndef TEST_HAS_NO_EXCEPTIONS
- try
- {
- TEST_IGNORE_NODISCARD lk.try_lock();
- assert(false);
+ std::shared_lock<Mutex> lock(mutex, std::defer_lock);
+ bool result = lock.try_lock();
+ assert(result);
+ assert(lock.owns_lock());
+ }));
}
- catch (std::system_error& e)
- {
- assert(e.code().value() == EDEADLK);
+
+ ready = true;
+ for (auto& t : threads)
+ t.join();
+ }
+
+ // Make sure that we throw an exception if we try to re-lock a mutex that is
+ // already locked by the current thread.
+ {
+ Mutex mutex;
+
+ std::shared_lock<Mutex> lock(mutex, std::defer_lock);
+ assert(lock.try_lock());
+ assert(lock.owns_lock());
+#ifndef TEST_HAS_NO_EXCEPTIONS
+ try {
+ TEST_IGNORE_NODISCARD lock.try_lock();
+ assert(false);
+ } catch (std::system_error const& e) {
+ assert(e.code() == std::errc::resource_deadlock_would_occur);
}
#endif
- lk.unlock();
- assert(lk.try_lock() == false);
- assert(try_lock_called == false);
- assert(lk.owns_lock() == false);
- lk.release();
+ }
+
+ // Make sure that we throw an exception if we try to lock a shared_lock
+ // that is not associated to any mutex.
+ {
+ std::shared_lock<Mutex> lock; // not associated to a mutex
#ifndef TEST_HAS_NO_EXCEPTIONS
- try
- {
- TEST_IGNORE_NODISCARD lk.try_lock();
- assert(false);
- }
- catch (std::system_error& e)
- {
- assert(e.code().value() == EPERM);
+ try {
+ TEST_IGNORE_NODISCARD lock.try_lock();
+ assert(false);
+ } catch (std::system_error const& e) {
+ assert(e.code() == std::errc::operation_not_permitted);
}
#endif
+ }
+}
+
+int main(int, char**) {
+#if TEST_STD_VER >= 17
+ test<std::shared_mutex>();
+#endif
+ test<std::shared_timed_mutex>();
+ test<TrackedMutex>();
+
+ // Use shared_lock with a dummy mutex class that tracks whether each
+ // operation has been called or not.
+ {
+ Monitor monitor;
+ TrackedMutex mutex{&monitor};
+
+ std::shared_lock<TrackedMutex> lock(mutex, std::defer_lock);
+ bool result = lock.try_lock();
+ assert(result);
+ assert(monitor.try_lock_shared_called);
+ assert(lock.owns_lock());
+ lock.unlock();
+ assert(monitor.unlock_shared_called);
+ }
return 0;
}
``````````
</details>
https://github.com/llvm/llvm-project/pull/91779
More information about the libcxx-commits
mailing list