add Synchronized::withLock() methods

Summary:
Add withLock() and related methods for invoking a lambda function while the
lock is held.  This is sometimes more convenient than opening a new scope and
using lock().

withLock() also retains some of the benefits of the SYNCHRONIZED macro in that
it forces users to put their critical section code in a new scope, making the
critical section more visibly distinct in the code.  This also encourages users
to only put necessary work inside the critical section, and do to other work
once the lock is released.

This also adds a LockedGuardPtr class, which is a slightly cheaper version of
LockedPtr.  The relationship between LockedGuardPtr and LockedPtr is very much
like that between std::lock_guard and std::unique_lock.  It saves a branch in
the destructor, and in the case of std::mutex it also saves a small amount of
storage space (since LockedPtr is specialized for std::mutex to also store a
std::unique_lock).

Reviewed By: yfeldblum, djwatson

Differential Revision: D3530368

fbshipit-source-id: 72a4f457b3f18e8e8f4cc6713218f6882bb89818

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index f36bf5599c8606a868e47de7a980fce13d44482a..01619bc2e0f0046e7c158ed7f08a95b8cb3d1396 100644 (file)
--- a/folly/Synchronized.h
+++ b/folly/Synchronized.h
@@ -39,6 +39,8 @@ template <class LockedType, class Mutex, class LockPolicy>
 class LockedPtrBase;
 template <class LockedType, class LockPolicy>
 class LockedPtr;
 class LockedPtrBase;
 template <class LockedType, class LockPolicy>
 class LockedPtr;

