Commit 4e509c4b authored by Christopher Dykes's avatar Christopher Dykes Committed by Facebook Github Bot

Switch TimedRWMutex implementation to use lock_guard where possible

Summary: As titled. There are some places where scoping prevents trivial conversion, so I've left those places with explicit lock/unlock calls.

Reviewed By: andriigrynenko, yfeldblum

Differential Revision: D4888812

fbshipit-source-id: c124306e0373ee9eb395cb98f88df63c91523d48
parent 04de301e
...@@ -15,6 +15,8 @@ ...@@ -15,6 +15,8 @@
*/ */
#pragma once #pragma once
#include <mutex>
namespace folly { namespace folly {
namespace fibers { namespace fibers {
...@@ -151,11 +153,11 @@ inline void TimedMutex::unlock() { ...@@ -151,11 +153,11 @@ inline void TimedMutex::unlock() {
template <typename BatonType> template <typename BatonType>
void TimedRWMutex<BatonType>::read_lock() { void TimedRWMutex<BatonType>::read_lock() {
lock_.lock(); std::unique_lock<folly::SpinLock> lock{lock_};
if (state_ == State::WRITE_LOCKED) { if (state_ == State::WRITE_LOCKED) {
MutexWaiter waiter; MutexWaiter waiter;
read_waiters_.push_back(waiter); read_waiters_.push_back(waiter);
lock_.unlock(); lock.unlock();
waiter.baton.wait(); waiter.baton.wait();
assert(state_ == State::READ_LOCKED); assert(state_ == State::READ_LOCKED);
return; return;
...@@ -166,18 +168,17 @@ void TimedRWMutex<BatonType>::read_lock() { ...@@ -166,18 +168,17 @@ void TimedRWMutex<BatonType>::read_lock() {
assert(read_waiters_.empty()); assert(read_waiters_.empty());
state_ = State::READ_LOCKED; state_ = State::READ_LOCKED;
readers_ += 1; readers_ += 1;
lock_.unlock();
} }
template <typename BatonType> template <typename BatonType>
template <typename Rep, typename Period> template <typename Rep, typename Period>
bool TimedRWMutex<BatonType>::timed_read_lock( bool TimedRWMutex<BatonType>::timed_read_lock(
const std::chrono::duration<Rep, Period>& duration) { const std::chrono::duration<Rep, Period>& duration) {
lock_.lock(); std::unique_lock<folly::SpinLock> lock{lock_};
if (state_ == State::WRITE_LOCKED) { if (state_ == State::WRITE_LOCKED) {
MutexWaiter waiter; MutexWaiter waiter;
read_waiters_.push_back(waiter); read_waiters_.push_back(waiter);
lock_.unlock(); lock.unlock();
if (!waiter.baton.timed_wait(duration)) { if (!waiter.baton.timed_wait(duration)) {
// We timed out. Two cases: // We timed out. Two cases:
...@@ -185,13 +186,11 @@ bool TimedRWMutex<BatonType>::timed_read_lock( ...@@ -185,13 +186,11 @@ bool TimedRWMutex<BatonType>::timed_read_lock(
// 2. We're not in the waiter list anymore. This could happen if the baton // 2. We're not in the waiter list anymore. This could happen if the baton
// times out but the mutex is unlocked before we reach this code. In // times out but the mutex is unlocked before we reach this code. In
// this case we'll pretend we got the lock on time. // this case we'll pretend we got the lock on time.
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
if (waiter.hook.is_linked()) { if (waiter.hook.is_linked()) {
read_waiters_.erase(read_waiters_.iterator_to(waiter)); read_waiters_.erase(read_waiters_.iterator_to(waiter));
lock_.unlock();
return false; return false;
} }
lock_.unlock();
} }
return true; return true;
} }
...@@ -201,13 +200,12 @@ bool TimedRWMutex<BatonType>::timed_read_lock( ...@@ -201,13 +200,12 @@ bool TimedRWMutex<BatonType>::timed_read_lock(
assert(read_waiters_.empty()); assert(read_waiters_.empty());
state_ = State::READ_LOCKED; state_ = State::READ_LOCKED;
readers_ += 1; readers_ += 1;
lock_.unlock();
return true; return true;
} }
template <typename BatonType> template <typename BatonType>
bool TimedRWMutex<BatonType>::try_read_lock() { bool TimedRWMutex<BatonType>::try_read_lock() {
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
if (state_ != State::WRITE_LOCKED) { if (state_ != State::WRITE_LOCKED) {
assert( assert(
(state_ == State::UNLOCKED && readers_ == 0) || (state_ == State::UNLOCKED && readers_ == 0) ||
...@@ -215,25 +213,22 @@ bool TimedRWMutex<BatonType>::try_read_lock() { ...@@ -215,25 +213,22 @@ bool TimedRWMutex<BatonType>::try_read_lock() {
assert(read_waiters_.empty()); assert(read_waiters_.empty());
state_ = State::READ_LOCKED; state_ = State::READ_LOCKED;
readers_ += 1; readers_ += 1;
lock_.unlock();
return true; return true;
} }
lock_.unlock();
return false; return false;
} }
template <typename BatonType> template <typename BatonType>
void TimedRWMutex<BatonType>::write_lock() { void TimedRWMutex<BatonType>::write_lock() {
lock_.lock(); std::unique_lock<folly::SpinLock> lock{lock_};
if (state_ == State::UNLOCKED) { if (state_ == State::UNLOCKED) {
verify_unlocked_properties(); verify_unlocked_properties();
state_ = State::WRITE_LOCKED; state_ = State::WRITE_LOCKED;
lock_.unlock();
return; return;
} }
MutexWaiter waiter; MutexWaiter waiter;
write_waiters_.push_back(waiter); write_waiters_.push_back(waiter);
lock_.unlock(); lock.unlock();
waiter.baton.wait(); waiter.baton.wait();
} }
...@@ -241,16 +236,15 @@ template <typename BatonType> ...@@ -241,16 +236,15 @@ template <typename BatonType>
template <typename Rep, typename Period> template <typename Rep, typename Period>
bool TimedRWMutex<BatonType>::timed_write_lock( bool TimedRWMutex<BatonType>::timed_write_lock(
const std::chrono::duration<Rep, Period>& duration) { const std::chrono::duration<Rep, Period>& duration) {
lock_.lock(); std::unique_lock<folly::SpinLock> lock{lock_};
if (state_ == State::UNLOCKED) { if (state_ == State::UNLOCKED) {
verify_unlocked_properties(); verify_unlocked_properties();
state_ = State::WRITE_LOCKED; state_ = State::WRITE_LOCKED;
lock_.unlock();
return true; return true;
} }
MutexWaiter waiter; MutexWaiter waiter;
write_waiters_.push_back(waiter); write_waiters_.push_back(waiter);
lock_.unlock(); lock.unlock();
if (!waiter.baton.timed_wait(duration)) { if (!waiter.baton.timed_wait(duration)) {
// We timed out. Two cases: // We timed out. Two cases:
...@@ -258,13 +252,11 @@ bool TimedRWMutex<BatonType>::timed_write_lock( ...@@ -258,13 +252,11 @@ bool TimedRWMutex<BatonType>::timed_write_lock(
// 2. We're not in the waiter list anymore. This could happen if the baton // 2. We're not in the waiter list anymore. This could happen if the baton
// times out but the mutex is unlocked before we reach this code. In // times out but the mutex is unlocked before we reach this code. In
// this case we'll pretend we got the lock on time. // this case we'll pretend we got the lock on time.
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
if (waiter.hook.is_linked()) { if (waiter.hook.is_linked()) {
write_waiters_.erase(write_waiters_.iterator_to(waiter)); write_waiters_.erase(write_waiters_.iterator_to(waiter));
lock_.unlock();
return false; return false;
} }
lock_.unlock();
} }
assert(state_ == State::WRITE_LOCKED); assert(state_ == State::WRITE_LOCKED);
return true; return true;
...@@ -272,20 +264,18 @@ bool TimedRWMutex<BatonType>::timed_write_lock( ...@@ -272,20 +264,18 @@ bool TimedRWMutex<BatonType>::timed_write_lock(
template <typename BatonType> template <typename BatonType>
bool TimedRWMutex<BatonType>::try_write_lock() { bool TimedRWMutex<BatonType>::try_write_lock() {
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
if (state_ == State::UNLOCKED) { if (state_ == State::UNLOCKED) {
verify_unlocked_properties(); verify_unlocked_properties();
state_ = State::WRITE_LOCKED; state_ = State::WRITE_LOCKED;
lock_.unlock();
return true; return true;
} }
lock_.unlock();
return false; return false;
} }
template <typename BatonType> template <typename BatonType>
void TimedRWMutex<BatonType>::unlock() { void TimedRWMutex<BatonType>::unlock() {
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
assert(state_ != State::UNLOCKED); assert(state_ != State::UNLOCKED);
assert( assert(
(state_ == State::READ_LOCKED && readers_ > 0) || (state_ == State::READ_LOCKED && readers_ > 0) ||
...@@ -322,12 +312,11 @@ void TimedRWMutex<BatonType>::unlock() { ...@@ -322,12 +312,11 @@ void TimedRWMutex<BatonType>::unlock() {
} else { } else {
assert(state_ == State::READ_LOCKED); assert(state_ == State::READ_LOCKED);
} }
lock_.unlock();
} }
template <typename BatonType> template <typename BatonType>
void TimedRWMutex<BatonType>::downgrade() { void TimedRWMutex<BatonType>::downgrade() {
lock_.lock(); std::lock_guard<SpinLock> guard{lock_};
assert(state_ == State::WRITE_LOCKED && readers_ == 0); assert(state_ == State::WRITE_LOCKED && readers_ == 0);
state_ = State::READ_LOCKED; state_ = State::READ_LOCKED;
readers_ += 1; readers_ += 1;
...@@ -341,7 +330,6 @@ void TimedRWMutex<BatonType>::downgrade() { ...@@ -341,7 +330,6 @@ void TimedRWMutex<BatonType>::downgrade() {
to_wake.baton.post(); to_wake.baton.post();
} }
} }
lock_.unlock();
} }
} }
} }
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