Adding upgradable locks to Synchronized
authorAaryaman Sagar <aary@instagram.com>
Fri, 19 Aug 2016 02:32:18 +0000 (19:32 -0700)
committerFacebook Github Bot 1 <facebook-github-bot-1-bot@fb.com>
Fri, 19 Aug 2016 02:38:32 +0000 (19:38 -0700)
Summary: This diff adds support for upgradeable locks to folly::Synchronized

Reviewed By: yfeldblum

Differential Revision: D3683205

fbshipit-source-id: 1b91ab07076566b4e5b535f2a2dbe1c8d9f3d1c2

folly/Synchronized.h
folly/test/SynchronizedTest.cpp

index 10c711c..beef1b7 100644 (file)
@@ -25,6 +25,7 @@
 
 #pragma once
 
+#include <folly/Likely.h>
 #include <folly/LockTraits.h>
 #include <folly/Preprocessor.h>
 #include <folly/SharedMutex.h>
@@ -42,13 +43,30 @@ class LockedPtr;
 template <class LockedType, class LockPolicy = LockPolicyExclusive>
 class LockedGuardPtr;
 
+/**
+ * Public version of LockInterfaceDispatcher that contains the MutexLevel enum
+ * for the passed in mutex type
+ *
+ * This is decoupled from MutexLevelValueImpl in LockTraits.h because this
+ * ensures that a heterogenous mutex with a different API can be used.  For
+ * example - if a mutex does not have a lock_shared() method but the
+ * LockTraits specialization for it supports a static non member
+ * lock_shared(Mutex&) it can be used as a shared mutex and will provide
+ * rlock() and wlock() functions.
+ */
+template <class Mutex>
+using MutexLevelValue = detail::MutexLevelValueImpl<
+    true,
+    LockTraits<Mutex>::is_shared,
+    LockTraits<Mutex>::is_upgrade>;
+
 /**
  * SynchronizedBase is a helper parent class for Synchronized<T>.
  *
  * It provides wlock() and rlock() methods for shared mutex types,
  * or lock() methods for purely exclusive mutex types.
  */
-template <class Subclass, bool is_shared>
+template <class Subclass, detail::MutexLevel level>
 class SynchronizedBase;
 
 /**
@@ -58,7 +76,7 @@ class SynchronizedBase;
  * accessing the data.
  */
 template <class Subclass>
-class SynchronizedBase<Subclass, true> {
+class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
  public:
   using LockedPtr = ::folly::LockedPtr<Subclass, LockPolicyExclusive>;
   using ConstWLockedPtr =
@@ -185,6 +203,102 @@ class SynchronizedBase<Subclass, true> {
   }
 };
 
+/**
+ * SynchronizedBase specialization for upgrade mutex types.
+ *
+ * This class provides all the functionality provided by the SynchronizedBase
+ * specialization for shared mutexes and a ulock() method that returns an
+ * upgradable lock RAII proxy
+ */
+template <class Subclass>
+class SynchronizedBase<Subclass, detail::MutexLevel::UPGRADE>
+    : public SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
+ public:
+  using UpgradeLockedPtr = ::folly::LockedPtr<Subclass, LockPolicyUpgrade>;
+  using ConstUpgradeLockedPtr =
+      ::folly::LockedPtr<const Subclass, LockPolicyUpgrade>;
+  using UpgradeLockedGuardPtr =
+      ::folly::LockedGuardPtr<Subclass, LockPolicyUpgrade>;
+  using ConstUpgradeLockedGuardPtr =
+      ::folly::LockedGuardPtr<const Subclass, LockPolicyUpgrade>;
+
+  /**
+   * Acquire an upgrade lock and return a LockedPtr that can be used to safely
+   * access the datum
+   *
+   * And the const version
+   */
+  UpgradeLockedPtr ulock() {
+    return UpgradeLockedPtr(static_cast<Subclass*>(this));
+  }
+  ConstUpgradeLockedPtr ulock() const {
+    return ConstUpgradeLockedPtr(static_cast<const Subclass*>(this));
+  }
+
+  /**
+   * Acquire an upgrade lock and return a LockedPtr that can be used to safely
+   * access the datum
+   *
+   * And the const version
+   */
+  template <class Rep, class Period>
+  UpgradeLockedPtr ulock(const std::chrono::duration<Rep, Period>& timeout) {
+    return UpgradeLockedPtr(static_cast<Subclass*>(this), timeout);
+  }
+  template <class Rep, class Period>
+  UpgradeLockedPtr ulock(
+      const std::chrono::duration<Rep, Period>& timeout) const {
+    return ConstUpgradeLockedPtr(static_cast<const Subclass*>(this), timeout);
+  }
+
+  /**
+   * 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.withULock([](auto& data) {
+   *     data.doStuff();
+   *     return data.getValue();
+   *   });
+   *
+   * This is probably not the function you want.  If the intent is to read the
+   * data object and determine whether you should upgrade to a write lock then
+   * the withULockPtr() method should be called instead, since it gives access
+   * to the LockedPtr proxy (which can be upgraded via the
+   * moveFromUpgradeToWrite() method)
+   */
+  template <class Function>
+  auto withULock(Function&& function) const {
+    ConstUpgradeLockedGuardPtr guardPtr(static_cast<const Subclass*>(this));
+    return function(*guardPtr);
+  }
+
+  /**
+   * Invoke a function while holding the lock exclusively.
+   *
+   * This is similar to withULock(), 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.
+   *
+   * This also allows you to upgrade the LockedPtr proxy to a write state so
+   * that changes can be made to the underlying data
+   */
+  template <class Function>
+  auto withULockPtr(Function&& function) {
+    return function(ulock());
+  }
+  template <class Function>
+  auto withULockPtr(Function&& function) const {
+    return function(ulock());
+  }
+};
+
 /**
  * SynchronizedBase specialization for non-shared mutex types.
  *
@@ -192,7 +306,7 @@ class SynchronizedBase<Subclass, true> {
  * data.
  */
 template <class Subclass>
