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

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index 9b627237c909b4da61122a65b0ee8c8dd7c1af39..34dd6624cc2f1f3db85cac98d53bd003d4cddca5 100644 (file)
--- a/folly/Synchronized.h
+++ b/folly/Synchronized.h
@@ -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

diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp
index fb17f920c333b97d0d058d62c888fe7514eec691..5ff08d1434d2db1d49d39db52072528fd47f7699 100644 (file)
--- a/folly/test/SynchronizedTest.cpp
+++ b/folly/test/SynchronizedTest.cpp
@@ -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;

diff --git a/folly/test/SynchronizedTestLib-inl.h b/folly/test/SynchronizedTestLib-inl.h
index 843d82401fa268338b1862b182851ba0fb31fd4a..10514089bfc59c142ad7eb805444e0ab5b9848bb 100644 (file)
--- a/folly/test/SynchronizedTestLib-inl.h
+++ b/folly/test/SynchronizedTestLib-inl.h
@@ -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) {