8000 Added MutexGuard::unlock(), fixed tryLockedGuard, added tests (#14652) · arangodb/arangodb@b0cd75d · GitHub
[go: up one dir, main page]
Skip to content

Commit b0cd75d

Browse files
goedderzgoedderz
authored
Added MutexGuard::unlock(), fixed tryLockedGuard, added tests (#14652)
1 parent 7afb1d0 commit b0cd75d

File tree

3 files changed

+140
-14
lines changed

3 files changed

+140
-14
lines changed

‎lib/Basics/Guarded.h

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
#include <mutex>
3131
#include <optional>
3232
#include <stdexcept>
33+
#include <utility>
3334
#include <variant>
3435

3536
/**
@@ -90,6 +91,11 @@ class MutexGuard {
9091
auto get() const noexcept -> T&;
9192
auto operator->() const noexcept -> T*;
9293

94+
// @brief Unlocks and releases the mutex, and releases the value.
95+
// The guard is unusable after this, and the value inaccessible from
96+
// the guard.
97+
void unlock() noexcept(noexcept(std::declval<L>().unlock()));
98+
9399
private:
94100
struct nop {
95101
void operator()(T*) {}
@@ -116,6 +122,15 @@ auto MutexGuard<T, L>::operator->() const noexcept -> T* {
116122
return std::addressof(get());
117123
}
118124

125+
template <class T, class L>
126+
void MutexGuard<T, L>::unlock() noexcept(noexcept(std::declval<L>().unlock())) {
127+
static_assert(noexcept(_value.reset()));
128+
static_assert(noexcept(_mutexLock.release()));
129+
_value.reset();
130+
_mutexLock.unlock();
131+
_mutexLock.release();
132+
}
133+
119134
template <class T, class M = std::mutex, template <class> class L = std::unique_lock>
120135
class Guarded {
121136
public:
@@ -249,7 +264,7 @@ auto Guarded<T, M, L>::getLockedGuard() const -> MutexGuard<T const, L<M>> {
249264

250265
template <class T, class M, template <class> class L>
251266
auto Guarded<T, M, L>::tryLockedGuard() -> std::optional<MutexGuard<T, L<M>>> {
252-
auto lock = lock_type(_mutex, std::try_lock);
267+
auto lock = lock_type(_mutex, std::try_to_lock);
253268

254269
if (lock.owns_lock()) {
255270
return MutexGuard(_value, std::move(lock));
@@ -261,7 +276,7 @@ auto Guarded<T, M, L>::tryLockedGuard() -> std::optional<MutexGuard<T, L<M>>> {
261276
template <class T, class M, template <class> class L>
262277
auto Guarded<T, M, L>::tryLockedGuard() const
263278
-> std::optional<MutexGuard<T const, L<M>>> {
264-
auto lock = lock_type(_mutex, std::try_lock);
279+
auto lock = lock_type(_mutex, std::try_to_lock);
265280

266281
if (lock.owns_lock()) {
267282
return MutexGuard(_value, std::move(lock));

‎tests/Basics/GuardedTest.cpp

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,8 @@
2222

2323
#include "gtest/gtest.h"
2424

25+
#include <Mocks/Death_Test.h>
26+
2527
#include "Basics/Guarded.h"
2628
#include "Basics/Mutex.h"
2729
#include "Basics/MutexLocker.h"
@@ -51,7 +53,11 @@ class GuardedTest : public ::testing::Test {
5153
using Guarded = Guarded<V, Mutex, Lock>;
5254
};
5355

56+
template <typename T>
57+
class GuardedDeathTest : public GuardedTest<T> {};
58+
5459
TYPED_TEST_CASE_P(GuardedTest);
60+
TYPED_TEST_CASE_P(GuardedDeathTest);
5561

5662
// Test helper that acquires a lock;
5763
// then executes a callback that tries to acquire the same lock;
@@ -160,6 +166,46 @@ TYPED_TEST_P(GuardedTest, test_guard_waits_for_access) {
160166
testWaitForLock<GuardedType>(acquireGuard);
161167
}
162168

169+
TYPED_TEST_P(GuardedTest, test_guard_unlock_releases_mutex) {
170+
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
171+
EXPECT_EQ(1, guardedObj.copy().val);
172+
auto guard = guardedObj.getLockedGuard();
173+
EXPECT_EQ(1, guard.get().val);
174+
guard.get().val = 2;
175+
EXPECT_EQ(2, guard.get().val);
176+
guard.unlock();
177+
178+
auto threadStarted = std::atomic<bool>{false};
179+
auto couldAcquireLock = std::atomic<bool>{false};
180+
auto thr = std::thread([&] {
181+
threadStarted.store(true, std::memory_order_release);
182+
guardedObj.doUnderLock([&](auto&&) {
183+
couldAcquireLock.store(true, std::memory_order_release);
184+
});
185+
});
186+
187+
EXPECT_EQ(2, guardedObj.copy().val);
188+
while (!threadStarted.load(std::memory_order_acquire)) {
189+
// busy wait
190+
}
191+
// wait generously for the thread to try to get the lock and do something
192+
std::this_thread::sleep_for(std::chrono::milliseconds{1});
193+
EXPECT_TRUE(couldAcquireLock.load(std::memory_order_acquire));
194+
thr.join();
195+
}
196+
197+
TYPED_TEST_P(GuardedDeathTest, test_guard_unlock_releases_value) {
198+
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
199+
EXPECT_EQ(1, guardedObj.copy().val);
200+
auto guard = guardedObj.getLockedGuard();
201+
EXPECT_EQ(1, guard.get().val);
202+
guard.get().val = 2;
203+
EXPECT_EQ(2, guard.get().val);
204+
guard.unlock();
205+
206+
ASSERT_DEATH_CORE_FREE({ ASSERT_NE(2, guard.get().val); }, "");
207+
}
208+
163209
TYPED_TEST_P(GuardedTest, test_do_allows_access) {
164210
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
165211
{
@@ -252,6 +298,32 @@ TYPED_TEST_P(GuardedTest, test_try_allows_access) {
252298
}
253299
}
254300

301+
TYPED_TEST_P(GuardedTest, test_try_guard_allows_access) {
302+
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
303+
{
304+
// try is allowed to spuriously fail for no reason. But we expect it to
305+
// succeed at some point when noone holds the lock.
306+
bool didExecute = false;
307+
while (!didExecute) {
308+
if (auto guard = guardedObj.tryLockedGuard()) {
309+
EXPECT_EQ(1, guard->get().val);
310+
guard->get().val = 2;
311+
didExecute = true;
312+
EXPECT_EQ(2, guard->get().val);
313+
}
314+
315+
{
316+
auto guard = guardedObj.getLockedGuard();
317+
if (didExecute) {
318+
EXPECT_EQ(guard->val, 2);
319+
} else {
320+
EXPECT_EQ(guard->val, 1);
321+
}
322+
}
323+
}
324+
}
325+
}
326+
255327
TYPED_TEST_P(GuardedTest, test_try_fails_access) {
256328
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
257329
{
@@ -279,11 +351,40 @@ TYPED_TEST_P(GuardedTest, test_try_fails_access) {
279351
}
280352
}
281353

282-
REGISTER_TYPED_TEST_CASE_P(GuardedTest, test_copy_allows_access, test_copy_waits_for_access,
283-
test_assign_allows_access, test_assign_waits_for_access,
284-
test_guard_allows_access, test_guard_waits_for_access,
354+
TYPED_TEST_P(GuardedTest, test_try_guard_fails_access) {
355+
auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
356+
{
357+
auto guard = guardedObj.getLockedGuard();
358+
bool threadStarted = false;
359+
bool threadFinished = false;
360+
auto thr = std::thread([&] {
361+
threadStarted = true;
362+
bool didExecute = false;
363+
if (auto guard = guardedObj.tryLockedGuard()) {
364+
EXPECT_EQ(1, guard->get().val);
365+
guard->get().val = 2;
366+
didExecute = true;
367+
EXPECT_EQ(2, guard->get().val);
368+
}
369+
EXPECT_FALSE(didExecute);
370+
threadFinished = true;
371+
});
372+
thr.join();
373+
EXPECT_TRUE(threadStarted);
374+
EXPECT_TRUE(threadFinished);
375+
EXPECT_EQ(guard->val, 1);
376+
}
377+
}
378+
379+
REGISTER_TYPED_TEST_CASE_P(GuardedTest, test_copy_allows_access,
380+
test_copy_waits_for_access, test_assign_allows_access,
381+
test_assign_waits_for_access, test_guard_allows_access,
382+
test_guard_waits_for_access, test_guard_unlock_releases_mutex,
285383
test_do_allows_access, test_do_waits_for_access,
286-
test_try_allows_access, test_try_fails_access);
384+
test_try_allows_access, test_try_guard_allows_access,
385+
test_try_fails_access, test_try_guard_fails_access);
386+
387+
REGISTER_TYPED_TEST_CASE_P(GuardedDeathTest, test_guard_unlock_releases_value);
287388

288389
template <template <typename> typename T>
289390
struct ParamT {
@@ -297,5 +398,6 @@ using TestedTypes =
297398
std::pair<arangodb::Mutex, ParamT<std::unique_lock>>>;
298399

299400
INSTANTIATE_TYPED_TEST_CASE_P(GuardedTestInstantiation, GuardedTest, TestedTypes);
401+
INSTANTIATE_TYPED_TEST_CASE_P(GuardedDeathTestInstantiation, GuardedDeathTest, TestedTypes);
300402

301403
} // namespace arangodb::tests

‎tests/Mocks/Death_Test.h

Lines changed: 17 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -30,22 +30,32 @@
3030
/// So this thin macro wraps around the GTEST :: EXPECT_DEATH macro and disables coredumps
3131
/// only within the expected forked process
3232

33-
#ifndef ARANGODB_TESTS_MOCKS_DEATH_TEST_CHANGER_H
34-
#define ARANGODB_TESTS_MOCKS_DEATH_TEST_CHANGER_H 1
33+
#pragma once
3534

3635
#ifndef _WIN32
3736

3837
#include <sys/resource.h>
3938

4039
// Enabled on Linux and Mac
4140

41+
inline void disableCoredump() {
42+
auto core_limit = rlimit{};
43+
core_limit.rlim_cur = 0;
44+
core_limit.rlim_max = 0;
45+
setrlimit(RLIMIT_CORE, &core_limit);
46+
}
47+
4248
#define EXPECT_DEATH_CORE_FREE(func, assertion) \
4349
EXPECT_DEATH( \
4450
[&]() { \
45-
rlimit core_limit; \
46-
core_limit.rlim_cur = 0; \
47-
core_limit.rlim_max = 0; \
48-
setrlimit(RLIMIT_CORE, &core_limit); \
51+
disableCoredump(); \
52+
func; \
53+
}(), \
54+
assertion)
55+
#define ASSERT_DEATH_CORE_FREE(func, assertion) \
56+
ASSERT_DEATH( \
57+
[&]() { \
58+
disableCoredump(); \
4959
func; \
5060
}(), \
5161
assertion)
@@ -57,7 +67,6 @@
5767
// please feel free to fix it here.
5868

5969
#define EXPECT_DEATH_CORE_FREE(func, assertion) EXPECT_TRUE(true)
70+
#define ASSERT_DEATH_CORE_FREE(func, assertion) ASSERT_TRUE(true)
6071

6172
#endif
62-
63-
#endif

0 commit comments

Comments
 (0)
0