Tried cherrypicking
<details><summary>minimal diff to change m_nodes_mutex mutex type and ForNode usage</summary>
diff --git a/src/net.h b/src/net.h
index d806b42a1e..f997bbe820 100644
--- a/src/net.h
+++ b/src/net.h
@@ -1519,7 +1519,7 @@ private:
mutable Mutex m_added_nodes_mutex;
std::vector<CNode*> m_nodes GUARDED_BY(m_nodes_mutex);
std::list<CNode*> m_nodes_disconnected;
- mutable RecursiveMutex m_nodes_mutex;
+ mutable Mutex m_nodes_mutex;
std::atomic<NodeId> nLastNodeId{0};
unsigned int nPrevNodeCount{0};
diff --git a/src/net_processing.cpp b/src/net_processing.cpp
index 3777832215..20dcca2ca7 100644
--- a/src/net_processing.cpp
+++ b/src/net_processing.cpp
@@ -1285,23 +1285,23 @@ void PeerManagerImpl::MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid)
}
m_connman.ForNode(nodeid, [this](CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(::cs_main) {
AssertLockHeld(::cs_main);
- if (lNodesAnnouncingHeaderAndIDs.size() >= 3) {
- // As per BIP152, we only get 3 of our peers to announce
- // blocks using compact encodings.
- m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){
- MakeAndPushMessage(*pnodeStop, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION);
- // save BIP152 bandwidth state: we select peer to be low-bandwidth
- pnodeStop->m_bip152_highbandwidth_to = false;
- return true;
- });
- lNodesAnnouncingHeaderAndIDs.pop_front();
- }
MakeAndPushMessage(*pfrom, NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOCKS_VERSION);
// save BIP152 bandwidth state: we select peer to be high-bandwidth
pfrom->m_bip152_highbandwidth_to = true;
lNodesAnnouncingHeaderAndIDs.push_back(pfrom->GetId());
return true;
});
+ if (lNodesAnnouncingHeaderAndIDs.size() > 3) {
+ // As per BIP152, we only get 3 of our peers to announce
+ // blocks using compact encodings.
+ m_connman.ForNode(lNodesAnnouncingHeaderAndIDs.front(), [this](CNode* pnodeStop){
+ MakeAndPushMessage(*pnodeStop, NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION);
+ // save BIP152 bandwidth state: we select peer to be low-bandwidth
+ pnodeStop->m_bip152_highbandwidth_to = false;
+ return true;
+ });
+ lNodesAnnouncingHeaderAndIDs.pop_front();
+ }
}
bool PeerManagerImpl::TipMayBeStale()
</details>
...and building with Clang 19.1.7 on Linux results in a lot of warnings spam:
<details><summary>"calling function 'MaybeCheckNotHeld' requires negative"-spam</summary>
In file included from ../src/node/peerman_args.cpp:5:
In file included from ../src/node/peerman_args.h:8:
In file included from ../src/net_processing.h:10:
../src/net.h:1186:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1186 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
In file included from ../src/node/peerman_args.cpp:5:
In file included from ../src/node/peerman_args.h:8:
In file included from ../src/net_processing.h:10:
../src/net.h:1195:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1195 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
2 warnings generated.
[3/26] Building CXX object src/CMakeFiles/bitcoin_node.dir/node/txreconciliation.cpp.o
In file included from ../src/node/txreconciliation.cpp:5:
In file included from ../src/node/txreconciliation.h:8:
../src/net.h:1186:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1186 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
In file included from ../src/node/txreconciliation.cpp:5:
In file included from ../src/node/txreconciliation.h:8:
../src/net.h:1195:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1195 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
2 warnings generated.
[4/26] Building CXX object src/CMakeFiles/bitcoin_node.dir/mapport.cpp.o
In file included from ../src/mapport.cpp:12:
../src/net.h:1186:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1186 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
In file included from ../src/mapport.cpp:12:
../src/net.h:1195:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1195 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
2 warnings generated.
[5/26] Building CXX object src/CMakeFiles/bitcoin_node.dir/headerssync.cpp.o
In file included from ../src/headerssync.cpp:5:
In file included from ../src/headerssync.h:11:
../src/net.h:1186:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1186 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
In file included from ../src/headerssync.cpp:5:
In file included from ../src/headerssync.h:11:
../src/net.h:1195:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1195 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
2 warnings generated.
[6/26] Building CXX object src/CMakeFiles/bitcoin_node.dir/node/context.cpp.o
In file included from ../src/node/context.cpp:13:
../src/net.h:1186:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1186 | LOCK(m_nodes_mutex);
| ^
../src/sync.h:259:56: note: expanded from macro 'LOCK'
259 | #define LOCK(cs) UniqueLock UNIQUE_NAME(criticalblock)(MaybeCheckNotHeld(cs), #cs, __FILE__, __LINE__)
| ^
In file included from ../src/node/context.cpp:13:
../src/net.h:1195:9: warning: calling function 'MaybeCheckNotHeld' requires negative capability '!m_nodes_mutex' [-Wthread-safety-analysis]
1195 | LOCK(m_nodes_mutex);
| ^
...
</details>
So I think it's quite natural to want to add all the annotations to silence the warnings within the same commit.
