Skip to content

F
folly
Commit d7389fcf authored by Yedidya Feldblum's avatar Yedidya Feldblum Committed by Facebook GitHub Bot
Browse files
Options

ProxyLockable revisions 

Summary:
* Rename from `proxy()` to `state()` and from `proxy_type` to `state_type`.
* Require `state_type` to be semiregular and testable (convertible explicitly to `bool`). A default-constructed instance and an instance just after assignment from a default-constructed instance must test false.
* Add `std::adopt_lock` constructor which can accept a `state_type`.

Reviewed By: aary

Differential Revision: D28724949

fbshipit-source-id: 64fc29bd875dc24cc3fd62d1b4b86f7ebdacfb73
parent 6fd0d1b5
Hide whitespace changes
Inline Side-by-side
Showing with 96 additions and 107 deletions
folly/synchronization/DistributedMutex-inl.h
View file @ d7389fcf
......@@ -751,25 +751,11 @@ inline std::uint64_t recover(std::uint64_t from) {
template <template <typename> class Atomic, bool TimePublishing>
class DistributedMutex<Atomic, TimePublishing>::DistributedMutexStateProxy {
public:
// DistributedMutexStateProxy is move constructible and assignable for
// convenience
DistributedMutexStateProxy(DistributedMutexStateProxy&& other) {
*this = std::move(other);
}
DistributedMutexStateProxy& operator=(DistributedMutexStateProxy&& other) {
DCHECK(!(*this)) << "Cannot move into a valid DistributedMutexStateProxy";
DistributedMutexStateProxy() = default;
next_ = std::exchange(other.next_, nullptr);
expected_ = std::exchange(other.expected_, 0);
timedWaiters_ = std::exchange(other.timedWaiters_, false);
combined_ = std::exchange(other.combined_, false);
waker_ = std::exchange(other.waker_, 0);
waiters_ = std::exchange(other.waiters_, nullptr);
ready_ = std::exchange(other.ready_, nullptr);
return *this;
}
DistributedMutexStateProxy(DistributedMutexStateProxy const&) = default;
DistributedMutexStateProxy& operator=(DistributedMutexStateProxy const&) =
default;
// The proxy is valid when a mutex acquisition attempt was successful,
// lock() is guaranteed to return a valid proxy, try_lock() is not
......
folly/synchronization/detail/ProxyLockable-inl.h
View file @ d7389fcf
......@@ -61,17 +61,14 @@ ProxyLockableUniqueLock<Mutex>::~ProxyLockableUniqueLock() {
}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex) noexcept {
proxy_.emplace(mutex.lock());
mutex_ = std::addressof(mutex);
}
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(mutex_type& mutex)
: mutex_{std::addressof(mutex)}, state_{mutex.lock()} {}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
ProxyLockableUniqueLock&& a) noexcept {
*this = std::move(a);
}
ProxyLockableUniqueLock&& a) noexcept
: mutex_{std::exchange(a.mutex_, nullptr)},
state_{std::exchange(a.state_, state_type{})} {}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>& ProxyLockableUniqueLock<Mutex>::operator=(
......@@ -79,111 +76,92 @@ ProxyLockableUniqueLock<Mutex>& ProxyLockableUniqueLock<Mutex>::operator=(
if (owns_lock()) {
unlock();
}
proxy_ = std::move(other.proxy_);
mutex_ = std::exchange(other.mutex_, nullptr);
state_ = std::exchange(other.state_, state_type{});
return *this;
}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex, std::defer_lock_t) noexcept {
mutex_ = std::addressof(mutex);
mutex_type& mutex, std::adopt_lock_t, state_type state)
: mutex_{std::addressof(mutex)}, state_{state} {
proxylockable_detail::throwIfNotLocked(state_);
}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex, std::try_to_lock_t) {
mutex_ = std::addressof(mutex);
if (auto state = mutex.try_lock()) {
proxy_.emplace(std::move(state));
}
}
mutex_type& mutex, std::defer_lock_t) noexcept
: mutex_{std::addressof(mutex)} {}
template <typename Mutex>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex, std::try_to_lock_t)
: mutex_{std::addressof(mutex)}, state_{mutex.try_lock()} {}
template <typename Mutex>
template <typename Rep, typename Period>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex, const std::chrono::duration<Rep, Period>& duration) {
mutex_ = std::addressof(mutex);
if (auto state = mutex.try_lock_for(duration)) {
proxy_.emplace(std::move(state));
}
}
mutex_type& mutex, const std::chrono::duration<Rep, Period>& timeout)
: mutex_{std::addressof(mutex)}, state_{mutex.try_lock_for(timeout)} {}
template <typename Mutex>
template <typename Clock, typename Duration>
ProxyLockableUniqueLock<Mutex>::ProxyLockableUniqueLock(
mutex_type& mutex, const std::chrono::time_point<Clock, Duration>& time) {
mutex_ = std::addressof(mutex);
if (auto state = mutex.try_lock_until(time)) {
proxy_.emplace(std::move(state));
}
}
mutex_type& mutex, const std::chrono::time_point<Clock, Duration>& deadline)
: mutex_{std::addressof(mutex)}, state_{mutex.try_lock_until(deadline)} {}
template <typename Mutex>
void ProxyLockableUniqueLock<Mutex>::lock() {
proxylockable_detail::throwIfAlreadyLocked(proxy_);
proxylockable_detail::throwIfAlreadyLocked(state_);
proxylockable_detail::throwIfNoMutex(mutex_);
proxy_.emplace(mutex_->lock());
state_ = mutex_->lock();
}
template <typename Mutex>
void ProxyLockableUniqueLock<Mutex>::unlock() {
proxylockable_detail::throwIfNoMutex(mutex_);
proxylockable_detail::throwIfNotLocked(proxy_);
proxylockable_detail::throwIfNotLocked(state_);
mutex_->unlock(std::move(*proxy_));
proxy_.reset();
mutex_->unlock(std::exchange(state_, state_type{}));
}
template <typename Mutex>
bool ProxyLockableUniqueLock<Mutex>::try_lock() {
proxylockable_detail::throwIfNoMutex(mutex_);
proxylockable_detail::throwIfAlreadyLocked(proxy_);
if (auto state = mutex_->try_lock()) {
proxy_.emplace(std::move(state));
return true;
}
proxylockable_detail::throwIfAlreadyLocked(state_);
return false;
state_ = mutex_->try_lock();
return !!state_;
}
template <typename Mutex>
template <typename Rep, typename Period>
bool ProxyLockableUniqueLock<Mutex>::try_lock_for(
const std::chrono::duration<Rep, Period>& duration) {
const std::chrono::duration<Rep, Period>& timeout) {
proxylockable_detail::throwIfNoMutex(mutex_);
proxylockable_detail::throwIfAlreadyLocked(proxy_);
if (auto state = mutex_->try_lock_for(duration)) {
proxy_.emplace(std::move(state));
return true;
}
proxylockable_detail::throwIfAlreadyLocked(state_);
return false;
state_ = mutex_->try_lock_for(timeout);
return !!state_;
}
template <typename Mutex>
template <typename Clock, typename Duration>
bool ProxyLockableUniqueLock<Mutex>::try_lock_until(
const std::chrono::time_point<Clock, Duration>& time) {
const std::chrono::time_point<Clock, Duration>& deadline) {
proxylockable_detail::throwIfNoMutex(mutex_);
proxylockable_detail::throwIfAlreadyLocked(proxy_);
proxylockable_detail::throwIfAlreadyLocked(state_);
if (auto state = mutex_->try_lock_until(time)) {
proxy_.emplace(std::move(state));
return true;
}
return false;
state_ = mutex_->try_lock_until(deadline);
return !!state_;
}
template <typename Mutex>
void ProxyLockableUniqueLock<Mutex>::swap(
ProxyLockableUniqueLock& other) noexcept {
std::swap(mutex_, other.mutex_);
std::swap(proxy_, other.proxy_);
std::swap(state_, other.state_);
}
template <typename Mutex>
......@@ -193,14 +171,20 @@ ProxyLockableUniqueLock<Mutex>::mutex() const noexcept {
}
template <typename Mutex>
typename ProxyLockableUniqueLock<Mutex>::proxy_type*
ProxyLockableUniqueLock<Mutex>::proxy() const noexcept {
return proxy_ ? std::addressof(proxy_.value()) : nullptr;
typename ProxyLockableUniqueLock<Mutex>::state_type const&
ProxyLockableUniqueLock<Mutex>::state() const noexcept {
return state_;
}
template <typename Mutex>
typename ProxyLockableUniqueLock<Mutex>::state_type&
ProxyLockableUniqueLock<Mutex>::state() noexcept {
return state_;
}
template <typename Mutex>
bool ProxyLockableUniqueLock<Mutex>::owns_lock() const noexcept {
return proxy_.has_value();
return !!state_;
}
template <typename Mutex>
......
folly/synchronization/detail/ProxyLockable.h
View file @ d7389fcf
......@@ -25,7 +25,7 @@ namespace detail {
/**
* ProxyLockable is a "concept" that is used usually for mutexes that don't
* return void, but rather a proxy object that contains data that should be
* return void, but rather a state object that contains data that should be
* passed to the unlock function.
*
* This is in contrast with the normal Lockable concept that imposes no
......@@ -43,7 +43,7 @@ template <typename Mutex>
class ProxyLockableUniqueLock {
public:
using mutex_type = Mutex;
using proxy_type = std::decay_t<decltype(std::declval<mutex_type>().lock())>;
using state_type = std::decay_t<decltype(std::declval<mutex_type&>().lock())>;
/**
* Default constructor initializes the unique_lock to an empty state
......@@ -67,15 +67,18 @@ class ProxyLockableUniqueLock {
*
* The mutex is guaranteed to be acquired after this function returns.
*/
ProxyLockableUniqueLock(mutex_type&) noexcept;
explicit ProxyLockableUniqueLock(mutex_type&);
/**
* Explicit locking constructors to control how the lock() method is called
*
* std::adopt_lock_t causes the mutex to get tracked, with provided lock state
* std::defer_lock_t causes the mutex to get tracked, but not locked
* std::try_to_lock_t causes try_lock() to be called. The current object is
* converts to true if the lock was successful
*/
ProxyLockableUniqueLock(
mutex_type& mutex, std::adopt_lock_t, state_type state);
ProxyLockableUniqueLock(mutex_type& mutex, std::defer_lock_t) noexcept;
ProxyLockableUniqueLock(mutex_type& mutex, std::try_to_lock_t);
......@@ -106,9 +109,9 @@ class ProxyLockableUniqueLock {
* These throw if there was no mutex, or if the mutex was already locked
*/
template <typename Rep, typename Period>
bool try_lock_for(const std::chrono::duration<Rep, Period>& duration);
bool try_lock_for(const std::chrono::duration<Rep, Period>& timeout);
template <typename Clock, typename Duration>
bool try_lock_until(const std::chrono::time_point<Clock, Duration>& time);
bool try_lock_until(const std::chrono::time_point<Clock, Duration>& deadline);
/**
* Swap this unique lock with the other one
......@@ -124,13 +127,14 @@ class ProxyLockableUniqueLock {
/**
* mutex() return a pointer to the mutex if there is a contained mutex and
* proxy() returns a pointer to the contained proxy if the mutex is locked
* state() returns a pointer to the contained state if the mutex is locked
*
* If the unique lock was not constructed with a mutex, then mutex() returns
* nullptr. If the mutex is not locked, then proxy() returns nullptr
* nullptr. If the mutex is not locked, then state() returns nullptr
*/
mutex_type* mutex() const noexcept;
proxy_type* proxy() const noexcept;
state_type const& state() const noexcept;
state_type& state() noexcept;
private:
friend class ProxyLockableTest;
......@@ -139,8 +143,8 @@ class ProxyLockableUniqueLock {
* If the optional has a value, the mutex is locked, if it is empty, it is
* not
*/
mutable folly::Optional<proxy_type> proxy_{};
mutex_type* mutex_{nullptr};
state_type state_{};
};
template <typename Mutex>
......
folly/synchronization/detail/test/ProxyLockableTest.cpp
View file @ d7389fcf
......@@ -82,7 +82,7 @@ TEST_F(ProxyLockableTest, UniqueLockDefaultConstruct) {
auto lck = ProxyLockableUniqueLock<MockMutex>{};
EXPECT_FALSE(lck.mutex());
EXPECT_FALSE(lck.proxy());
EXPECT_FALSE(lck.state());
EXPECT_FALSE(lck.owns_lock());
EXPECT_FALSE(lck.operator bool());
}
......@@ -92,7 +92,7 @@ TEST_F(ProxyLockableTest, UniqueLockLockOnConstruct) {
auto lck = ProxyLockableUniqueLock<MockMutex>{mutex};
EXPECT_TRUE(lck.mutex());
EXPECT_TRUE(lck.proxy());
EXPECT_TRUE(lck.state());
EXPECT_EQ(mutex.locked_, 1);
}
......@@ -102,15 +102,15 @@ TEST_F(ProxyLockableTest, UniqueLockConstructMoveConstructAssign) {
auto one = ProxyLockableUniqueLock<MockMutex>{mutex};
EXPECT_TRUE(one.mutex());
EXPECT_TRUE(one.proxy());
EXPECT_TRUE(one.state());
auto two = std::move(one);
EXPECT_FALSE(one.mutex());
EXPECT_FALSE(one.proxy());
EXPECT_FALSE(one.state());
EXPECT_FALSE(one.owns_lock());
EXPECT_FALSE(one.operator bool());
EXPECT_TRUE(two.mutex());
EXPECT_TRUE(two.proxy());
EXPECT_TRUE(two.state());
auto three = std::move(one);
EXPECT_FALSE(one.mutex());
......@@ -120,21 +120,36 @@ TEST_F(ProxyLockableTest, UniqueLockConstructMoveConstructAssign) {
auto four = std::move(two);
EXPECT_TRUE(four.mutex());
EXPECT_TRUE(four.proxy());
EXPECT_FALSE(one.proxy());
EXPECT_FALSE(one.proxy());
EXPECT_TRUE(four.state());
EXPECT_FALSE(one.state());
EXPECT_FALSE(one.state());
EXPECT_EQ(mutex.locked_, 1);
four = std::move(three);
EXPECT_EQ(mutex.locked_, 0);
EXPECT_FALSE(four.mutex());
EXPECT_FALSE(four.proxy());
EXPECT_FALSE(four.state());
four = ProxyLockableUniqueLock<MockMutex>{mutex};
EXPECT_EQ(mutex.locked_, 1);
EXPECT_TRUE(four.mutex());
EXPECT_TRUE(four.proxy());
EXPECT_TRUE(four.state());
four = ProxyLockableUniqueLock<MockMutex>{};
EXPECT_EQ(mutex.locked_, 0);
EXPECT_FALSE(four.mutex());
EXPECT_FALSE(four.state());
}
TEST_F(ProxyLockableTest, UniqueLockAdoptLock) {
auto mutex = MockMutex{};
auto state = mutex.lock();
auto lck = ProxyLockableUniqueLock<MockMutex>{mutex, std::adopt_lock, state};
EXPECT_EQ(mutex.locked_, 1);
lck.unlock();
EXPECT_EQ(mutex.locked_, 0);
}
TEST_F(ProxyLockableTest, UniqueLockDeferLock) {
......@@ -155,7 +170,7 @@ void testTryToLock(Make make) {
auto lck = make(mutex);
EXPECT_TRUE(lck.mutex());
EXPECT_TRUE(lck.proxy());
EXPECT_TRUE(lck.state());
EXPECT_EQ(mutex.locked_, 1);
}
......@@ -165,7 +180,7 @@ void testTryToLock(Make make) {
auto lck = make(mutex);
EXPECT_EQ(mutex.locked_, 1);
EXPECT_TRUE(lck.mutex());
EXPECT_FALSE(lck.proxy());
EXPECT_FALSE(lck.state());
}
} // namespace
......@@ -195,12 +210,12 @@ TEST_F(ProxyLockableTest, UniqueLockLockExplicitLockAfterDefer) {
auto mutex = MockMutex{};
auto lck = ProxyLockableUniqueLock<MockMutex>{mutex, std::defer_lock};
EXPECT_TRUE(lck.mutex());
EXPECT_FALSE(lck.proxy());
EXPECT_FALSE(lck.state());
lck.lock();
EXPECT_TRUE(lck.mutex());
EXPECT_TRUE(lck.proxy());
EXPECT_TRUE(lck.state());
EXPECT_EQ(mutex.locked_, 1);
}
......@@ -208,12 +223,12 @@ TEST_F(ProxyLockableTest, UniqueLockLockExplicitUnlockAfterDefer) {
auto mutex = MockMutex{};
auto lck = ProxyLockableUniqueLock<MockMutex>{mutex, std::defer_lock};
EXPECT_TRUE(lck.mutex());
EXPECT_FALSE(lck.proxy());
EXPECT_FALSE(lck.state());
lck.lock();
EXPECT_TRUE(lck.mutex());
EXPECT_TRUE(lck.proxy());
EXPECT_TRUE(lck.state());
EXPECT_EQ(mutex.locked_, 1);
lck.unlock();
......@@ -224,12 +239,12 @@ TEST_F(ProxyLockableTest, UniqueLockLockExplicitTryLockAfterDefer) {
auto mutex = MockMutex{};
auto lck = ProxyLockableUniqueLock<MockMutex>{mutex, std::defer_lock};
EXPECT_TRUE(lck.mutex());
EXPECT_FALSE(lck.proxy());
EXPECT_FALSE(lck.state());
EXPECT_TRUE(lck.try_lock());
EXPECT_TRUE(lck.mutex());
EXPECT_TRUE(lck.proxy());
EXPECT_TRUE(lck.state());
EXPECT_EQ(mutex.locked_, 1);
lck.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