Merge bitcoin/bitcoin#25676: sync: simplify and remove unused code from sync.h

75c3f9f8806259ac7ac02e725d2f2f48e5a1d954 sync: rename AnnotatedMixin::UniqueLock to AnnotatedMixin::unique_lock (Vasil Dimov)
8d9ee8efe80edeb04824b0daee236ed1a3f53a87 sync: remove DebugLock alias template (Vasil Dimov)
4b2e16763fbe70e7cdcf82b438d415e9d96f1674 sync: avoid confusing name overlap (Mutex) (Vasil Dimov)
9d7ae4b66c9ce202d51286daac9be7e599d6a629 sync: remove unused template parameter from ::UniqueLock (Vasil Dimov)
11c190e3f18b43ecb120a5f3e81243fb6fd97261 sync: simplify MaybeCheckNotHeld() definitions by using a template (Vasil Dimov)

Pull request description:

  Summary:

  * Reduce 4 of the `MaybeCheckNotHeld()` definitions to 2 by using a template.
  * Remove unused template parameter from `::UniqueLock`.
  * Use `MutexType` instead of `Mutex` for a template parameter name to avoid overlap/confusion with the `Mutex` class.
  * Rename `AnnotatedMixin::UniqueLock` to `AnnotatedMixin::unique_lock` to avoid overlap/confusion with the global `UniqueLock` and for consistency with `UniqueLock::reverse_lock`.

  The first commit `sync: simplify MaybeCheckNotHeld() definitions by using a template` is also part of https://github.com/bitcoin/bitcoin/pull/25390

ACKs for top commit:
  aureleoules:
    ACK 75c3f9f8806259ac7ac02e725d2f2f48e5a1d954 - LGTM
  ryanofsky:
    Code review ACK 75c3f9f8806259ac7ac02e725d2f2f48e5a1d954. Nice cleanups! Just suggested changes since last review: keeping UniqueLock name and fixing a missed rename in a code comment

Tree-SHA512: ec261f6a444bdfe4f06e844b57b3606fdd9b2f842647cae15266d9729970d87585c808d482fbba0b31c33a4aa03527c36e282c92b28d9052711f75a7048c96f1

1 files changed, 19 insertions, 22 deletions

diff --git a/src/sync.h b/src/sync.h
index a9d0091bd2..1f4e191214 100644
--- a/src/sync.h
+++ b/src/sync.h
@@ -111,7 +111,7 @@ public:
         return PARENT::try_lock();
     }
 
-    using UniqueLock = std::unique_lock<PARENT>;
+    using unique_lock = std::unique_lock<PARENT>;
 #ifdef __clang__
     //! For negative capabilities in the Clang Thread Safety Analysis.
     //! A negative requirement uses the EXCLUSIVE_LOCKS_REQUIRED attribute, in conjunction
@@ -147,11 +147,13 @@ inline void AssertLockNotHeldInline(const char* name, const char* file, int line
 inline void AssertLockNotHeldInline(const char* name, const char* file, int line, GlobalMutex* cs) LOCKS_EXCLUDED(cs) { AssertLockNotHeldInternal(name, file, line, cs); }
 #define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs)
 
-/** Wrapper around std::unique_lock style lock for Mutex. */
-template <typename Mutex, typename Base = typename Mutex::UniqueLock>
-class SCOPED_LOCKABLE UniqueLock : public Base
+/** Wrapper around std::unique_lock style lock for MutexType. */
+template <typename MutexType>
+class SCOPED_LOCKABLE UniqueLock : public MutexType::unique_lock
 {
 private:
+    using Base = typename MutexType::unique_lock;
+
     void Enter(const char* pszName, const char* pszFile, int nLine)
     {
         EnterCritical(pszName, pszFile, nLine, Base::mutex());
@@ -173,7 +175,7 @@ private:
     }
 
 public:
-    UniqueLock(Mutex& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
+    UniqueLock(MutexType& mutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(mutexIn) : Base(mutexIn, std::defer_lock)
     {
         if (fTry)
             TryEnter(pszName, pszFile, nLine);
@@ -181,7 +183,7 @@ public:
             Enter(pszName, pszFile, nLine);
     }
 
-    UniqueLock(Mutex* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
+    UniqueLock(MutexType* pmutexIn, const char* pszName, const char* pszFile, int nLine, bool fTry = false) EXCLUSIVE_LOCK_FUNCTION(pmutexIn)
     {
         if (!pmutexIn) return;
 
@@ -241,29 +243,24 @@ public:
 
 #define REVERSE_LOCK(g) typename std::decay<decltype(g)>::type::reverse_lock UNIQUE_NAME(revlock)(g, #g, __FILE__, __LINE__)
 
-template<typename MutexArg>
-using DebugLock = UniqueLock<typename std::remove_reference<typename std::remove_pointer<MutexArg>::type>::type>;
-
 // When locking a Mutex, require negative capability to ensure the lock
 // is not already held
 inline Mutex& MaybeCheckNotHeld(Mutex& cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; }
 inline Mutex* MaybeCheckNotHeld(Mutex* cs) EXCLUSIVE_LOCKS_REQUIRED(!cs) LOCK_RETURNED(cs) { return cs; }
 
-// When locking a GlobalMutex, just check it is not locked in the surrounding scope
-inline GlobalMutex& MaybeCheckNotHeld(GlobalMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; }
-inline GlobalMutex* MaybeCheckNotHeld(GlobalMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; }
-
-// When locking a RecursiveMutex, it's okay to already hold the lock
-// but check that it is not known to be locked in the surrounding scope anyway
-inline RecursiveMutex& MaybeCheckNotHeld(RecursiveMutex& cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; }
-inline RecursiveMutex* MaybeCheckNotHeld(RecursiveMutex* cs) LOCKS_EXCLUDED(cs) LOCK_RETURNED(cs) { return cs; }
+// When locking a GlobalMutex or RecursiveMutex, just check it is not
+// locked in the surrounding scope.
+template <typename MutexType>
+inline MutexType& MaybeCheckNotHeld(MutexType& m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; }
+template <typename MutexType>
+inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; }
 
-#define LOCK(cs) DebugLock<decltype(cs)> UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
+#define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
 #define LOCK2(cs1, cs2)                                               \
-    DebugLock<decltype(cs1)> criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \
-    DebugLock<decltype(cs2)> criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__)
-#define TRY_LOCK(cs, name) DebugLock<decltype(cs)> name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true)
-#define WAIT_LOCK(cs, name) DebugLock<decltype(cs)> name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
+    UniqueLock criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \
+    UniqueLock criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__)
+#define TRY_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true)
+#define WAIT_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
 
 #define ENTER_CRITICAL_SECTION(cs)                            \
     {                                                         \