Commit 38237272 authored by Adam Simpkins's avatar Adam Simpkins Committed by Facebook Github Bot 7

add an unlock() method to Synchronized<T>::LockedPtr

Summary:
Add an unlock() method to Synchronized LockedPtr objects.  This will make it
easier to replace current users of the UNSYNCHRONIZED macro.

Of the handful of places currently using UNSYNCHRONIZED, many of them want to
simply release the lock before logging a message and returning.  However,
UNSYNCHRONIZED is a poor choice for this, since it will re-acquire the lock on
scope exit.  In these situations where the function returns from inside an
UNSYNCHRONIZED block the code unnecessarily re-acquires the lock just to
immediately release it.  The unlock() method will provide a cleaner mechanism
for these call sites to simply drop the lock early before returning.

Reviewed By: yfeldblum

Differential Revision: D3547652

fbshipit-source-id: 4d28fe9f3aad0d7348e918d1a3d6c705bfec242b
parent 5d90c138
...@@ -659,6 +659,20 @@ class LockedPtrBase { ...@@ -659,6 +659,20 @@ class LockedPtrBase {
} }
} }
/**
* Unlock the synchronized data.
*
* The LockedPtr can no longer be dereferenced after unlock() has been
* called. isValid() will return false on an unlocked LockedPtr.
*
* unlock() can only be called on a LockedPtr that is valid.
*/
void unlock() {
DCHECK(parent_ != nullptr);
LockPolicy::unlock(parent_->mutex_);
parent_ = nullptr;
}
protected: protected:
LockedPtrBase() {} LockedPtrBase() {}
explicit LockedPtrBase(SynchronizedType* parent) : parent_(parent) { explicit LockedPtrBase(SynchronizedType* parent) : parent_(parent) {
...@@ -699,6 +713,7 @@ class LockedPtrBase { ...@@ -699,6 +713,7 @@ class LockedPtrBase {
} }
UnlockerData releaseLock() { UnlockerData releaseLock() {
DCHECK(parent_ != nullptr);
auto current = parent_; auto current = parent_;
parent_ = nullptr; parent_ = nullptr;
LockPolicy::unlock(current->mutex_); LockPolicy::unlock(current->mutex_);
...@@ -759,6 +774,20 @@ class LockedPtrBase<SynchronizedType, std::mutex, LockPolicy> { ...@@ -759,6 +774,20 @@ class LockedPtrBase<SynchronizedType, std::mutex, LockPolicy> {
return lock_; return lock_;
} }
/**
* Unlock the synchronized data.
*
* The LockedPtr can no longer be dereferenced after unlock() has been
* called. isValid() will return false on an unlocked LockedPtr.
*
* unlock() can only be called on a LockedPtr that is valid.
*/
void unlock() {
DCHECK(parent_ != nullptr);
lock_.unlock();
parent_ = nullptr;
}
protected: protected:
LockedPtrBase() {} LockedPtrBase() {}
explicit LockedPtrBase(SynchronizedType* parent) explicit LockedPtrBase(SynchronizedType* parent)
...@@ -772,6 +801,7 @@ class LockedPtrBase<SynchronizedType, std::mutex, LockPolicy> { ...@@ -772,6 +801,7 @@ class LockedPtrBase<SynchronizedType, std::mutex, LockPolicy> {
} }
UnlockerData releaseLock() { UnlockerData releaseLock() {
DCHECK(parent_ != nullptr);
UnlockerData data(std::move(lock_), parent_); UnlockerData data(std::move(lock_), parent_);
parent_ = nullptr; parent_ = nullptr;
data.first.unlock(); data.first.unlock();
......
...@@ -63,6 +63,10 @@ TYPED_TEST(SynchronizedTest, WithLock) { ...@@ -63,6 +63,10 @@ TYPED_TEST(SynchronizedTest, WithLock) {
testWithLock<TypeParam>(); testWithLock<TypeParam>();
} }
TYPED_TEST(SynchronizedTest, Unlock) {
testUnlock<TypeParam>();
}
TYPED_TEST(SynchronizedTest, Deprecated) { TYPED_TEST(SynchronizedTest, Deprecated) {
testDeprecated<TypeParam>(); testDeprecated<TypeParam>();
} }
......
...@@ -42,6 +42,7 @@ inline std::mt19937& getRNG() { ...@@ -42,6 +42,7 @@ inline std::mt19937& getRNG() {
void randomSleep(std::chrono::milliseconds min, std::chrono::milliseconds max) { void randomSleep(std::chrono::milliseconds min, std::chrono::milliseconds max) {
std::uniform_int_distribution<> range(min.count(), max.count()); std::uniform_int_distribution<> range(min.count(), max.count());
std::chrono::milliseconds duration(range(getRNG())); std::chrono::milliseconds duration(range(getRNG()));
/* sleep override */
std::this_thread::sleep_for(duration); std::this_thread::sleep_for(duration);
} }
...@@ -354,6 +355,104 @@ testWithLock() { ...@@ -354,6 +355,104 @@ testWithLock() {
}); });
} }
template <class Mutex>
void testUnlockCommon() {
folly::Synchronized<int, Mutex> value{7};
const auto& cv = value;
{
auto lv = value.contextualLock();
EXPECT_EQ(7, *lv);
*lv = 5;
lv.unlock();
EXPECT_TRUE(lv.isNull());
EXPECT_FALSE(lv);
auto rlv = cv.contextualLock();
EXPECT_EQ(5, *rlv);
rlv.unlock();
EXPECT_TRUE(rlv.isNull());
EXPECT_FALSE(rlv);
auto rlv2 = cv.contextualRLock();
EXPECT_EQ(5, *rlv2);
rlv2.unlock();
lv = value.contextualLock();
EXPECT_EQ(5, *lv);
*lv = 9;
}
EXPECT_EQ(9, *value.contextualRLock());
}
// testUnlock() version for shared lock types
template <class Mutex>
typename std::enable_if<folly::LockTraits<Mutex>::is_shared>::type
testUnlock() {
folly::Synchronized<int, Mutex> value{10};
{
auto lv = value.wlock();
EXPECT_EQ(10, *lv);
*lv = 5;
lv.unlock();
EXPECT_FALSE(lv);
EXPECT_TRUE(lv.isNull());
auto rlv = value.rlock();
EXPECT_EQ(5, *rlv);
rlv.unlock();
EXPECT_FALSE(rlv);
EXPECT_TRUE(rlv.isNull());
auto lv2 = value.wlock();
EXPECT_EQ(5, *lv2);
*lv2 = 7;
lv = std::move(lv2);
EXPECT_FALSE(lv2);
EXPECT_TRUE(lv2.isNull());
EXPECT_FALSE(lv.isNull());
EXPECT_EQ(7, *lv);
}
testUnlockCommon<Mutex>();
}
// testUnlock() version for non-shared lock types
template <class Mutex>
typename std::enable_if<!folly::LockTraits<Mutex>::is_shared>::type
testUnlock() {
folly::Synchronized<int, Mutex> value{10};
{
auto lv = value.lock();
EXPECT_EQ(10, *lv);
*lv = 5;
lv.unlock();
EXPECT_TRUE(lv.isNull());
EXPECT_FALSE(lv);
auto lv2 = value.lock();
EXPECT_EQ(5, *lv2);
*lv2 = 6;
lv2.unlock();
EXPECT_TRUE(lv2.isNull());
EXPECT_FALSE(lv2);
lv = value.lock();
EXPECT_EQ(6, *lv);
*lv = 7;
lv2 = std::move(lv);
EXPECT_TRUE(lv.isNull());
EXPECT_FALSE(lv);
EXPECT_FALSE(lv2.isNull());
EXPECT_EQ(7, *lv2);
}
testUnlockCommon<Mutex>();
}
// Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs // Testing the deprecated SYNCHRONIZED and SYNCHRONIZED_CONST APIs
template <class Mutex> template <class Mutex>
void testDeprecated() { void testDeprecated() {
......
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