diff --git a/src/sync.h b/src/sync.h --- a/src/sync.h +++ b/src/sync.h @@ -50,6 +50,8 @@ 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); @@ -67,6 +69,9 @@ static inline void EnterCritical(const char *pszName, const char *pszFile, int nLine, void *cs, bool fTry = false) {} static inline void LeaveCritical() {} +static inline void CheckLastCritical(void *cs, std::string &lockname, + const char *guardname, const char *file, + int line) {} static inline void AssertLockHeldInternal(const char *pszName, const char *pszFile, int nLine, void *cs) ASSERT_EXCLUSIVE_LOCK(cs) {} @@ -168,8 +173,52 @@ } operator bool() { 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 using DebugLock = UniqueLock::type>::type>; diff --git a/src/sync.cpp b/src/sync.cpp --- a/src/sync.cpp +++ b/src/sync.cpp @@ -13,6 +13,7 @@ #include #include #include +#include #ifdef DEBUG_LOCKCONTENTION void PrintLockContention(const char *pszName, const char *pszFile, int nLine) { @@ -46,6 +47,8 @@ m_thread_name); } + std::string Name() const { return mutexName; } + private: bool fTry; std::string mutexName; @@ -149,6 +152,21 @@ 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(); } diff --git a/src/test/reverselock_tests.cpp b/src/test/reverselock_tests.cpp --- a/src/test/reverselock_tests.cpp +++ b/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 +#include #include @@ -11,20 +11,49 @@ BOOST_FIXTURE_TEST_SUITE(reverselock_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(reverselock_basics) { - boost::mutex mutex; - boost::unique_lock lock(mutex); + Mutex mutex; + WAIT_LOCK(mutex, lock); BOOST_CHECK(lock.owns_lock()); { - reverse_lock> 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 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(); @@ -33,7 +62,7 @@ bool failed = false; try { - reverse_lock> rlock(lock); + REVERSE_LOCK(lock); } catch (...) { failed = true; } @@ -48,7 +77,7 @@ lock.lock(); BOOST_CHECK(lock.owns_lock()); { - reverse_lock> rlock(lock); + REVERSE_LOCK(lock); BOOST_CHECK(!lock.owns_lock()); }