Added assign and copy

arangodb · mchacki · Jul 14, 2021 · Jul 9, 2019 · Jul 18, 2019 · Oct 9, 2019
commit 2d843dcb0254aaa31978ec16c8d314bd7ffa087a
diff --git a/lib/Basics/Guarded.h b/lib/Basics/Guarded.h
@@ -61,6 +61,17 @@
  *
  * getLockedGuard() will lock the mutex, and mutexGuard will release it upon
  * destruction.
+ *
+ * For simple access, there are copy() and store():
+ *
+ *     UnderGuard value = guarded.copy();
+ *     guarded.store(UnderGuard{3});
+ *
+ * If copy/assign don't suffice for some reason - e.g. because you want to:
+ *  - "try" for the lock, or
+ *  - access to one specific member or anything like that, or
+ *  - get/modify a non-copy-constructible or non-copy-assignable value,
+ *  use any of the more general methods described above instead.
  */
 
 namespace arangodb {
@@ -112,7 +123,7 @@ auto MutexGuard<T, L>::operator->() const noexcept -> T const* {
   return std::addressof(get());
 }
 
-template <class T, class M = std::mutex, template<class> class L = std::unique_lock>
+template <class T, class M = std::mutex, template <class> class L = std::unique_lock>
 class Guarded {
  public:
   using value_type = T;
@@ -141,6 +152,14 @@ class Guarded {
             class Q = std::conditional_t<std::is_void_v<R>, std::monostate, R>>
   [[nodiscard]] auto tryUnderLock(F&& callback) const -> std::optional<Q>;
 
+  // get a copy of the value, made under the lock.
+  template <typename U = T, std::enable_if_t<std::is_copy_constructible_v<U>, int> = 0>
+  auto copy() const -> T;
+
+  // assign a new value using operator=, under the lock.
+  template <class U, std::enable_if_t<std::is_assignable_v<T, U>, int> = 0>
+  void assign(U&&);
+
   auto getLockedGuard() -> MutexGuard<value_type, lock_type>;
   auto getLockedGuard() const -> MutexGuard<value_type const, lock_type>;
 
@@ -150,20 +169,20 @@ class Guarded {
 
  private:
   value_type _value;
-  mutex_type _mutex;
+  mutable mutex_type _mutex;
 };
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 Guarded<T, M, L>::Guarded(T&& value) : _value{std::move(value)}, _mutex{} {}
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 template <typename... Args>
 Guarded<T, M, L>::Guarded(Args... args) : _value{args...}, _mutex{} {}
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 template <class F, class R>
 auto Guarded<T, M, L>::doUnderLock(F&& callback) -> R {
-  auto guard = L<M>(_mutex);
+  auto guard = lock_type(_mutex);
 
   if constexpr (!std::is_void_v<R>) {
     return std::forward<F>(callback)(_value);
@@ -173,10 +192,10 @@ auto Guarded<T, M, L>::doUnderLock(F&& callback) -> R {
   }
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 template <class F, class R>
 auto Guarded<T, M, L>::doUnderLock(F&& callback) const -> R {
-  auto guard = L<M>(_mutex);
+  auto guard = lock_type(_mutex);
 
   if constexpr (!std::is_void_v<R>) {
     return std::forward<F>(callback)(_value);
@@ -186,10 +205,10 @@ auto Guarded<T, M, L>::doUnderLock(F&& callback) const -> R {
   }
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 template <class F, class R, class Q>
 auto Guarded<T, M, L>::tryUnderLock(F&& callback) -> std::optional<Q> {
-  auto guard = L<M>(_mutex, std::try_to_lock);
+  auto guard = lock_type(_mutex, std::try_to_lock);
 
   if (guard.owns_lock()) {
     if constexpr (!std::is_void_v<R>) {
@@ -203,7 +222,7 @@ auto Guarded<T, M, L>::tryUnderLock(F&& callback) -> std::optional<Q> {
   }
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 template <class F, class R, class Q>
 auto Guarded<T, M, L>::tryUnderLock(F&& callback) const -> std::optional<Q> {
   auto guard = lock_type(_mutex, std::try_to_lock);
@@ -220,17 +239,31 @@ auto Guarded<T, M, L>::tryUnderLock(F&& callback) const -> std::optional<Q> {
   }
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
+template <typename U, std::enable_if_t<std::is_copy_constructible_v<U>, int>>
+auto Guarded<T, M, L>::copy() const -> T {
+  auto guard = lock_type(this->_mutex);
+  return _value;
+}
+
+template <class T, class M, template <class> class L>
+template <class U, std::enable_if_t<std::is_assignable_v<T, U>, int>>
+void Guarded<T, M, L>::assign(U&& value) {
+  auto guard = lock_type(_mutex);
+  _value = std::forward<U>(value);
+}
+
+template <class T, class M, template <class> class L>
 auto Guarded<T, M, L>::getLockedGuard() -> MutexGuard<T, L<M>> {
-  return MutexGuard(_value, lock_type{_mutex});
+  return MutexGuard(_value, lock_type(_mutex));
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 auto Guarded<T, M, L>::getLockedGuard() const -> MutexGuard<T const, L<M>> {
   return MutexGuard(_value, lock_type(_mutex));
 }
 
-template <class T, class M, template<class> class L>
+template <class T, class M, template <class> class L>
 auto Guarded<T, M, L>::tryLockedGuard() -> std::optional<MutexGuard<T, L<M>>> {
   auto lock = lock_type(_mutex, std::try_lock);
 
@@ -241,8 +274,9 @@ auto Guarded<T, M, L>::tryLockedGuard() -> std::optional<MutexGuard<T, L<M>>> {
   }
 }
 
-template <class T, class M, template<class> class L>
-auto Guarded<T, M, L>::tryLockedGuard() const -> std::optional<MutexGuard<T const, L<M>>> {
+template <class T, class M, template <class> class L>
+auto Guarded<T, M, L>::tryLockedGuard() const
+    -> std::optional<MutexGuard<T const, L<M>>> {
   auto lock = lock_type(_mutex, std::try_lock);
 
   if (lock.owns_lock()) {

diff --git a/tests/Basics/GuardedTest.cpp b/tests/Basics/GuardedTest.cpp
@@ -41,25 +41,56 @@ struct UnderGuardAtomic {
 template <typename T>
 class GuardedTest : public ::testing::Test {
  protected:
-
   using Mutex = typename T::first_type;
-  template<typename M>
+  template <typename M>
   using Lock = typename T::second_type::template instantiate<M>;
-  template<typename V>
+  template <typename V>
   using Guarded = Guarded<V, Mutex, Lock>;
 };
 
 TYPED_TEST_CASE_P(GuardedTest);
 
+TYPED_TEST_P(GuardedTest, test_copy_allows_access) {
+  auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
+  auto const value = guardedObj.copy();
+  EXPECT_EQ(1, value.val);
+  EXPECT_EQ(1, guardedObj.copy().val);
+}
+
+TYPED_TEST_P(GuardedTest, test_copy_waits_for_access) {
+  // TODO Get a lock first, then assert that trying to copy waits for the lock
+  //      to be released.
+}
+
+TYPED_TEST_P(GuardedTest, test_assign_allows_access) {
+  auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
+  EXPECT_EQ(1, guardedObj.copy().val);
+  // move assignment
+  guardedObj.assign(UnderGuard{2});
+  EXPECT_EQ(2, guardedObj.copy().val);
+  // copy assignment
+  auto const val = UnderGuard{3};
+  guardedObj.assign(val);
+  EXPECT_EQ(3, guardedObj.copy().val);
+}
+
+TYPED_TEST_P(GuardedTest, test_assign_waits_for_access) {
+  // TODO Get a lock first, then assert that trying to assign waits for the lock
+  //      to be released.
+}
+
 TYPED_TEST_P(GuardedTest, test_guard_allows_access) {
   auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{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);
   }
+  EXPECT_EQ(2, guardedObj.copy().val);
 }
+
 TYPED_TEST_P(GuardedTest, test_guard_waits_for_access) {
   auto guardedObj = typename TestFixture::template Guarded<UnderGuard>{1};
   // TODO Get a lock first, then assert that trying to get a guard waits for the
@@ -96,13 +127,14 @@ TYPED_TEST_P(GuardedTest, test_do_waits_for_access) {
       thr = std::thread([&] {
         threadStarted.store(true, std::memory_order_release);
         bool didExecute = false;
-        auto const res = guardedObj.doUnderLock([&didExecute](UnderGuardAtomic& obj) -> std::optional<std::monostate> {
-          EXPECT_EQ(1, obj.val.load(std::memory_order_relaxed));
-          obj.val.store(2, std::memory_order_release);
-          didExecute = true;
-          EXPECT_EQ(2, obj.val.load(std::memory_order_relaxed));
-          return std::monostate{};
-        });
+        auto const res = guardedObj.doUnderLock(
+            [&didExecute](UnderGuardAtomic& obj) -> std::optional<std::monostate> {
+              EXPECT_EQ(1, obj.val.load(std::memory_order_relaxed));
+              obj.val.store(2, std::memory_order_release);
+              didExecute = true;
+              EXPECT_EQ(2, obj.val.load(std::memory_order_relaxed));
+              return std::monostate{};
+            });
         static_assert(std::is_same_v<std::optional<std::monostate> const, decltype(res)>);
         EXPECT_TRUE(res.has_value());
         EXPECT_TRUE(didExecute);
@@ -184,20 +216,21 @@ TYPED_TEST_P(GuardedTest, test_try_fails_access) {
   }
 }
 
-REGISTER_TYPED_TEST_CASE_P(GuardedTest, test_guard_allows_access, test_guard_waits_for_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,
                            test_do_allows_access, test_do_waits_for_access,
                            test_try_allows_access, test_try_fails_access);
 
-template<template <typename> typename T>
+template <template <typename> typename T>
 struct ParamT {
   template <typename U>
   using instantiate = T<U>;
 };
 
-using TestedTypes = ::testing::Types<
-    std::pair<std::mutex, ParamT<std::unique_lock>>,
-    std::pair<arangodb::Mutex, ParamT<std::unique_lock>>
->;
+using TestedTypes =
+    ::testing::Types<std::pair<std::mutex, ParamT<std::unique_lock>>,
+                     std::pair<arangodb::Mutex, ParamT<std::unique_lock>>>;
 
 INSTANTIATE_TYPED_TEST_CASE_P(GuardedTestInstantiation, GuardedTest, TestedTypes);