sync.h: add REVERSE_LOCK

Author: ajtowns

src/sync.cpp

@@ -13,7 +13,7 @@
 #include <util/strencodings.h>
 #include <util/threadnames.h>
 
-
+#include <system_error>
 #include <map>
 #include <set>
 
@@ -60,6 +60,11 @@ struct CLockLocation {
             mutexName, sourceFile, itostr(sourceLine), (fTry ? " (TRY)" : ""), m_thread_name);
     }
 
+    std::string Name() const
+    {
+        return mutexName;
+    }
+
 private:
     bool fTry;
     std::string mutexName;
@@ -155,6 +160,18 @@ void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs
     push_lock(cs, CLockLocation(pszName, pszFile, nLine, fTry, util::ThreadGetInternalName()));
 }
 
+void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line)
+{
+    if (!g_lockstack.empty()) {
+        const auto& lastlock = g_lockstack.back();
+        if (lastlock.first == cs) {
+            lockname = lastlock.second.Name();
+            return;
+        }
+    }
+    throw std::system_error(EPERM, std::generic_category(), strprintf("%s:%s %s was not most recent critical section locked", file, line, guardname));
+}
+
 void LeaveCritical()
 {
     pop_lock();

src/sync.h

@@ -50,6 +50,7 @@ LEAVE_CRITICAL_SECTION(mutex); // no RAII
 #ifdef DEBUG_LOCKORDER
 void EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false);
 void LeaveCritical();
+void CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line);
 std::string LocksHeld();
 void AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs);
 void AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs);
@@ -64,6 +65,7 @@ extern bool g_debug_lockorder_abort;
 #else
 void static inline EnterCritical(const char* pszName, const char* pszFile, int nLine, void* cs, bool fTry = false) {}
 void static inline LeaveCritical() {}
+void static inline CheckLastCritical(void* cs, std::string& lockname, const char* guardname, const char* file, int line) {}
 void static inline AssertLockHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) ASSERT_EXCLUSIVE_LOCK(cs) {}
 void static inline AssertLockNotHeldInternal(const char* pszName, const char* pszFile, int nLine, void* cs) {}
 void static inline DeleteLock(void* cs) {}
@@ -171,8 +173,45 @@ class SCOPED_LOCKABLE UniqueLock : public Base
     {
         return Base::owns_lock();
     }
+
+protected:
+    // needed for reverse_lock
+    UniqueLock() { }
+
+public:
+    /**
+     * An RAII-style reverse lock. Unlocks on construction and locks on destruction.
+     */
+    class reverse_lock {
+    public:
+        explicit reverse_lock(UniqueLock& _lock, const char* _guardname, const char* _file, int _line) : lock(_lock), file(_file), line(_line) {
+            CheckLastCritical((void*)lock.mutex(), lockname, _guardname, _file, _line);
+            lock.unlock();
+            LeaveCritical();
+            lock.swap(templock);
+        }
+
+        ~reverse_lock() {
+            templock.swap(lock);
+            EnterCritical(lockname.c_str(), file.c_str(), line, (void*)lock.mutex());
+            lock.lock();
+        }
+
+     private:
+        reverse_lock(reverse_lock const&);
+        reverse_lock& operator=(reverse_lock const&);
+
+        UniqueLock& lock;
+        UniqueLock templock;
+        std::string lockname;
+        const std::string file;
+        const int line;
+     };
+     friend class reverse_lock;
 };
 
+#define REVERSE_LOCK(g) decltype(g)::reverse_lock PASTE2(revlock, __COUNTER__)(g, #g, __FILE__, __LINE__)
+
 template<typename MutexArg>
 using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>;
 

src/test/reverselock_tests.cpp

@@ -2,7 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
-#include <reverselock.h>
+#include <sync.h>
 #include <test/util/setup_common.h>
 
 #include <boost/test/unit_test.hpp>
@@ -11,21 +11,50 @@ BOOST_FIXTURE_TEST_SUITE(reverselock_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(reverselock_basics)
 {
-    boost::mutex mutex;
-    boost::unique_lock<boost::mutex> lock(mutex);
+    Mutex mutex;
+    WAIT_LOCK(mutex, lock);
 
     BOOST_CHECK(lock.owns_lock());
     {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
         BOOST_CHECK(!lock.owns_lock());
     }
     BOOST_CHECK(lock.owns_lock());
 }
 
+BOOST_AUTO_TEST_CASE(reverselock_multiple)
+{
+    Mutex mutex2;
+    Mutex mutex;
+    WAIT_LOCK(mutex2, lock2);
+    WAIT_LOCK(mutex, lock);
+
+    // Make sure undoing two locks succeeds
+    {
+        REVERSE_LOCK(lock);
+        BOOST_CHECK(!lock.owns_lock());
+        REVERSE_LOCK(lock2);
+        BOOST_CHECK(!lock2.owns_lock());
+    }
+    BOOST_CHECK(lock.owns_lock());
+    BOOST_CHECK(lock2.owns_lock());
+}
+
 BOOST_AUTO_TEST_CASE(reverselock_errors)
 {
-    boost::mutex mutex;
-    boost::unique_lock<boost::mutex> lock(mutex);
+    Mutex mutex2;
+    Mutex mutex;
+    WAIT_LOCK(mutex2, lock2);
+    WAIT_LOCK(mutex, lock);
+
+#ifdef DEBUG_LOCKORDER
+    // Make sure trying to reverse lock a previous lock fails
+    try {
+        REVERSE_LOCK(lock2);
+        BOOST_CHECK(false); // REVERSE_LOCK(lock2) succeeded
+    } catch(...) { }
+    BOOST_CHECK(lock2.owns_lock());
+#endif
 
     // Make sure trying to reverse lock an unlocked lock fails
     lock.unlock();
@@ -34,7 +63,7 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
 
     bool failed = false;
     try {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
     } catch(...) {
         failed = true;
     }
@@ -49,7 +78,7 @@ BOOST_AUTO_TEST_CASE(reverselock_errors)
     lock.lock();
     BOOST_CHECK(lock.owns_lock());
     {
-        reverse_lock<boost::unique_lock<boost::mutex> > rlock(lock);
+        REVERSE_LOCK(lock);
         BOOST_CHECK(!lock.owns_lock());
     }