From: Louis Kruger <louisk@fb.com>
Date: Tue, 1 Apr 2014 20:22:42 +0000 (-0700)
Subject: Fix TIMED_SYNCHRONIZED_CONST bug
X-Git-Tag: v0.22.0~625
X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=7de41cd34151873c87d86353e596fbae2ebc74a8;p=folly.git

Fix TIMED_SYNCHRONIZED_CONST bug

Summary:
TIMED_SYNCHRONIZED_CONST is acquiring a read-write lock but releasing a shared lock.  Chaos ensues.
@override-unit-failures

Test Plan: Add unit test, test times out before fix, passes after fix.

Reviewed By: andrei.alexandrescu@fb.com

FB internal diff: D1251518

Blame Revision: D327492
---

diff --git a/folly/Synchronized.h b/folly/Synchronized.h
index 538bdbab..83e59e80 100644
--- a/folly/Synchronized.h
+++ b/folly/Synchronized.h
@@ -427,7 +427,7 @@ struct Synchronized {
       acquire();
     }
     ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) {
-      if (parent->mutex_.timed_lock(
+      if (parent->mutex_.timed_lock_shared(
             boost::posix_time::milliseconds(milliseconds))) {
         parent_ = parent;
         return;
diff --git a/folly/test/SynchronizedTest.cpp b/folly/test/SynchronizedTest.cpp
index a741670d..dd23dd45 100644
--- a/folly/test/SynchronizedTest.cpp
+++ b/folly/test/SynchronizedTest.cpp
@@ -119,6 +119,8 @@ TEST(Synchronized, TimedSynchronized) {
   testTimedSynchronized<boost::timed_mutex>();
   testTimedSynchronized<boost::recursive_timed_mutex>();
   testTimedSynchronized<boost::shared_mutex>();
+
+  testTimedSynchronizedWithConst<boost::shared_mutex>();
 }
 #endif
 
diff --git a/folly/test/SynchronizedTestLib-inl.h b/folly/test/SynchronizedTestLib-inl.h
index 3daeba16..cbd43624 100644
--- a/folly/test/SynchronizedTestLib-inl.h
+++ b/folly/test/SynchronizedTestLib-inl.h
@@ -274,6 +274,56 @@ template <class Mutex> void testTimedSynchronized() {
   }
 }
 
+template <class Mutex> void testTimedSynchronizedWithConst() {
+  folly::Synchronized<std::vector<int>, Mutex> v;
+
+  struct Local {
+    static bool threadMain(int i,
+                           folly::Synchronized<std::vector<int>, Mutex>& pv) {
+      usleep(::random(100 * 1000, 1000 * 1000));
+
+      // Test operator->
+      pv->push_back(i);
+
+      usleep(::random(5 * 1000, 1000 * 1000));
+      // Test TIMED_SYNCHRONIZED_CONST
+      for (;;) {
+        TIMED_SYNCHRONIZED_CONST (10, v, pv) {
+          if (v) {
+            auto found = std::find(v->begin(), v->end(),  i);
+            CHECK(found != v->end());
+            return true;
+          } else {
+            // do nothing
+            usleep(::random(10 * 1000, 100 * 1000));
+          }
+        }
+      }
+    }
+  };
+
+  std::vector<std::thread> results;
+
+  static const size_t threads = 100;
+  FOR_EACH_RANGE (i, 0, threads) {
+    results.push_back(std::thread([&, i]() { Local::threadMain(i, v); }));
+  }
+
+  FOR_EACH (i, results) {
+    i->join();
+  }
+
+  std::vector<int> result;
+  v.swap(result);
+
+  EXPECT_EQ(result.size(), threads);
+  sort(result.begin(), result.end());
+
+  FOR_EACH_RANGE (i, 0, threads) {
+    EXPECT_EQ(result[i], i);
+  }
+}
+
 template <class Mutex> void testConstCopy() {
   std::vector<int> input = {1, 2, 3};
   const folly::Synchronized<std::vector<int>, Mutex> v(input);
diff --git a/folly/test/SynchronizedTestLib.h b/folly/test/SynchronizedTestLib.h
index 43e51443..c3602cb7 100644
--- a/folly/test/SynchronizedTestLib.h
+++ b/folly/test/SynchronizedTestLib.h
@@ -40,6 +40,8 @@ template <class Mutex> void testDualLockingWithConst();
 
 template <class Mutex> void testTimedSynchronized();
 
+template <class Mutex> void testTimedSynchronizedWithConst();
+
 template <class Mutex> void testConstCopy();
 
 #include "folly/test/SynchronizedTestLib-inl.h"