+template <class LockedType, class LockPolicy = LockPolicyExclusive>
+class LockedGuardPtr;

 
 /**
  * SynchronizedBase is a helper parent class for Synchronized<T>.
 
 /**
  * SynchronizedBase is a helper parent class for Synchronized<T>.


@@ -112,6 +114,75 @@ class SynchronizedBase<Subclass, true> {
       const std::chrono::duration<Rep, Period>& timeout) const {
     return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
   }
       const std::chrono::duration<Rep, Period>& timeout) const {
     return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
   }

+
+  /*
+   * Note: C++ 17 adds guaranteed copy elision.  (http://wg21.link/P0135)
+   * Once compilers support this, it would be nice to add wguard() and rguard()
+   * methods that return LockedGuardPtr objects.
+   */
+
+  /**
+   * Invoke a function while holding the lock exclusively.
+   *
+   * A reference to the datum will be passed into the function as its only
+   * argument.
+   *
+   * This can be used with a lambda argument for easily defining small critical
+   * sections in the code.  For example:
+   *
+   *   auto value = obj.withWLock([](auto& data) {
+   *     data.doStuff();
+   *     return data.getValue();
+   *   });
+   */
+  template <class Function>
+  auto withWLock(Function&& function) {
+    LockedGuardPtr<Subclass, LockPolicyExclusive> guardPtr(
+        static_cast<Subclass*>(this));
+    return function(*guardPtr);
+  }
+  template <class Function>
+  auto withWLock(Function&& function) const {
+    LockedGuardPtr<const Subclass, LockPolicyExclusive> guardPtr(
+        static_cast<const Subclass*>(this));
+    return function(*guardPtr);
+  }
+
+  /**
+   * Invoke a function while holding the lock exclusively.
+   *
+   * This is similar to withWLock(), but the function will be passed a
+   * LockedPtr rather than a reference to the data itself.
+   *
+   * This allows scopedUnlock() to be called on the LockedPtr argument if
+   * desired.
+   */
+  template <class Function>
+  auto withWLockPtr(Function&& function) {
+    return function(wlock());
+  }
+  template <class Function>
+  auto withWLockPtr(Function&& function) const {
+    return function(wlock());
+  }
+
+  /**
+   * Invoke a function while holding an the lock in shared mode.
+   *
+   * A const reference to the datum will be passed into the function as its
+   * only argument.
+   */
+  template <class Function>
+  auto withRLock(Function&& function) const {
+    LockedGuardPtr<const Subclass, LockPolicyShared> guardPtr(
+        static_cast<const Subclass*>(this));
+    return function(*guardPtr);
+  }
+
+  template <class Function>
+  auto withRLockPtr(Function&& function) const {
+    return function(rlock());
+  }

 };
 
 /**
 };
 
 /**


@@ -160,6 +231,57 @@ class SynchronizedBase<Subclass, false> {
   ConstLockedPtr lock(const std::chrono::duration<Rep, Period>& timeout) const {
     return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
   }
   ConstLockedPtr lock(const std::chrono::duration<Rep, Period>& timeout) const {
     return ConstLockedPtr(static_cast<const Subclass*>(this), timeout);
   }

+
+  /*
+   * Note: C++ 17 adds guaranteed copy elision.  (http://wg21.link/P0135)
+   * Once compilers support this, it would be nice to add guard() methods that
+   * return LockedGuardPtr objects.
+   */
+
+  /**
+   * Invoke a function while holding the lock.
+   *
+   * A reference to the datum will be passed into the function as its only
+   * argument.
+   *
+   * This can be used with a lambda argument for easily defining small critical
+   * sections in the code.  For example:
+   *
+   *   auto value = obj.withLock([](auto& data) {
+   *     data.doStuff();
+   *     return data.getValue();
+   *   });
+   */
+  template <class Function>
+  auto withLock(Function&& function) {
+    LockedGuardPtr<Subclass, LockPolicyExclusive> guardPtr(
+        static_cast<Subclass*>(this));
+    return function(*guardPtr);
+  }
+  template <class Function>
+  auto withLock(Function&& function) const {
+    LockedGuardPtr<const Subclass, LockPolicyExclusive> guardPtr(
+        static_cast<const Subclass*>(this));
+    return function(*guardPtr);
+  }
+
+  /**
+   * Invoke a function while holding the lock exclusively.
+   *
+   * This is similar to withWLock(), but the function will be passed a
+   * LockedPtr rather than a reference to the data itself.
+   *
+   * This allows scopedUnlock() and getUniqueLock() to be called on the
+   * LockedPtr argument.
+   */
+  template <class Function>
+  auto withLockPtr(Function&& function) {
+    return function(lock());
+  }
+  template <class Function>
+  auto withLockPtr(Function&& function) const {
+    return function(lock());
+  }

 };
 
 /**
 };
 
 /**


@@ -457,6 +579,8 @@ struct Synchronized : public SynchronizedBase<
   friend class folly::LockedPtrBase;
   template <class LockedType, class LockPolicy>
   friend class folly::LockedPtr;
   friend class folly::LockedPtrBase;
   template <class LockedType, class LockPolicy>
   friend class folly::LockedPtr;

+  template <class LockedType, class LockPolicy>
+  friend class folly::LockedGuardPtr;

 
   /**
    * Helper constructors to enable Synchronized for
 
   /**
    * Helper constructors to enable Synchronized for


@@ -481,6 +605,27 @@ struct Synchronized : public SynchronizedBase<
 template <class SynchronizedType, class LockPolicy>
 class ScopedUnlocker;
 
 template <class SynchronizedType, class LockPolicy>
 class ScopedUnlocker;
 

+namespace detail {
+/*
+ * A helper alias that resolves to "const T" if the template parameter
+ * is a const Synchronized<T>, or "T" if the parameter is not const.
+ */
+template <class SynchronizedType>
+using SynchronizedDataType = typename std::conditional<
+    std::is_const<SynchronizedType>::value,
+    typename SynchronizedType::DataType const,
+    typename SynchronizedType::DataType>::type;
+/*
+ * A helper alias that resolves to a ConstLockedPtr if the template parameter
+ * is a const Synchronized<T>, or a LockedPtr if the parameter is not const.
+ */
+template <class SynchronizedType>
+using LockedPtrType = typename std::conditional<
+    std::is_const<SynchronizedType>::value,
+    typename SynchronizedType::ConstLockedPtr,
+    typename SynchronizedType::LockedPtr>::type;
+} // detail
+

 /**
  * A helper base class for implementing LockedPtr.
  *
 /**
  * A helper base class for implementing LockedPtr.
  *


@@ -701,10 +846,7 @@ class LockedPtr : public LockedPtrBase<
       LockPolicy>;
   using UnlockerData = typename Base::UnlockerData;
   // CDataType is the DataType with the appropriate const-qualification
       LockPolicy>;
   using UnlockerData = typename Base::UnlockerData;
   // CDataType is the DataType with the appropriate const-qualification

-  using CDataType = typename std::conditional<
-      std::is_const<SynchronizedType>::value,
-      typename SynchronizedType::DataType const,
-      typename SynchronizedType::DataType>::type;
+  using CDataType = detail::SynchronizedDataType<SynchronizedType>;

 
  public:
   using DataType = typename SynchronizedType::DataType;
 
  public:
   using DataType = typename SynchronizedType::DataType;


@@ -812,17 +954,61 @@ class LockedPtr : public LockedPtrBase<
   }
 };
 
   }
 };
 

-namespace detail {
-/*
- * A helper alias to select a ConstLockedPtr if the template parameter is a
- * const Synchronized<T>, or a LockedPtr if the parameter is not const.
+/**
+ * LockedGuardPtr is a simplified version of LockedPtr.
+ *
+ * It is non-movable, and supports fewer features than LockedPtr.  However, it
+ * is ever-so-slightly more performant than LockedPtr.  (The destructor can
+ * unconditionally release the lock, without requiring a conditional branch.)
+ *
+ * The relationship between LockedGuardPtr and LockedPtr is similar to that
+ * between std::lock_guard and std::unique_lock.

  */
  */

