Commit 15be4529 authored by Aaryaman Sagar's avatar Aaryaman Sagar Committed by Facebook Github Bot

Add back const write lock methods to Synchronized

Summary:
The behavior of not allowing write locks on const methods is inconsistent, as
there is no way of determining the semantics of the protected object.  In
particular there are two types of classes that come into mind here

- Pointer and reference like classes.  These can be const, but the underlying
  data item need not be const.  Here it should be perfectly resaonable to
  allow users to acquire a write lock on a const Synchronized<> object
- Types with mutable members.  These can be write locked even when const, this
  behavior is probably okay.

On the other hand the previous motivation of this diff - inconsistency with
upgrade locks is being removed.  They will no longer expose non-const access

Reviewed By: yfeldblum

Differential Revision: D13478631

fbshipit-source-id: 652a08c61abf35c3eadc45cedc5d300fbef83a6b
parent eeeed147
...@@ -103,6 +103,9 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> { ...@@ -103,6 +103,9 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
LockedPtr wlock() { LockedPtr wlock() {
return LockedPtr(static_cast<Subclass*>(this)); return LockedPtr(static_cast<Subclass*>(this));
} }
ConstWLockedPtr wlock() const {
return ConstWLockedPtr(static_cast<const Subclass*>(this));
}
/** /**
* Attempts to acquire the lock in exclusive mode. If acquisition is * Attempts to acquire the lock in exclusive mode. If acquisition is
...@@ -114,6 +117,9 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> { ...@@ -114,6 +117,9 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
TryWLockedPtr tryWLock() { TryWLockedPtr tryWLock() {
return TryWLockedPtr{static_cast<Subclass*>(this)}; return TryWLockedPtr{static_cast<Subclass*>(this)};
} }
ConstTryWLockedPtr tryWLock() const {
return ConstTryWLockedPtr{static_cast<const Subclass*>(this)};
}
/** /**
* Acquire a read lock, and return a ConstLockedPtr that can be used to * Acquire a read lock, and return a ConstLockedPtr that can be used to
...@@ -145,6 +151,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> { ...@@ -145,6 +151,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
LockedPtr wlock(const std::chrono::duration<Rep, Period>& timeout) { LockedPtr wlock(const std::chrono::duration<Rep, Period>& timeout) {
return LockedPtr(static_cast<Subclass*>(this), timeout); return LockedPtr(static_cast<Subclass*>(this), timeout);
} }
template <class Rep, class Period>
LockedPtr wlock(const std::chrono::duration<Rep, Period>& timeout) const {
return LockedPtr(static_cast<const Subclass*>(this), timeout);
}
/** /**
* Attempts to acquire the lock, or fails if the timeout elapses first. * Attempts to acquire the lock, or fails if the timeout elapses first.
...@@ -177,6 +187,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> { ...@@ -177,6 +187,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
auto withWLock(Function&& function) { auto withWLock(Function&& function) {
return function(*wlock()); return function(*wlock());
} }
template <class Function>
auto withWLock(Function&& function) const {
return function(*wlock());
}
/** /**
* Invoke a function while holding the lock exclusively. * Invoke a function while holding the lock exclusively.
...@@ -191,6 +205,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> { ...@@ -191,6 +205,10 @@ class SynchronizedBase<Subclass, detail::MutexLevel::SHARED> {
auto withWLockPtr(Function&& function) { auto withWLockPtr(Function&& function) {
return function(wlock()); return function(wlock());
} }
template <class Function>
auto withWLockPtr(Function&& function) const {
return function(wlock());
}
/** /**
* Invoke a function while holding an the lock in shared mode. * Invoke a function while holding an the lock in shared mode.
...@@ -1519,6 +1537,10 @@ template <typename D, typename M, typename... Args> ...@@ -1519,6 +1537,10 @@ template <typename D, typename M, typename... Args>
auto wlock(Synchronized<D, M>& synchronized, Args&&... args) { auto wlock(Synchronized<D, M>& synchronized, Args&&... args) {
return detail::wlock(synchronized, std::forward<Args>(args)...); return detail::wlock(synchronized, std::forward<Args>(args)...);
} }
template <typename D, typename M, typename... Args>
auto wlock(const Synchronized<D, M>& synchronized, Args&&... args) {
return detail::wlock(synchronized, std::forward<Args>(args)...);
}
template <typename Data, typename Mutex, typename... Args> template <typename Data, typename Mutex, typename... Args>
auto rlock(const Synchronized<Data, Mutex>& synchronized, Args&&... args) { auto rlock(const Synchronized<Data, Mutex>& synchronized, Args&&... args) {
return detail::rlock(synchronized, std::forward<Args>(args)...); return detail::rlock(synchronized, std::forward<Args>(args)...);
......
...@@ -216,6 +216,10 @@ testWithLock() { ...@@ -216,6 +216,10 @@ testWithLock() {
obj.withWLock([](const std::vector<int>& lockedObj) { obj.withWLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size()); EXPECT_EQ(1002, lockedObj.size());
}); });
constObj.withWLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size());
EXPECT_EQ(11, lockedObj.back());
});
obj.withRLock([](const std::vector<int>& lockedObj) { obj.withRLock([](const std::vector<int>& lockedObj) {
EXPECT_EQ(1002, lockedObj.size()); EXPECT_EQ(1002, lockedObj.size());
EXPECT_EQ(11, lockedObj.back()); EXPECT_EQ(11, lockedObj.back());
...@@ -228,6 +232,10 @@ testWithLock() { ...@@ -228,6 +232,10 @@ testWithLock() {
obj.withWLock([](auto& lockedObj) { lockedObj.push_back(12); }); obj.withWLock([](auto& lockedObj) { lockedObj.push_back(12); });
obj.withWLock( obj.withWLock(
[](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); }); [](const auto& lockedObj) { EXPECT_EQ(1003, lockedObj.size()); });
constObj.withWLock([](const auto& lockedObj) {
EXPECT_EQ(1003, lockedObj.size());
EXPECT_EQ(12, lockedObj.back());
});
obj.withRLock([](const auto& lockedObj) { obj.withRLock([](const auto& lockedObj) {
EXPECT_EQ(1003, lockedObj.size()); EXPECT_EQ(1003, lockedObj.size());
EXPECT_EQ(12, lockedObj.back()); EXPECT_EQ(12, lockedObj.back());
...@@ -257,6 +265,10 @@ testWithLock() { ...@@ -257,6 +265,10 @@ testWithLock() {
} }
EXPECT_EQ(15, lockedObj->back()); EXPECT_EQ(15, lockedObj->back());
}); });
constObj.withWLockPtr([](auto&& lockedObj) {
EXPECT_EQ(1005, lockedObj->size());
EXPECT_EQ(15, lockedObj->back());
});
#else #else
obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) { obj.withWLockPtr([](typename SynchType::LockedPtr&& lockedObj) {
lockedObj->push_back(13); lockedObj->push_back(13);
...@@ -269,6 +281,10 @@ testWithLock() { ...@@ -269,6 +281,10 @@ testWithLock() {
lockedObj->push_back(16); lockedObj->push_back(16);
EXPECT_EQ(1006, lockedObj->size()); EXPECT_EQ(1006, lockedObj->size());
}); });
constObj.withWLockPtr([](typename SynchType::ConstWLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back());
});
obj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) { obj.withRLockPtr([](typename SynchType::ConstLockedPtr&& lockedObj) {
EXPECT_EQ(1006, lockedObj->size()); EXPECT_EQ(1006, lockedObj->size());
EXPECT_EQ(16, lockedObj->back()); EXPECT_EQ(16, lockedObj->back());
......
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