From 46a2251e9dbf09919021f95eb2fd1935a856a586 Mon Sep 17 00:00:00 2001
From: Sebastian Messmer <mail@smessmer.de>
Date: Sat, 24 Sep 2016 10:56:36 +0200
Subject: [PATCH] Fix potential (although improbable) deadlock

---
 ChangeLog.txt                                        | 3 +++
 src/blockstore/implementations/caching/cache/Cache.h | 1 +
 src/cpp-utils/lock/LockPool.h                        | 1 +
 src/cpp-utils/lock/MutexPoolLock.h                   | 8 ++++++--
 4 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/ChangeLog.txt b/ChangeLog.txt
index a6e439f0..736ec295 100644
--- a/ChangeLog.txt
+++ b/ChangeLog.txt
@@ -1,5 +1,8 @@
 Version 0.9.6 (unreleased)
 ---------------
+Fixed bugs:
+* Fix potential (although very improbable) deadlock
+
 Compatibility:
 * Compatible with libcurl version >= 7.50.0, and <= 7.21.6 (tested down to 7.19.0)
 * Compatible with Crypto++ 5.6.4
diff --git a/src/blockstore/implementations/caching/cache/Cache.h b/src/blockstore/implementations/caching/cache/Cache.h
index 2a7462a7..8b828edc 100644
--- a/src/blockstore/implementations/caching/cache/Cache.h
+++ b/src/blockstore/implementations/caching/cache/Cache.h
@@ -111,6 +111,7 @@ void Cache<Key, Value, MAX_ENTRIES>::_deleteEntry(std::unique_lock<std::mutex> *
   // i.e. pop() and push() can be called here, except for pop() on the element in _currentlyFlushingEntries
   lock->unlock();
   value = boost::none; // Call destructor
+  lockEntryFromBeingPopped.unlock();  // unlock this one first to keep same locking oder (preventing potential deadlock)
   lock->lock();
 };
 
diff --git a/src/cpp-utils/lock/LockPool.h b/src/cpp-utils/lock/LockPool.h
index 7c652237..8e8cd878 100644
--- a/src/cpp-utils/lock/LockPool.h
+++ b/src/cpp-utils/lock/LockPool.h
@@ -42,6 +42,7 @@ namespace cpputils {
 
     template<class LockName>
     inline void LockPool<LockName>::lock(const LockName &lock, std::unique_lock<std::mutex> *lockToFreeWhileWaiting) {
+        ASSERT(lockToFreeWhileWaiting->owns_lock(), "Given lock must be locked");
         std::unique_lock<std::mutex> 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.
diff --git a/src/cpp-utils/lock/MutexPoolLock.h b/src/cpp-utils/lock/MutexPoolLock.h
index 43cbc228..965b04c5 100644
--- a/src/cpp-utils/lock/MutexPoolLock.h
+++ b/src/cpp-utils/lock/MutexPoolLock.h
@@ -23,11 +23,15 @@ namespace cpputils {
 
         ~MutexPoolLock() {
             if (_pool != nullptr) {
-                _pool->release(_lockName);
-                _pool = nullptr;
+                unlock();
             }
         }
 
+        void unlock() {
+            _pool->release(_lockName);
+            _pool = nullptr;
+        }
+
     private:
         LockPool<LockName> *_pool;
         LockName _lockName;