From 11c190e3f18b43ecb120a5f3e81243fb6fd97261 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Wed, 13 Jul 2022 10:13:31 +0200 Subject: [PATCH 1/5] sync: simplify MaybeCheckNotHeld() definitions by using a template Reduce 4 of the `MaybeCheckNotHeld()` definitions to 2 by using a template. This also makes the function usable for other [BasicLockable](https://en.cppreference.com/w/cpp/named_req/BasicLockable) types. --- src/sync.h | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/sync.h b/src/sync.h index c34d969041e..515e9d12fc1 100644 --- a/src/sync.h +++ b/src/sync.h @@ -249,14 +249,12 @@ using DebugLock = UniqueLock +inline MutexType& MaybeCheckNotHeld(MutexType& m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; } +template +inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; } #define LOCK(cs) DebugLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__) #define LOCK2(cs1, cs2) \ From 9d7ae4b66c9ce202d51286daac9be7e599d6a629 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 21 Jul 2022 14:56:38 +0200 Subject: [PATCH 2/5] sync: remove unused template parameter from ::UniqueLock The template parameter `typename Base = typename Mutex::UniqueLock` is not used, so remove it. Use internally defined type `Base` to avoid repetitions of `Mutex::UniqueLock`. --- src/sync.h | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/sync.h b/src/sync.h index 515e9d12fc1..f6d9a6cbe50 100644 --- a/src/sync.h +++ b/src/sync.h @@ -148,10 +148,12 @@ inline void AssertLockNotHeldInline(const char* name, const char* file, int line #define AssertLockNotHeld(cs) AssertLockNotHeldInline(#cs, __FILE__, __LINE__, &cs) /** Wrapper around std::unique_lock style lock for Mutex. */ -template -class SCOPED_LOCKABLE UniqueLock : public Base +template +class SCOPED_LOCKABLE UniqueLock : public Mutex::UniqueLock { private: + using Base = typename Mutex::UniqueLock; + void Enter(const char* pszName, const char* pszFile, int nLine) { EnterCritical(pszName, pszFile, nLine, Base::mutex()); From 4b2e16763fbe70e7cdcf82b438d415e9d96f1674 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Thu, 21 Jul 2022 15:12:43 +0200 Subject: [PATCH 3/5] sync: avoid confusing name overlap (Mutex) Use `MutexType` instead of `Mutex` for the template parameter of `UniqueLock` because there is already a class named `Mutex` and the naming overlap is confusing. `MutexType` is used elsewhere in `sync.h`. --- src/sync.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/sync.h b/src/sync.h index f6d9a6cbe50..730d599bd96 100644 --- a/src/sync.h +++ b/src/sync.h @@ -147,12 +147,12 @@ 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 -class SCOPED_LOCKABLE UniqueLock : public Mutex::UniqueLock +/** Wrapper around std::unique_lock style lock for MutexType. */ +template +class SCOPED_LOCKABLE UniqueLock : public MutexType::UniqueLock { private: - using Base = typename Mutex::UniqueLock; + using Base = typename MutexType::UniqueLock; void Enter(const char* pszName, const char* pszFile, int nLine) { @@ -175,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); @@ -183,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; From 8d9ee8efe80edeb04824b0daee236ed1a3f53a87 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Fri, 22 Jul 2022 11:23:00 +0200 Subject: [PATCH 4/5] sync: remove DebugLock alias template Use `UniqueLock` directly. Type deduction works just fine from the first argument to the constructor of `UniqueLock`, so there is no need to repeat ```cpp UniqueLock::type>::type> ``` five times in the `LOCK` macros. Just `UniqueLock` suffices. --- src/sync.h | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/sync.h b/src/sync.h index 730d599bd96..4956bc6837a 100644 --- a/src/sync.h +++ b/src/sync.h @@ -243,9 +243,6 @@ public: #define REVERSE_LOCK(g) typename std::decay::type::reverse_lock UNIQUE_NAME(revlock)(g, #g, __FILE__, __LINE__) -template -using DebugLock = UniqueLock::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; } @@ -258,12 +255,12 @@ inline MutexType& MaybeCheckNotHeld(MutexType& m) LOCKS_EXCLUDED(m) LOCK_RETURNE template inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNED(m) { return m; } -#define LOCK(cs) DebugLock 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 criticalblock1(MaybeCheckNotHeld(cs1), #cs1, __FILE__, __LINE__); \ - DebugLock criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__) -#define TRY_LOCK(cs, name) DebugLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true) -#define WAIT_LOCK(cs, name) DebugLock 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) \ { \ From 75c3f9f8806259ac7ac02e725d2f2f48e5a1d954 Mon Sep 17 00:00:00 2001 From: Vasil Dimov Date: Mon, 10 Oct 2022 09:10:14 +0200 Subject: [PATCH 5/5] sync: rename AnnotatedMixin::UniqueLock to AnnotatedMixin::unique_lock This avoids confusion with the global `UniqueLock` and the snake case is consistent with `UniqueLock::reverse_lock. --- src/sync.h | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/sync.h b/src/sync.h index 4956bc6837a..463f25489cc 100644 --- a/src/sync.h +++ b/src/sync.h @@ -111,7 +111,7 @@ public: return PARENT::try_lock(); } - using UniqueLock = std::unique_lock; + using unique_lock = std::unique_lock; #ifdef __clang__ //! For negative capabilities in the Clang Thread Safety Analysis. //! A negative requirement uses the EXCLUSIVE_LOCKS_REQUIRED attribute, in conjunction @@ -149,10 +149,10 @@ inline void AssertLockNotHeldInline(const char* name, const char* file, int line /** Wrapper around std::unique_lock style lock for MutexType. */ template -class SCOPED_LOCKABLE UniqueLock : public MutexType::UniqueLock +class SCOPED_LOCKABLE UniqueLock : public MutexType::unique_lock { private: - using Base = typename MutexType::UniqueLock; + using Base = typename MutexType::unique_lock; void Enter(const char* pszName, const char* pszFile, int nLine) {