-class SynchronizedBase<Subclass, false> {
+class SynchronizedBase<Subclass, detail::MutexLevel::UNIQUE> {
  public:
   using LockedPtr = ::folly::LockedPtr<Subclass, LockPolicyExclusive>;
   using ConstLockedPtr =
@@ -310,10 +424,10 @@ class SynchronizedBase<Subclass, false> {
 template <class T, class Mutex = SharedMutex>
 struct Synchronized : public SynchronizedBase<
                           Synchronized<T, Mutex>,
-                          LockTraits<Mutex>::is_shared> {
+                          MutexLevelValue<Mutex>::value> {
  private:
   using Base =
-      SynchronizedBase<Synchronized<T, Mutex>, LockTraits<Mutex>::is_shared>;
+      SynchronizedBase<Synchronized<T, Mutex>, MutexLevelValue<Mutex>::value>;
   static constexpr bool nxCopyCtor{
       std::is_nothrow_copy_constructible<T>::value};
   static constexpr bool nxMoveCtor{
@@ -991,6 +1105,74 @@ class LockedPtr : public LockedPtrBase<
   ScopedUnlocker<SynchronizedType, LockPolicy> scopedUnlock() {
     return ScopedUnlocker<SynchronizedType, LockPolicy>(this);
   }
+
+  /***************************************************************************
+   * Upgradable lock methods.
+   * These are disabled via SFINAE when the mutex is not upgradable
+   **************************************************************************/
+  /**
+   * Move the locked ptr from an upgrade state to an exclusive state.  The
+   * current lock is left in a null state.
+   */
+  template <
+      typename SyncType = SynchronizedType,
+      typename = typename std::enable_if<
+          LockTraits<typename SyncType::MutexType>::is_upgrade>::type>
+  LockedPtr<SynchronizedType, LockPolicyFromUpgradeToExclusive>
+  moveFromUpgradeToWrite() {
+    auto* parent_to_pass_on = this->parent_;
+    this->parent_ = nullptr;
+    return LockedPtr<SynchronizedType, LockPolicyFromUpgradeToExclusive>(
+        parent_to_pass_on);
+  }
+
+  /**
+   * Move the locked ptr from an exclusive state to an upgrade state.  The
+   * current lock is left in a null state.
+   */
+  template <
+      typename SyncType = SynchronizedType,
+      typename = typename std::enable_if<
+          LockTraits<typename SyncType::MutexType>::is_upgrade>::type>
+  LockedPtr<SynchronizedType, LockPolicyFromExclusiveToUpgrade>
+  moveFromWriteToUpgrade() {
+    auto* parent_to_pass_on = this->parent_;
+    this->parent_ = nullptr;
+    return LockedPtr<SynchronizedType, LockPolicyFromExclusiveToUpgrade>(
+        parent_to_pass_on);
+  }
+
+  /**
+   * Move the locked ptr from an upgrade state to a shared state.  The
+   * current lock is left in a null state.
+   */
+  template <
+      typename SyncType = SynchronizedType,
+      typename = typename std::enable_if<
+          LockTraits<typename SyncType::MutexType>::is_upgrade>::type>
+  LockedPtr<SynchronizedType, LockPolicyFromUpgradeToShared>
+  moveFromUpgradeToShared() {
+    auto* parent_to_pass_on = this->parent_;
+    this->parent_ = nullptr;
+    return LockedPtr<SynchronizedType, LockPolicyFromUpgradeToShared>(
+        parent_to_pass_on);
+  }
+
+  /**
+   * Move the locked ptr from an exclusive state to a shared state.  The
+   * current lock is left in a null state.
+   */
+  template <
+      typename SyncType = SynchronizedType,
+      typename = typename std::enable_if<
+          LockTraits<typename SyncType::MutexType>::is_upgrade>::type>
+  LockedPtr<SynchronizedType, LockPolicyFromExclusiveToShared>
+  moveFromWriteToShared() {
+    auto* parent_to_pass_on = this->parent_;
+    this->parent_ = nullptr;
+    return LockedPtr<SynchronizedType, LockPolicyFromExclusiveToShared>(
+        parent_to_pass_on);
+  }
 };
 
 /**
index 5826ef2..b734964 100644 (file)
@@ -189,6 +189,152 @@ class SynchronizedLockTest : public testing::Test {
   }
 };
 
+/**
+ * To avoid typing
+ */
+// static constexpr auto one_ms = std::chrono::milliseconds(1);
+
+/**
+ * Test mutex to help to automate assertions, taken from LockTraitsTest.cpp
+ */
+class FakeAllPowerfulAssertingMutexInternal {
+ public:
+  enum class CurrentLockState { UNLOCKED, SHARED, UPGRADE, UNIQUE };
+
+  void lock() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNLOCKED);
+    this->lock_state = CurrentLockState::UNIQUE;
+  }
+  void unlock() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNIQUE);
+    this->lock_state = CurrentLockState::UNLOCKED;
+  }
+  void lock_shared() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNLOCKED);
+    this->lock_state = CurrentLockState::SHARED;
+  }
+  void unlock_shared() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::SHARED);
+    this->lock_state = CurrentLockState::UNLOCKED;
+  }
+  void lock_upgrade() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNLOCKED);
+    this->lock_state = CurrentLockState::UPGRADE;
+  }
+  void unlock_upgrade() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UPGRADE);
+    this->lock_state = CurrentLockState::UNLOCKED;
+  }
+
+  void unlock_upgrade_and_lock() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UPGRADE);
+    this->lock_state = CurrentLockState::UNIQUE;
+  }
+  void unlock_and_lock_upgrade() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNIQUE);
+    this->lock_state = CurrentLockState::UPGRADE;
+  }
+  void unlock_and_lock_shared() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNIQUE);
+    this->lock_state = CurrentLockState::SHARED;
+  }
+  void unlock_upgrade_and_lock_shared() {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UPGRADE);
+    this->lock_state = CurrentLockState::SHARED;
+  }
+
+  template <class Rep, class Period>
+  bool try_lock_for(const std::chrono::duration<Rep, Period>&) {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNLOCKED);
+    this->lock_state = CurrentLockState::UNIQUE;
+    return true;
+  }
+
+  template <class Rep, class Period>
+  bool try_lock_upgrade_for(const std::chrono::duration<Rep, Period>&) {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UNLOCKED);
+    this->lock_state = CurrentLockState::UPGRADE;
+    return true;
+  }
+
+  template <class Rep, class Period>
+  bool try_unlock_upgrade_and_lock_for(
+      const std::chrono::duration<Rep, Period>&) {
+    EXPECT_EQ(this->lock_state, CurrentLockState::UPGRADE);
+    this->lock_state = CurrentLockState::UNIQUE;
+    return true;
+  }
+
+  /*
+   * Initialize the FakeMutex with an unlocked state
+   */
+  CurrentLockState lock_state{CurrentLockState::UNLOCKED};
+};
+
+/**
+ * The following works around the internal mutex for synchronized being
+ * private
+ *
+ * This is horridly thread unsafe.
+ */
+static FakeAllPowerfulAssertingMutexInternal globalAllPowerfulAssertingMutex;
+
+class FakeAllPowerfulAssertingMutex {
+ public:
+  void lock() {
+    globalAllPowerfulAssertingMutex.lock();
+  }
+  void unlock() {
+    globalAllPowerfulAssertingMutex.unlock();
+  }
+  void lock_shared() {
+    globalAllPowerfulAssertingMutex.lock_shared();
+  }
+  void unlock_shared() {
+    globalAllPowerfulAssertingMutex.unlock_shared();
+  }
+  void lock_upgrade() {
+    globalAllPowerfulAssertingMutex.lock_upgrade();
+  }
+  void unlock_upgrade() {
+    globalAllPowerfulAssertingMutex.unlock_upgrade();
+  }
+
+  void unlock_upgrade_and_lock() {
+    globalAllPowerfulAssertingMutex.unlock_upgrade_and_lock();
+  }
+  void unlock_and_lock_upgrade() {
+    globalAllPowerfulAssertingMutex.unlock_and_lock_upgrade();
+  }
+  void unlock_and_lock_shared() {
+    globalAllPowerfulAssertingMutex.unlock_and_lock_shared();
+  }
+  void unlock_upgrade_and_lock_shared() {
+    globalAllPowerfulAssertingMutex.unlock_upgrade_and_lock_shared();
+  }
+
+  template <class Rep, class Period>
+  bool try_lock_for(const std::chrono::duration<Rep, Period>& arg) {
+    return globalAllPowerfulAssertingMutex.try_lock_for(arg);
+  }
+
+  template <class Rep, class Period>
+  bool try_lock_upgrade_for(const std::chrono::duration<Rep, Period>& arg) {
+    return globalAllPowerfulAssertingMutex.try_lock_upgrade_for(arg);
+  }
+
+  template <class Rep, class Period>
+  bool try_unlock_upgrade_and_lock_for(
+      const std::chrono::duration<Rep, Period>& arg) {
+    return globalAllPowerfulAssertingMutex.try_unlock_upgrade_and_lock_for(arg);
+  }
+
+  // reset state on destruction
+  ~FakeAllPowerfulAssertingMutex() {
+    globalAllPowerfulAssertingMutex = FakeAllPowerfulAssertingMutexInternal{};
+  }
+};
+
 // Single level of SYNCHRONIZED and UNSYNCHRONIZED, although nested test are
 // super set of it, it is possible single level test passes while nested tests
 // fail
