Commit 50f4f92c authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot 5

improve Synchronized LockedPtr class, and add new lock() APIs

Summary:
This refactors the Synchronized::LockedPtr class, and adds new
lock()/wlock()/rlock() APIs to Synchronized.

The LockedPtr changes include:

- Consolidate code so a single template class can be used for both const
  and non-const operation, rather than requiring separate class definitions.
  A LockPolicy template parameter controls if the lock should be acquired in
  exclusive or shared mode, and a SynchronizedType parameter controls whether
  or not the internal data is const or not.

- Specialize LockedPtr for std::mutex, so it uses std::unique_lock<std::mutex>
  internally.  This requires slightly more storage space internally, but it
  allows Synchronized<T, std::mutex> to be used with std::condition_variable.

- Implement operator*() to dereference the pointer and retrieve a reference to
  the locked data.

- Implement operator!() and provide an isValid() method to check the validity
  of the LockedPtr.  These are needed to tell if a timed acquire operation
  succeeded or timed out.

- Drop the LockedPtr copy constructor.  Previously the copy constructor
  acquired the lock a second time.  If anyone needs the ability to hold a
  shared lock a second time solely via a LockedPtr (and not via the original
  Synchronized object), I think we should add back a much more explicit API to
  do this.

Furthermore, this adds lock(), wlock(), and rlock() methods to Synchronized to
explicitly obtain a LockedPtr.  These APIs behave similar to operator->(), but
are more explicit, and require the caller to make a concious choice about
whether or not an exclusive or shared lock should be acquired.  The lock()
method is present only on Synchronized instantiations using an exclusive mutex,
and the wlock() and rlock() methods are present only on Synchronized
instantiations that use a shared mutex.

I plan to deprecate the existing Synchronized::operator->() method and the
various SYNCHRONIZED macros in upcoming diffs.  For now this adds comments
directing users to the new methods, but does not start any of the technical
deprecation changes yet.

Reviewed By: yfeldblum

Differential Revision: D3526489

