diff --git a/src/cpp-utils/lock/LockPool.h b/src/cpp-utils/lock/LockPool.h index 8e8cd878..968770ef 100644 --- a/src/cpp-utils/lock/LockPool.h +++ b/src/cpp-utils/lock/LockPool.h @@ -20,11 +20,13 @@ namespace cpputils { public: LockPool(); ~LockPool(); - void lock(const LockName &lock, std::unique_lock *lockToFreeWhileWaiting = nullptr); - void release(const LockName &lock); + void lock(const LockName &lockName); + void lock(const LockName &lockName, std::unique_lock *lockToFreeWhileWaiting); + void release(const LockName &lockName); private: - bool _isLocked(const LockName &lock) const; + bool _isLocked(const LockName &lockName) const; + template void _lock(const LockName &lockName, OuterLock *lockToFreeWhileWaiting); std::vector _lockedLocks; std::mutex _mutex; @@ -41,31 +43,44 @@ namespace cpputils { } template - inline void LockPool::lock(const LockName &lock, std::unique_lock *lockToFreeWhileWaiting) { + inline void LockPool::lock(const LockName &lockName, std::unique_lock *lockToFreeWhileWaiting) { ASSERT(lockToFreeWhileWaiting->owns_lock(), "Given lock must be locked"); std::unique_lock mutexLock(_mutex); // TODO Is shared_lock enough here? - if (_isLocked(lock)) { - // Order of locking/unlocking is important and should be the same order as everywhere else to prevent deadlocks. - // Since when entering the function, lockToFreeWhileWaiting is already locked and mutexLock is locked afterwards, - // the condition variable should do it in the same order. We use combinedLock for this. - CombinedLock combinedLock(lockToFreeWhileWaiting, &mutexLock); - _cv.wait(combinedLock, [this, &lock]{ - return !_isLocked(lock); + // Order of locking/unlocking is important and should be the same order as everywhere else to prevent deadlocks. + // Since when entering the function, lockToFreeWhileWaiting is already locked and mutexLock is locked afterwards, + // the condition variable should do it in the same order. We use combinedLock for this. + CombinedLock combinedLock(lockToFreeWhileWaiting, &mutexLock); + _lock(lockName, &combinedLock); + ASSERT(mutexLock.owns_lock() && lockToFreeWhileWaiting->owns_lock(), "Locks haven't been correctly relocked"); + } + + template + inline void LockPool::lock(const LockName &lockName) { + std::unique_lock mutexLock(_mutex); // TODO Is shared_lock enough here? + _lock(lockName, &mutexLock); + ASSERT(mutexLock.owns_lock(), "Lock hasn't been correctly relocked"); + } + + template + template + inline void LockPool::_lock(const LockName &lockName, OuterLock *mutexLock) { + if (_isLocked(lockName)) { + _cv.wait(*mutexLock, [this, &lockName]{ + return !_isLocked(lockName); }); - ASSERT(mutexLock.owns_lock() && lockToFreeWhileWaiting->owns_lock(), "Locks haven't been correctly relocked"); } - _lockedLocks.push_back(lock); + _lockedLocks.push_back(lockName); } template - inline bool LockPool::_isLocked(const LockName &lock) const { - return std::find(_lockedLocks.begin(), _lockedLocks.end(), lock) != _lockedLocks.end(); + inline bool LockPool::_isLocked(const LockName &lockName) const { + return std::find(_lockedLocks.begin(), _lockedLocks.end(), lockName) != _lockedLocks.end(); } template - inline void LockPool::release(const LockName &lock) { + inline void LockPool::release(const LockName &lockName) { std::unique_lock mutexLock(_mutex); - auto found = std::find(_lockedLocks.begin(), _lockedLocks.end(), lock); + auto found = std::find(_lockedLocks.begin(), _lockedLocks.end(), lockName); ASSERT(found != _lockedLocks.end(), "Lock given to release() was not locked"); _lockedLocks.erase(found); _cv.notify_all(); diff --git a/src/cpp-utils/lock/MutexPoolLock.h b/src/cpp-utils/lock/MutexPoolLock.h index 965b04c5..a5bd731c 100644 --- a/src/cpp-utils/lock/MutexPoolLock.h +++ b/src/cpp-utils/lock/MutexPoolLock.h @@ -28,6 +28,7 @@ namespace cpputils { } void unlock() { + ASSERT(_pool != nullptr, "MutexPoolLock is not locked"); _pool->release(_lockName); _pool = nullptr; }