-template <class SynchronizedType>
-using LockedPtrType = typename std::conditional<
-    std::is_const<SynchronizedType>::value,
-    typename SynchronizedType::ConstLockedPtr,
-    typename SynchronizedType::LockedPtr>::type;
-} // detail
+template <class SynchronizedType, class LockPolicy>
+class LockedGuardPtr {
+ private:
+  // CDataType is the DataType with the appropriate const-qualification
+  using CDataType = detail::SynchronizedDataType<SynchronizedType>;
+
+ public:
+  using DataType = typename SynchronizedType::DataType;
+  using MutexType = typename SynchronizedType::MutexType;
+  using Synchronized = typename std::remove_const<SynchronizedType>::type;
+
+  LockedGuardPtr() = delete;
+
+  /**
+   * Takes a Synchronized<T> and locks it.
+   */
+  explicit LockedGuardPtr(SynchronizedType* parent) : parent_(parent) {
+    LockPolicy::lock(parent_->mutex_);
+  }
+
+  /**
+   * Destructor releases.
+   */
+  ~LockedGuardPtr() {
+    LockPolicy::unlock(parent_->mutex_);
+  }
+
+  /**
+   * Access the locked data.
+   */
+  CDataType* operator->() const {
+    return &parent_->datum_;
+  }
+
+  /**
+   * Access the locked data.
+   */
+  CDataType& operator*() const {
+    return parent_->datum_;
+  }
+
+ private:
+  // This is the entire state of LockedGuardPtr.
+  SynchronizedType* const parent_{nullptr};
+};

 
 /**
  * Acquire locks for multiple Synchronized<T> objects, in a deadlock-safe
 
 /**
  * Acquire locks for multiple Synchronized<T> objects, in a deadlock-safe

diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp
index 3c07f57edef4d4dea7060643229e52c42fc969b3..de45a4045b97d237dec722a4654f146fb99c9262 100644 (file)
--- a/folly/test/SynchronizedTest.cpp
+++ b/folly/test/SynchronizedTest.cpp
@@ -59,6 +59,10 @@ TYPED_TEST(SynchronizedTest, Basic) {
   testBasic<TypeParam>();
 }
 
   testBasic<TypeParam>();
 }
 

+TYPED_TEST(SynchronizedTest, WithLock) {
+  testWithLock<TypeParam>();
+}
+

 TYPED_TEST(SynchronizedTest, Deprecated) {
   testDeprecated<TypeParam>();
 }
 TYPED_TEST(SynchronizedTest, Deprecated) {
   testDeprecated<TypeParam>();
 }

diff --git a/folly/test/SynchronizedTestLib-inl.h b/folly/test/SynchronizedTestLib-inl.h
index 3166de8ac4c63d75ad389228cd507a1f624aa5f2..d4e027cfc4a0da7e3f4a6b3db1ebc83b3f94f6b7 100644 (file)
--- a/folly/test/SynchronizedTestLib-inl.h
+++ b/folly/test/SynchronizedTestLib-inl.h
@@ -108,6 +108,7 @@ template <class Mutex>
 typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
 testBasicImpl() {
   folly::Synchronized<std::vector<int>, Mutex> obj;
 typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
 testBasicImpl() {
   folly::Synchronized<std::vector<int>, Mutex> obj;

+  const auto& constObj = obj;

 
   obj.wlock()->resize(1000);
 
 
   obj.wlock()->resize(1000);
 


@@ -141,7 +142,6 @@ testBasicImpl() {
   obj.wlock()->front() = 2;
 
   {
   obj.wlock()->front() = 2;
 
   {

-    const auto& constObj = obj;

     // contextualLock() on a const reference should grab a shared lock
     auto lockedObj = constObj.contextualLock();
     EXPECT_EQ(2, lockedObj->front());
     // contextualLock() on a const reference should grab a shared lock
     auto lockedObj = constObj.contextualLock();
     EXPECT_EQ(2, lockedObj->front());


@@ -160,6 +160,7 @@ template <class Mutex>
 typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
 testBasicImpl() {
   folly::Synchronized<std::vector<int>, Mutex> obj;
 typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
 testBasicImpl() {
   folly::Synchronized<std::vector<int>, Mutex> obj;

+  const auto& constObj = obj;

 
   obj.lock()->resize(1000);
 
 
   obj.lock()->resize(1000);
 


@@ -178,6 +179,12 @@ testBasicImpl() {
       EXPECT_EQ(1001, obj.lock()->size());
     }
   }
       EXPECT_EQ(1001, obj.lock()->size());
     }
   }

+  {
+    auto lockedObj = constObj.lock();
+    EXPECT_EQ(1001, lockedObj->size());
+    EXPECT_EQ(10, lockedObj->back());
+    EXPECT_EQ(1000, obj2.lock()->size());
+  }

 
   obj.lock()->front() = 2;
 
 
   obj.lock()->front() = 2;
 


@@ -193,6 +200,160 @@ void testBasic() {
   testBasicImpl<Mutex>();
 }
 
   testBasicImpl<Mutex>();
 }
 

+// testWithLock() version for shared lock types
+template <class Mutex>
+typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
+testWithLock() {
+  folly::Synchronized<std::vector<int>, Mutex> obj;
+  const auto& constObj = obj;
+
+  // Test withWLock() and withRLock()
+  obj.withWLock([](std::vector<int>& lockedObj) {
+    lockedObj.resize(1000);
+    lockedObj.push_back(10);
+    lockedObj.push_back(11);
+  });
+  obj.withWLock([](const std::vector<int>& lockedObj) {
+    EXPECT_EQ(1002, lockedObj.size());
+  });
+  constObj.withWLock([](const std::vector<int>& lockedObj) {
+    EXPECT_EQ(1002, lockedObj.size());
+    EXPECT_EQ(11, lockedObj.back());
+  });
+  obj.withRLock([](const std::vector<int>& lockedObj) {
+    EXPECT_EQ(1002, lockedObj.size());
+    EXPECT_EQ(11, lockedObj.back());
+  });
+  constObj.withRLock([](const std::vector<int>& lockedObj) {
+    EXPECT_EQ(1002, lockedObj.size());
+  });
+
+#if __cpp_generic_lambdas >= 201304
+  obj.withWLock([](auto& lockedObj) { lockedObj.push_back(12); });
+  obj.withWLock(
+      [](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
+  constObj.withWLock([](const auto& lockedObj) {
+    EXPECT_EQ(1003, lockedObj.size());
+    EXPECT_EQ(12, lockedObj.back());
+  });
+  obj.withRLock([](const auto& lockedObj) {
+    EXPECT_EQ(1003, lockedObj.size());
+    EXPECT_EQ(12, lockedObj.back());
+  });
+  constObj.withRLock(
+      [](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
+  obj.withWLock([](auto& lockedObj) { lockedObj.pop_back(); });
+#endif
+
+  // Test withWLockPtr() and withRLockPtr()
+  using SynchType = folly::Synchronized<std::vector<int>, Mutex>;
+#if __cpp_generic_lambdas >= 201304
+  obj.withWLockPtr([](auto&& lockedObj) { lockedObj->push_back(13); });
+  obj.withRLockPtr([](auto&& lockedObj) {
+    EXPECT_EQ(1003, lockedObj->size());
+    EXPECT_EQ(13, lockedObj->back());
+  });
+  constObj.withRLockPtr([](auto&& lockedObj) {
+    EXPECT_EQ(1003, lockedObj->size());
+    EXPECT_EQ(13, lockedObj->back());
+  });
+  obj.withWLockPtr([&](auto&& lockedObj) {
+    lockedObj->push_back(14);
+    {
+      auto unlocker = lockedObj.scopedUnlock();
+      obj.wlock()->push_back(15);
+    }
+    EXPECT_EQ(15, lockedObj->back());
+  });
+  constObj.withWLockPtr([](auto&& lockedObj) {
+    EXPECT_EQ(1005, lockedObj->size());
+    EXPECT_EQ(15, lockedObj->back());
+  });
+#else
+  obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
+    lockedObj->push_back(13);
+    lockedObj->push_back(14);
+    lockedObj->push_back(15);
+  });
+#endif
+
+  obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
+    lockedObj->push_back(16);
+    EXPECT_EQ(1006, lockedObj->size());
+  });
+  constObj.withWLockPtr([](typename SynchType::ConstWLockedPtr&& lockedObj) {
+    EXPECT_EQ(1006, lockedObj->size());
+    EXPECT_EQ(16, lockedObj->back());
+  });
+  obj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
+    EXPECT_EQ(1006, lockedObj->size());
+    EXPECT_EQ(16, lockedObj->back());
+  });
+  constObj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
+    EXPECT_EQ(1006, lockedObj->size());
+    EXPECT_EQ(16, lockedObj->back());
+  });
+}
+
+// testWithLock() version for non-shared lock types
+template <class Mutex>
+typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
+testWithLock() {
+  folly::Synchronized<std::vector<int>, Mutex> obj;
+
+  // Test withLock()
+  obj.withLock([](std::vector<int>& lockedObj) {
+    lockedObj.resize(1000);
+    lockedObj.push_back(10);
+    lockedObj.push_back(11);
+  });
+  obj.withLock([](const std::vector<int>& lockedObj) {
+    EXPECT_EQ(1002, lockedObj.size());
+  });
+
+#if __cpp_generic_lambdas >= 201304
+  obj.withLock([](auto& lockedObj) { lockedObj.push_back(12); });
+  obj.withLock(
+      [](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
+  obj.withLock([](auto& lockedObj) { lockedObj.pop_back(); });
+#endif
+
+  // Test withLockPtr()
+  using SynchType = folly::Synchronized<std::vector<int>, Mutex>;
+#if __cpp_generic_lambdas >= 201304
+  obj.withLockPtr([](auto&& lockedObj) { lockedObj->push_back(13); });
+  obj.withLockPtr([](auto&& lockedObj) {
+    EXPECT_EQ(1003, lockedObj->size());
+    EXPECT_EQ(13, lockedObj->back());
+  });
+  obj.withLockPtr([&](auto&& lockedObj) {
+    lockedObj->push_back(14);
+    {
+      auto unlocker = lockedObj.scopedUnlock();
+      obj.lock()->push_back(15);
+    }
+    EXPECT_EQ(1005, lockedObj->size());
+    EXPECT_EQ(15, lockedObj->back());
+  });
+#else
+  obj.withLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
+    lockedObj->push_back(13);
+    lockedObj->push_back(14);
+    lockedObj->push_back(15);
+  });
+#endif
+
+  obj.withLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
+    lockedObj->push_back(16);
+    EXPECT_EQ(1006, lockedObj->size());
+  });
+  const auto& constObj = obj;
+  constObj.withLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
+    EXPECT_EQ(1006, lockedObj->size());
+    EXPECT_EQ(16, lockedObj->back());
+  });
+}
+

 // Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs
 template <class Mutex>
 void testDeprecated() {
 // Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs
 template <class Mutex>
 void testDeprecated() {