Yeah, I had hoped for the same :(
The problem here (as usual with these annotations) is aliasing.
As a simplified example:
0CCheckQueueControl<CScriptCheck> control(m_chainman.GetCheckQueue());
1CCheckQueueControl<CScriptCheck> control2(m_chainman.GetCheckQueue());
Here, clang has no idea that &control.m_lock.m_control_mutex == &control2.m_lock.m_control_mutex.
The only alternative, really, would be something like:
0diff --git a/src/checkqueue.h b/src/checkqueue.h
1index ade793ab392..5249e040d24 100644
2--- a/src/checkqueue.h
3+++ b/src/checkqueue.h
4@@ -188,6 +188,11 @@ public:
5 }
6 }
7
8+ UniqueLock<Mutex> TryLock()
9+ {
10+ return {IN_PLACE_TRY_LOCK(m_control_mutex)};
11+ }
12+
13 ~CCheckQueue()
14 {
15 WITH_LOCK(m_mutex, m_request_stop = true);
16@@ -209,13 +214,14 @@ class CCheckQueueControl
17 {
18 private:
19 CCheckQueue<T, R>& pqueue;
20+ UniqueLock<Mutex> m_lock;
21 bool fDone;
22
23 public:
24 CCheckQueueControl() = delete;
25 CCheckQueueControl(const CCheckQueueControl&) = delete;
26 CCheckQueueControl& operator=(const CCheckQueueControl&) = delete;
27- explicit CCheckQueueControl(CCheckQueue<T>& pqueueIn) : pqueue(pqueueIn), fDone(false)
28+ explicit CCheckQueueControl(CCheckQueue<T>& pqueueIn, UniqueLock<Mutex>&& lock) : pqueue(pqueueIn), m_lock(std::move(lock)), fDone(false)
29 {
30 ENTER_CRITICAL_SECTION(pqueue.m_control_mutex);
31 }
32diff --git a/src/sync.h b/src/sync.h
33index b22956ef1ab..db4fd977cee 100644
34--- a/src/sync.h
35+++ b/src/sync.h
36@@ -193,6 +193,8 @@ public:
37 Enter(pszName, pszFile, nLine);
38 }
39
40+ UniqueLock(UniqueLock&&) = default;
41+
42 ~UniqueLock() UNLOCK_FUNCTION()
43 {
44 if (Base::owns_lock())
45@@ -260,6 +262,7 @@ inline MutexType* MaybeCheckNotHeld(MutexType* m) LOCKS_EXCLUDED(m) LOCK_RETURNE
46 UniqueLock criticalblock2(MaybeCheckNotHeld(cs2), #cs2, __FILE__, __LINE__)
47 #define TRY_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true)
48 #define WAIT_LOCK(cs, name) UniqueLock name(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
49+#define IN_PLACE_TRY_LOCK(cs) MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__, true
50
51 #define ENTER_CRITICAL_SECTION(cs) \
52 { \
53diff --git a/src/validation.cpp b/src/validation.cpp
54index 3fa7afe3090..d690191b38e 100644
55--- a/src/validation.cpp
56+++ b/src/validation.cpp
57@@ -2612,7 +2612,10 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state,
58 // doesn't invalidate pointers into the vector, and keep txsdata in scope
59 // for as long as `control`.
60 std::optional<CCheckQueueControl<CScriptCheck>> control;
61- if (fScriptChecks && parallel_script_checks) control.emplace(m_chainman.GetCheckQueue());
62+ auto& queue = m_chainman.GetCheckQueue();
63+ auto queue_lock = queue.TryLock();
64+ assert(queue_lock);
65+ if (fScriptChecks && parallel_script_checks) control.emplace(m_chainman.GetCheckQueue(), std::move(queue_lock));
66
67 std::vector<PrecomputedTransactionData> txsdata(block.vtx.size());
Unfortunately, try_lock is allowed to fail spuriously, so that’s not safe :(
