Commit 1c3b25b2 authored by Philip Pronin's avatar Philip Pronin Committed by Facebook Github Bot

nuke UNSYNCHRONIZED

Summary:
API of `UNSYNCHRONIZED` is confusing as if you have two nested
`SYNCHRONIZED` blocks, `UNSYNCHRONIZED` always unlocks the inner-most,
even if you pass in the variable name used in the outer `SYNCHRONIZED`
block.

The macro was marked as "deprecated" in D3526489, remove it here.

Reviewed By: yfeldblum

Differential Revision: D4371297

fbshipit-source-id: 13ddc1ff77cb3d5045844c5ade0e95dbe2bccf6d
parent f4dd104c
......@@ -1361,25 +1361,6 @@ void swap(Synchronized<T, M>& lhs, Synchronized<T, M>& rhs) {
FB_VA_GLUE(FB_ARG_1, (__VA_ARGS__)), \
(FB_VA_GLUE(FB_ARG_2_OR_1, (__VA_ARGS__))).asConst())
/**
* Temporarily disables synchronization inside a SYNCHRONIZED block.
*
* Note: This macro is deprecated, and kind of broken. The input parameter
* does not control what it unlocks--it always unlocks the lock acquired by the
* most recent SYNCHRONIZED scope. If you have two nested SYNCHRONIZED blocks,
* UNSYNCHRONIZED always unlocks the inner-most, even if you pass in the
* variable name used in the outer SYNCHRONIZED block.
*
* This macro will be removed soon in a subsequent diff.
*/
#define UNSYNCHRONIZED(name) \
for (auto SYNCHRONIZED_state3 = SYNCHRONIZED_lockedPtr.scopedUnlock(); \
!SYNCHRONIZED_state; \
SYNCHRONIZED_state = true) \
for (auto& name = *SYNCHRONIZED_state3.getSynchronized(); \
!SYNCHRONIZED_state; \
SYNCHRONIZED_state = true)
/**
* Synchronizes two Synchronized objects (they may encapsulate
* different data). Synchronization is done in increasing address of
......
......@@ -330,71 +330,6 @@ class FakeAllPowerfulAssertingMutex {
}
};
// 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
TEST_F(SynchronizedLockTest, SyncUnSync) {
folly::Synchronized<std::vector<int>, FakeMutex> obj;
EXPECT_EQ((CountPair{0, 0}), FakeMutex::getLockUnlockCount());
SYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{1, 0}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{1, 1}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{2, 1}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{2, 2}), FakeMutex::getLockUnlockCount());
}
// 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());
SYNCHRONIZED(objCopy, obj) {
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());
}
EXPECT_EQ((CountPair{3, 1}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{3, 2}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 2}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 3}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount());
}
// Different nesting behavior, UNSYNCHRONIZED called on different depth of
// SYNCHRONIZED
TEST_F(SynchronizedLockTest, NestedSyncUnSync2) {
folly::Synchronized<std::vector<int>, FakeMutex> obj;
EXPECT_EQ((CountPair{0, 0}), FakeMutex::getLockUnlockCount());
SYNCHRONIZED(objCopy, obj) {
EXPECT_EQ((CountPair{1, 0}), FakeMutex::getLockUnlockCount());
SYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{2, 0}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{2, 1}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{3, 1}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{3, 2}), FakeMutex::getLockUnlockCount());
UNSYNCHRONIZED(obj) {
EXPECT_EQ((CountPair{3, 3}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 3}), FakeMutex::getLockUnlockCount());
}
EXPECT_EQ((CountPair{4, 4}), FakeMutex::getLockUnlockCount());
}
TEST_F(SynchronizedLockTest, TestCopyConstructibleValues) {
struct NonCopyConstructible {
NonCopyConstructible(const NonCopyConstructible&) = delete;
......
......@@ -467,17 +467,10 @@ void testDeprecated() {
EXPECT_EQ(1001, obj.size());
EXPECT_EQ(10, obj.back());
EXPECT_EQ(1000, obj2->size());
UNSYNCHRONIZED(o) {
EXPECT_EQ(1001, o->size());
}
}
SYNCHRONIZED_CONST (obj) {
EXPECT_EQ(1001, obj.size());
UNSYNCHRONIZED(o) {
EXPECT_EQ(1001, o->size());
}
}
SYNCHRONIZED (lockedObj, *&obj) {
......
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