@@ -253,3 +399,169 @@ TEST_F(SynchronizedLockTest, NestedSyncUnSync2) {
   }
   EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount());
 }
+
+TEST_F(SynchronizedLockTest, UpgradableLocking) {
+  folly::Synchronized<int, FakeAllPowerfulAssertingMutex> sync;
+
+  // sanity assert
+  static_assert(
+      std::is_same<std::decay<decltype(*sync.ulock())>::type, int>::value,
+      "The ulock function was not well configured, blame aary@instagram.com");
+
+  {
+    auto ulock = sync.ulock();
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UPGRADE);
+  }
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test going from upgrade to exclusive
+  {
+    auto ulock = sync.ulock();
+    auto wlock = ulock.moveFromUpgradeToWrite();
+    EXPECT_EQ(static_cast<bool>(ulock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNIQUE);
+  }
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test going from upgrade to shared
+  {
+    auto ulock = sync.ulock();
+    auto slock = ulock.moveFromUpgradeToShared();
+    EXPECT_EQ(static_cast<bool>(ulock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::SHARED);
+  }
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test going from exclusive to upgrade
+  {
+    auto wlock = sync.wlock();
+    auto ulock = wlock.moveFromWriteToUpgrade();
+    EXPECT_EQ(static_cast<bool>(wlock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UPGRADE);
+  }
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test going from exclusive to shared
+  {
+    auto wlock = sync.wlock();
+    auto slock = wlock.moveFromWriteToShared();
+    EXPECT_EQ(static_cast<bool>(wlock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::SHARED);
+  }
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+}
+
+TEST_F(SynchronizedLockTest, UpgradableLockingWithULock) {
+  folly::Synchronized<int, FakeAllPowerfulAssertingMutex> sync;
+
+  // sanity assert
+  static_assert(
+      std::is_same<std::decay<decltype(*sync.ulock())>::type, int>::value,
+      "The ulock function was not well configured, blame aary@instagram.com");
+
+  // test from upgrade to write
+  sync.withULockPtr([](auto ulock) {
+    EXPECT_EQ(static_cast<bool>(ulock), true);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UPGRADE);
+
+    auto wlock = ulock.moveFromUpgradeToWrite();
+    EXPECT_EQ(static_cast<bool>(ulock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNIQUE);
+  });
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test from write to upgrade
+  sync.withWLockPtr([](auto wlock) {
+    EXPECT_EQ(static_cast<bool>(wlock), true);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNIQUE);
+
+    auto ulock = wlock.moveFromWriteToUpgrade();
+    EXPECT_EQ(static_cast<bool>(wlock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UPGRADE);
+  });
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test from upgrade to shared
+  sync.withULockPtr([](auto ulock) {
+    EXPECT_EQ(static_cast<bool>(ulock), true);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UPGRADE);
+
+    auto slock = ulock.moveFromUpgradeToShared();
+    EXPECT_EQ(static_cast<bool>(ulock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::SHARED);
+  });
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+
+  // test from write to shared
+  sync.withWLockPtr([](auto wlock) {
+    EXPECT_EQ(static_cast<bool>(wlock), true);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNIQUE);
+
+    auto slock = wlock.moveFromWriteToShared();
+    EXPECT_EQ(static_cast<bool>(wlock), false);
+    EXPECT_EQ(
+        globalAllPowerfulAssertingMutex.lock_state,
+        FakeAllPowerfulAssertingMutexInternal::CurrentLockState::SHARED);
+  });
+
+  // should be unlocked here
+  EXPECT_EQ(
+      globalAllPowerfulAssertingMutex.lock_state,
+      FakeAllPowerfulAssertingMutexInternal::CurrentLockState::UNLOCKED);
+}