diff --git a/lib/Basics/Guarded.h b/lib/Basics/Guarded.h index 636e091ed1d3..478cfb4e4585 100644 --- a/lib/Basics/Guarded.h +++ b/lib/Basics/Guarded.h @@ -30,6 +30,7 @@ #include #include #include +#include #include /** @@ -90,6 +91,11 @@ class MutexGuard { auto get() const noexcept -> T&; auto operator->() const noexcept -> T*; + // @brief Unlocks and releases the mutex, and releases the value. + // The guard is unusable after this, and the value inaccessible from + // the guard. + void unlock() noexcept(noexcept(std::declval().unlock())); + private: struct nop { void operator()(T*) {} @@ -116,6 +122,15 @@ auto MutexGuard::operator->() const noexcept -> T* { return std::addressof(get()); } +template +void MutexGuard::unlock() noexcept(noexcept(std::declval().unlock())) { + static_assert(noexcept(_value.reset())); + static_assert(noexcept(_mutexLock.release())); + _value.reset(); + _mutexLock.unlock(); + _mutexLock.release(); +} + template class L = std::unique_lock> class Guarded { public: @@ -249,7 +264,7 @@ auto Guarded::getLockedGuard() const -> MutexGuard> { template class L> auto Guarded::tryLockedGuard() -> std::optional>> { - auto lock = lock_type(_mutex, std::try_lock); + auto lock = lock_type(_mutex, std::try_to_lock); if (lock.owns_lock()) { return MutexGuard(_value, std::move(lock)); @@ -261,7 +276,7 @@ auto Guarded::tryLockedGuard() -> std::optional>> { template class L> auto Guarded::tryLockedGuard() const -> std::optional>> { - auto lock = lock_type(_mutex, std::try_lock); + auto lock = lock_type(_mutex, std::try_to_lock); if (lock.owns_lock()) { return MutexGuard(_value, std::move(lock)); diff --git a/tests/Basics/GuardedTest.cpp b/tests/Basics/GuardedTest.cpp index 571efbecd027..26d256866cf9 100644 --- a/tests/Basics/GuardedTest.cpp +++ b/tests/Basics/GuardedTest.cpp @@ -22,6 +22,8 @@ #include "gtest/gtest.h" +#include + #include "Basics/Guarded.h" #include "Basics/Mutex.h" #include "Basics/MutexLocker.h" @@ -51,7 +53,11 @@ class GuardedTest : public ::testing::Test { using Guarded = Guarded; }; +template +class GuardedDeathTest : public GuardedTest {}; + TYPED_TEST_CASE_P(GuardedTest); +TYPED_TEST_CASE_P(GuardedDeathTest); // Test helper that acquires a lock; // then executes a callback that tries to acquire the same lock; @@ -160,6 +166,46 @@ TYPED_TEST_P(GuardedTest, test_guard_waits_for_access) { testWaitForLock(acquireGuard); } +TYPED_TEST_P(GuardedTest, test_guard_unlock_releases_mutex) { + auto guardedObj = typename TestFixture::template Guarded{1}; + EXPECT_EQ(1, guardedObj.copy().val); + auto guard = guardedObj.getLockedGuard(); + EXPECT_EQ(1, guard.get().val); + guard.get().val = 2; + EXPECT_EQ(2, guard.get().val); + guard.unlock(); + + auto threadStarted = std::atomic{false}; + auto couldAcquireLock = std::atomic{false}; + auto thr = std::thread([&] { + threadStarted.store(true, std::memory_order_release); + guardedObj.doUnderLock([&](auto&&) { + couldAcquireLock.store(true, std::memory_order_release); + }); + }); + + EXPECT_EQ(2, guardedObj.copy().val); + while (!threadStarted.load(std::memory_order_acquire)) { + // busy wait + } + // wait generously for the thread to try to get the lock and do something + std::this_thread::sleep_for(std::chrono::milliseconds{1}); + EXPECT_TRUE(couldAcquireLock.load(std::memory_order_acquire)); + thr.join(); +} + +TYPED_TEST_P(GuardedDeathTest, test_guard_unlock_releases_value) { + auto guardedObj = typename TestFixture::template Guarded{1}; + EXPECT_EQ(1, guardedObj.copy().val); + auto guard = guardedObj.getLockedGuard(); + EXPECT_EQ(1, guard.get().val); + guard.get().val = 2; + EXPECT_EQ(2, guard.get().val); + guard.unlock(); + + ASSERT_DEATH_CORE_FREE({ ASSERT_NE(2, guard.get().val); }, ""); +} + TYPED_TEST_P(GuardedTest, test_do_allows_access) { auto guardedObj = typename TestFixture::template Guarded{1}; { @@ -252,6 +298,32 @@ TYPED_TEST_P(GuardedTest, test_try_allows_access) { } } +TYPED_TEST_P(GuardedTest, test_try_guard_allows_access) { + auto guardedObj = typename TestFixture::template Guarded{1}; + { + // try is allowed to spuriously fail for no reason. But we expect it to + // succeed at some point when noone holds the lock. + bool didExecute = false; + while (!didExecute) { + if (auto guard = guardedObj.tryLockedGuard()) { + EXPECT_EQ(1, guard->get().val); + guard->get().val = 2; + didExecute = true; + EXPECT_EQ(2, guard->get().val); + } + + { + auto guard = guardedObj.getLockedGuard(); + if (didExecute) { + EXPECT_EQ(guard->val, 2); + } else { + EXPECT_EQ(guard->val, 1); + } + } + } + } +} + TYPED_TEST_P(GuardedTest, test_try_fails_access) { auto guardedObj = typename TestFixture::template Guarded{1}; { @@ -279,11 +351,40 @@ TYPED_TEST_P(GuardedTest, test_try_fails_access) { } } -REGISTER_TYPED_TEST_CASE_P(GuardedTest, test_copy_allows_access, test_copy_waits_for_access, - test_assign_allows_access, test_assign_waits_for_access, - test_guard_allows_access, test_guard_waits_for_access, +TYPED_TEST_P(GuardedTest, test_try_guard_fails_access) { + auto guardedObj = typename TestFixture::template Guarded{1}; + { + auto guard = guardedObj.getLockedGuard(); + bool threadStarted = false; + bool threadFinished = false; + auto thr = std::thread([&] { + threadStarted = true; + bool didExecute = false; + if (auto guard = guardedObj.tryLockedGuard()) { + EXPECT_EQ(1, guard->get().val); + guard->get().val = 2; + didExecute = true; + EXPECT_EQ(2, guard->get().val); + } + EXPECT_FALSE(didExecute); + threadFinished = true; + }); + thr.join(); + EXPECT_TRUE(threadStarted); + EXPECT_TRUE(threadFinished); + EXPECT_EQ(guard->val, 1); + } +} + +REGISTER_TYPED_TEST_CASE_P(GuardedTest, test_copy_allows_access, + test_copy_waits_for_access, test_assign_allows_access, + test_assign_waits_for_access, test_guard_allows_access, + test_guard_waits_for_access, test_guard_unlock_releases_mutex, test_do_allows_access, test_do_waits_for_access, - test_try_allows_access, test_try_fails_access); + test_try_allows_access, test_try_guard_allows_access, + test_try_fails_access, test_try_guard_fails_access); + +REGISTER_TYPED_TEST_CASE_P(GuardedDeathTest, test_guard_unlock_releases_value); template