fbshipit-source-id: 8a96a09b68656ff9215dcdfdf32ecd2bfbb1727f
parent 594f18f7
......@@ -249,4 +249,77 @@ typename std::enable_if<!LockTraits<Mutex>::is_shared>::type
unlock_shared_or_unique(Mutex& mutex) {
LockTraits<Mutex>::unlock(mutex);
}
/*
* Lock policy classes.
*
* These can be used as template parameters to provide compile-time
* selection over the type of lock operation to perform.
*/
/**
* A lock policy that performs exclusive lock operations.
*/
class LockPolicyExclusive {
public:
template <class Mutex>
static void lock(Mutex& mutex) {
LockTraits<Mutex>::lock(mutex);
}
template <class Mutex, class Rep, class Period>
static bool try_lock_for(
Mutex& mutex,
const std::chrono::duration<Rep, Period>& timeout) {
return LockTraits<Mutex>::try_lock_for(mutex, timeout);
}
template <class Mutex>
static void unlock(Mutex& mutex) {
LockTraits<Mutex>::unlock(mutex);
}
};
/**
* A lock policy that performs shared lock operations.
* This policy only works with shared mutex types.
*/
class LockPolicyShared {
public:
template <class Mutex>
static void lock(Mutex& mutex) {
LockTraits<Mutex>::lock_shared(mutex);
}
template <class Mutex, class Rep, class Period>
static bool try_lock_for(
Mutex& mutex,
const std::chrono::duration<Rep, Period>& timeout) {
return LockTraits<Mutex>::try_lock_shared_for(mutex, timeout);
}
template <class Mutex>
static void unlock(Mutex& mutex) {
LockTraits<Mutex>::unlock_shared(mutex);
}
};
/**
* A lock policy that performs a shared lock operation if a shared mutex type
* is given, or a normal exclusive lock operation on non-shared mutex types.
*/
class LockPolicyShareable {
public:
template <class Mutex>
static void lock(Mutex& mutex) {
lock_shared_or_unique(mutex);
}
template <class Mutex, class Rep, class Period>
static bool try_lock_for(
Mutex& mutex,
const std::chrono::duration<Rep, Period>& timeout) {
return try_lock_shared_or_unique_for(mutex, timeout);
}
template <class Mutex>
static void unlock(Mutex& mutex) {
unlock_shared_or_unique(mutex);
}
};
} // folly
This diff is collapsed.
......@@ -59,10 +59,22 @@ TYPED_TEST(SynchronizedTest, Basic) {
testBasic<TypeParam>();
}
TYPED_TEST(SynchronizedTest, Deprecated) {
testDeprecated<TypeParam>();
}
TYPED_TEST(SynchronizedTest, Concurrency) {
testConcurrency<TypeParam>();
}
TYPED_TEST(SynchronizedTest, AcquireLocked) {
testAcquireLocked<TypeParam>();
}
TYPED_TEST(SynchronizedTest, AcquireLockedWithConst) {
testAcquireLockedWithConst<TypeParam>();
}
TYPED_TEST(SynchronizedTest, DualLocking) {
testDualLocking<TypeParam>();
}
......@@ -94,6 +106,10 @@ using SynchronizedTimedTestTypes = testing::Types<
folly::SharedMutexWritePriority>;
TYPED_TEST_CASE(SynchronizedTimedTest, SynchronizedTimedTestTypes);
TYPED_TEST(SynchronizedTimedTest, Timed) {
testTimed<TypeParam>();
}
TYPED_TEST(SynchronizedTimedTest, TimedSynchronized) {
testTimedSynchronized<TypeParam>();
}
......@@ -114,6 +130,10 @@ using SynchronizedTimedWithConstTestTypes = testing::Types<
TYPED_TEST_CASE(
SynchronizedTimedWithConstTest, SynchronizedTimedWithConstTestTypes);
TYPED_TEST(SynchronizedTimedWithConstTest, TimedShared) {
testTimedShared<TypeParam>();
}
TYPED_TEST(SynchronizedTimedWithConstTest, TimedSynchronizeWithConst) {
testTimedSynchronizedWithConst<TypeParam>();
}
......@@ -179,7 +199,7 @@ TEST_F(SynchronizedLockTest, SyncUnSync) {
EXPECT_EQ((CountPair{2, 2}), FakeMutex::getLockUnlockCount());
}
// Nested SYNCHRONIZED UNSYNCHRONIZED test, 2 levels for each are used here
// Nested SYNCHRONIZED UNSYNCHRONIZED test, 2 levels of synchronization
TEST_F(SynchronizedLockTest, NestedSyncUnSync) {
folly::Synchronized<std::vector<int>, FakeMutex> obj;
EXPECT_EQ((CountPair{0, 0}), FakeMutex::getLockUnlockCount());
......@@ -187,12 +207,15 @@ TEST_F(SynchronizedLockTest, NestedSyncUnSync) {
EXPECT_EQ((CountPair{1, 0}), FakeMutex::getLockUnlockCount());
SYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{2, 0}), FakeMutex::getLockUnlockCount());
// Note: UNSYNCHRONIZED has always been kind of broken here.
// The input parameter is ignored (other than to overwrite what the input
// variable name refers to), and it unlocks the most object acquired in
// the most recent SYNCHRONIZED scope.
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{2, 1}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{2, 2}),
FakeMutex::getLockUnlockCount());
}
}
EXPECT_EQ((CountPair{3, 1}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{3, 2}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 2}), FakeMutex::getLockUnlockCount());
......@@ -202,7 +225,7 @@ TEST_F(SynchronizedLockTest, NestedSyncUnSync) {
EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount());
}
// Different nesting behavior, UNSYNCHRONIZED called on differen depth of
// Different nesting behavior, UNSYNCHRONIZED called on different depth of
// SYNCHRONIZED
TEST_F(SynchronizedLockTest, NestedSyncUnSync2) {
folly::Synchronized<std::vector<int>, FakeMutex> obj;
......
This diff is collapsed.
......@@ -30,14 +30,34 @@
namespace folly {
namespace sync_tests {
template <class Mutex> void testBasic();
template <class Mutex> void testConcurrency();
template <class Mutex> void testDualLocking();
template <class Mutex> void testDualLockingWithConst();
template <class Mutex> void testTimedSynchronized();
template <class Mutex> void testTimedSynchronizedWithConst();
template <class Mutex> void testConstCopy();
template <class Mutex> void testInPlaceConstruction();
template <class Mutex>
void testBasic();
template <class Mutex>
void testDeprecated();
template <class Mutex>
void testConcurrency();
template <class Mutex>
void testAcquireLocked();
template <class Mutex>
void testAcquireLockedWithConst();
template <class Mutex>
void testDualLockingWithConst();
template <class Mutex>
void testDualLocking();
template <class Mutex>
void testDualLockingWithConst();
template <class Mutex>
void testTimed();
template <class Mutex>
void testTimedShared();
template <class Mutex>
void testTimedSynchronizedDeprecated();
template <class Mutex>
void testTimedSynchronizedWithConst();
template <class Mutex>
void testConstCopy();
template <class Mutex>
void testInPlaceConstruction();
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment