Commit 7de41cd3 authored by Louis Kruger's avatar Louis Kruger Committed by Sara Golemon

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
parent 30c26b8f
...@@ -427,7 +427,7 @@ struct Synchronized { ...@@ -427,7 +427,7 @@ struct Synchronized {
acquire(); acquire();
} }
ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) { ConstLockedPtr(const Synchronized* parent, unsigned int milliseconds) {
if (parent->mutex_.timed_lock( if (parent->mutex_.timed_lock_shared(
boost::posix_time::milliseconds(milliseconds))) { boost::posix_time::milliseconds(milliseconds))) {
parent_ = parent; parent_ = parent;
return; return;
......
...@@ -119,6 +119,8 @@ TEST(Synchronized, TimedSynchronized) { ...@@ -119,6 +119,8 @@ TEST(Synchronized, TimedSynchronized) {
testTimedSynchronized<boost::timed_mutex>(); testTimedSynchronized<boost::timed_mutex>();
testTimedSynchronized<boost::recursive_timed_mutex>(); testTimedSynchronized<boost::recursive_timed_mutex>();
testTimedSynchronized<boost::shared_mutex>(); testTimedSynchronized<boost::shared_mutex>();
testTimedSynchronizedWithConst<boost::shared_mutex>();
} }
#endif #endif
......
...@@ -274,6 +274,56 @@ template <class Mutex> void testTimedSynchronized() { ...@@ -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() { template <class Mutex> void testConstCopy() {
std::vector<int> input = {1, 2, 3}; std::vector<int> input = {1, 2, 3};
const folly::Synchronized<std::vector<int>, Mutex> v(input); const folly::Synchronized<std::vector<int>, Mutex> v(input);
......
...@@ -40,6 +40,8 @@ template <class Mutex> void testDualLockingWithConst(); ...@@ -40,6 +40,8 @@ template <class Mutex> void testDualLockingWithConst();
template <class Mutex> void testTimedSynchronized(); template <class Mutex> void testTimedSynchronized();
template <class Mutex> void testTimedSynchronizedWithConst();
template <class Mutex> void testConstCopy(); template <class Mutex> void testConstCopy();
#include "folly/test/SynchronizedTestLib-inl.h" #include "folly/test/SynchronizedTestLib-inl.h"
......
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