net: add NetEventsInterface::g_msgproc

ajtowns commented at 5:36 pm on September 7, 2022: contributor

There are many cases where we assume message processing is single-threaded in order for how we access node-related memory to be safe. Add an explicit mutex that we can use to document this, which allows the compiler to catch any cases where we try to access that memory from other threads and break that assumption.

fanquake added the label P2P on Sep 7, 2022

ajtowns commented at 5:59 pm on September 7, 2022: contributor

Resurrected from #24474.

The main purpose of this PR is to document the variables we assume are only accessed via one thread, and hence which we don’t bother to grab a mutex before accessing. Since these annotations are checked automatically by clang in CI, this should help avoid possible bugs in future.

To do this, this PR replaces the per-peer CNode::cs_sendProcessing (that was never actually used as a guard annotation for any variables, nor locked from any thread other than the message processing thread, and can only be used to guard other members of the CNode class) with a single, global, ~NetEventsInterface:g_mutex_msgproc_thread~ NetEventsInterface:g_msgproc_mutex that is locked once from the message processing thread and released only when the thread exits.

It would also be possible to have a “dummy” lockable type and use that to document these variables, rather than an actual mutex. I’ve done an example of that sort of approach at #25174 (comment) but for this case where the lock is only ever acquired once at startup and has no possible contention, the overhead seems so minimal as to not be worth worrying about.

In the long-term, simplifying the locking scheme, and perhaps allowing more things to happen in parallel are obviously still worthwhile; this PR doesn’t mean to imply that these things should only be used from a single thread in future, rather it’s just documenting that they are right now.

See also #25454#pullrequestreview-1030142634 #25557 (comment)

fanquake requested review from theuni on Sep 8, 2022

fanquake requested review from vasild on Sep 8, 2022

DrahtBot commented at 5:35 pm on September 8, 2022: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#25597 (refactor: replace RecursiveMutex cs_sendProcessing with Mutex by theStack)
#25572 (refactor: Introduce EvictionManager and use it for the inbound eviction logic by dergoegge)
#24545 (BIP324: Enable v2 P2P encrypted transport by dhruv)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

in src/net_processing.h:87 in f55747fa66 outdated

83@@ -84,7 +84,7 @@ class PeerManager : public CValidationInterface, public NetEventsInterface
84 
85     /** Process a single message from a peer. Public for fuzz testing */
86     virtual void ProcessMessage(CNode& pfrom, const std::string& msg_type, CDataStream& vRecv,
87-                                const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) = 0;
88+                                const std::chrono::microseconds time_received, const std::atomic<bool>& interruptMsgProc) EXCLUSIVE_LOCKS_REQUIRED(g_mutex_msgproc_thread) = 0;

jonatack commented at 6:10 am on September 11, 2022:

Runtime lock assertions on the definitions for the methods with added annotations on their declarations, e.g. for the first commit (same for the later commits):

 0--- a/src/net_processing.cpp
 1+++ b/src/net_processing.cpp
 2@@ -3135,6 +3135,7 @@ void PeerManagerImpl::ProcessMessage(CNode& pfrom, const std::string& msg_type,
 3                                      const std::chrono::microseconds time_received,
 4                                      const std::atomic<bool>& interruptMsgProc)
 5 {
 6+    AssertLockHeld(g_mutex_msgproc_thread);
 7 
 8@@ -4748,6 +4749,7 @@ bool PeerManagerImpl::MaybeDiscourageAndDisconnect(CNode& pnode, Peer& peer)
 9 
10 bool PeerManagerImpl::ProcessMessages(CNode* pfrom, std::atomic<bool>& interruptMsgProc)
11 {
12+    AssertLockHeld(g_mutex_msgproc_thread);
13 
14@@ -5240,6 +5242,7 @@ bool PeerManagerImpl::SetupAddressRelay(const CNode& node, Peer& peer)
15 
16 bool PeerManagerImpl::SendMessages(CNode* pto)
17 {
18+    AssertLockHeld(g_mutex_msgproc_thread);

ajtowns commented at 4:52 am on September 15, 2022:

Added the runtime assertions for these three, but not any of the private/internal methods in net_processing, since that doesn’t seem likely to have any benefit.

vasild commented at 9:18 am on September 15, 2022:

Actually, the guidelines are pretty explicit for this:

Combine annotations in function declarations with run-time asserts in function definitions:

I think it is better to remove or modify a rule from the guidelines instead of violating it.

jonatack commented at 6:14 am on September 11, 2022: contributor

Approach ACK. Review, each commit debug builds cleanly (clang 13) without thread safety errors, verified that removing some of the added locks in the first commit does cause thread safety errors. Ran bitcoind with the suggested runtime lock assertions.

in src/net.h:631 in f55747fa66 outdated

626@@ -629,6 +627,9 @@ class CNode
627 class NetEventsInterface
628 {
629 public:
630+    /** Mutex for anything that is only accessed via the msg processing thread */
631+    static Mutex g_mutex_msgproc_thread;

vasild commented at 3:27 pm on September 12, 2022:

nit: since it is not required that the variables protected by this mutex should only be used from a single thread in future, maybe drop _thread from the name of this mutex.

nit: it seems like unwritten convention to put _mutex at the end: foobar_mutex, I think this is the first one to use mutex_foobar

ajtowns commented at 2:41 am on September 13, 2022:

Renamed

vasild approved

vasild commented at 3:33 pm on September 12, 2022: contributor

ACK f55747fa66afe1dcca242c0903b17c93f0437279

Are you planning to actually make the message processing multi-threaded?

ajtowns force-pushed on Sep 13, 2022

ajtowns commented at 3:36 am on September 13, 2022: contributor

Are you planning to actually make the message processing multi-threaded?

Original point of all this was just to be able to make g_cs_orphans private to TxOrphanage without reducing guard annotations (#21527 and vExtraTxn*).

I don’t think multiple net_processing threads is likely to help much – I think blocking mostly happens due to cs_main getting locked which would just cause multiple threads to stall if we had them, rather than some node requiring a lot of local processing that holds up some other node’s local processing. Having either reader-writer locks for the mempool and cs_main or switching to a RCU model might help there, but is probably a lot of work. Either way, it seems helpful in the meantime to make it clearer how we protect things from race conditions.

ajtowns renamed this:
~~net: add NetEventsInterface::g_mutex_msgproc_thread mutex~~
net: add NetEventsInterface::g_msgproc_mutex
on Sep 13, 2022

vasild approved

vasild commented at 9:32 am on September 14, 2022: contributor

ACK 22e7c464db9719ebc8eba0143f5a6024aac658c3

A missing “by” in the commit message of net: drop cs_sendProcessing:

SendMessages() is now protected g_msgproc_mutex;

jonatack commented at 10:52 am on September 14, 2022: contributor

Any reason #26036 (review) wasn’t addressed in the last push? It would be helpful to know if the thread safety guidelines in the developer notes should be updated.

in src/net_processing.cpp:5102 in db5c65bafd outdated

5098@@ -5099,7 +5099,7 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
5099 
5100     // Remove addr records that the peer already knows about, and add new
5101     // addrs to the m_addr_known filter on the same pass.
5102-    auto addr_already_known = [&peer](const CAddress& addr) {
5103+    auto addr_already_known = [&peer](const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) {

MarcoFalke commented at 11:01 am on September 14, 2022:

db5c65bafd6360236e6009cbe4b470606720dce3: Any reason to add the namespace bloat here, but not elsewhere? It seems unlikely that there is another symbol with the same name in the global or another namespace. For that reason I’d also prefer to just make it a global in the global namespace to also de-bloat the other places.

  0diff --git a/src/net_processing.cpp b/src/net_processing.cpp
  1index 594141ca84..d9a1124b64 100644
  2--- a/src/net_processing.cpp
  3+++ b/src/net_processing.cpp
  4@@ -537,7 +537,7 @@ public:
  5 
  6 private:
  7     /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
  8-    void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main, NetEventsInterface::g_msgproc_mutex);
  9+    void ConsiderEviction(CNode& pto, Peer& peer, std::chrono::seconds time_in_seconds) EXCLUSIVE_LOCKS_REQUIRED(cs_main, g_msgproc_mutex);
 10 
 11     /** If we have extra outbound peers, try to disconnect the one with the oldest block announcement */
 12     void EvictExtraOutboundPeers(std::chrono::seconds now) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
 13@@ -601,7 +601,7 @@ private:
 14     void ProcessHeadersMessage(CNode& pfrom, Peer& peer,
 15                                std::vector<CBlockHeader>&& headers,
 16                                bool via_compact_block)
 17-        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, NetEventsInterface::g_msgproc_mutex);
 18+        EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex);
 19     /** Various helpers for headers processing, invoked by ProcessHeadersMessage() */
 20     /** Return true if headers are continuous and have valid proof-of-work (DoS points assigned on failure) */
 21     bool CheckHeadersPoW(const std::vector<CBlockHeader>& headers, const Consensus::Params& consensusParams, Peer& peer);
 22@@ -610,7 +610,7 @@ private:
 23     /** Deal with state tracking and headers sync for peers that send the
 24      * occasional non-connecting header (this can happen due to BIP 130 headers
 25      * announcements for blocks interacting with the 2hr (MAX_FUTURE_BLOCK_TIME) rule). */
 26-    void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 27+    void HandleFewUnconnectingHeaders(CNode& pfrom, Peer& peer, const std::vector<CBlockHeader>& headers) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 28     /** Return true if the headers connect to each other, false otherwise */
 29     bool CheckHeadersAreContinuous(const std::vector<CBlockHeader>& headers) const;
 30     /** Try to continue a low-work headers sync that has already begun.
 31@@ -633,7 +633,7 @@ private:
 32      */
 33     bool IsContinuationOfLowWorkHeadersSync(Peer& peer, CNode& pfrom,
 34             std::vector<CBlockHeader>& headers)
 35-        EXCLUSIVE_LOCKS_REQUIRED(peer.m_headers_sync_mutex, !m_headers_presync_mutex, NetEventsInterface::g_msgproc_mutex);
 36+        EXCLUSIVE_LOCKS_REQUIRED(peer.m_headers_sync_mutex, !m_headers_presync_mutex, g_msgproc_mutex);
 37     /** Check work on a headers chain to be processed, and if insufficient,
 38      * initiate our anti-DoS headers sync mechanism.
 39      *
 40@@ -649,7 +649,7 @@ private:
 41     bool TryLowWorkHeadersSync(Peer& peer, CNode& pfrom,
 42                                   const CBlockIndex* chain_start_header,
 43                                   std::vector<CBlockHeader>& headers)
 44-        EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex, NetEventsInterface::g_msgproc_mutex);
 45+        EXCLUSIVE_LOCKS_REQUIRED(!peer.m_headers_sync_mutex, !m_peer_mutex, !m_headers_presync_mutex, g_msgproc_mutex);
 46 
 47     /** Return true if the given header is an ancestor of
 48      *  m_chainman.m_best_header or our current tip */
 49@@ -659,7 +659,7 @@ private:
 50      * We don't issue a getheaders message if we have a recent one outstanding.
 51      * This returns true if a getheaders is actually sent, and false otherwise.
 52      */
 53-    bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 54+    bool MaybeSendGetHeaders(CNode& pfrom, const CBlockLocator& locator, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 55     /** Potentially fetch blocks from this peer upon receipt of a new headers tip */
 56     void HeadersDirectFetchBlocks(CNode& pfrom, const Peer& peer, const CBlockIndex* pindexLast);
 57     /** Update peer state based on received headers message */
 58@@ -683,10 +683,10 @@ private:
 59     void MaybeSendPing(CNode& node_to, Peer& peer, std::chrono::microseconds now);
 60 
 61     /** Send `addr` messages on a regular schedule. */
 62-    void MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 63+    void MaybeSendAddr(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 64 
 65     /** Send a single `sendheaders` message, after we have completed headers sync with a peer. */
 66-    void MaybeSendSendHeaders(CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 67+    void MaybeSendSendHeaders(CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 68 
 69     /** Relay (gossip) an address to a few randomly chosen nodes.
 70      *
 71@@ -695,10 +695,10 @@ private:
 72      * [@param](/bitcoin-bitcoin/contributor/param/)[in] fReachable   Whether the address' network is reachable. We relay unreachable
 73      *                         addresses less.
 74      */
 75-    void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, NetEventsInterface::g_msgproc_mutex);
 76+    void RelayAddress(NodeId originator, const CAddress& addr, bool fReachable) EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, g_msgproc_mutex);
 77 
 78     /** Send `feefilter` message. */
 79-    void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 80+    void MaybeSendFeefilter(CNode& node, Peer& peer, std::chrono::microseconds current_time) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 81 
 82     const CChainParams& m_chainparams;
 83     CConnman& m_connman;
 84@@ -1010,7 +1010,7 @@ private:
 85      * [@return](/bitcoin-bitcoin/contributor/return/)   True if address relay is enabled with peer
 86      *            False if address relay is disallowed
 87      */
 88-    bool SetupAddressRelay(const CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex);
 89+    bool SetupAddressRelay(const CNode& node, Peer& peer) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex);
 90 };
 91 
 92 const CNodeState* PeerManagerImpl::State(NodeId pnode) const EXCLUSIVE_LOCKS_REQUIRED(cs_main)
 93@@ -5099,7 +5099,7 @@ void PeerManagerImpl::MaybeSendAddr(CNode& node, Peer& peer, std::chrono::micros
 94 
 95     // Remove addr records that the peer already knows about, and add new
 96     // addrs to the m_addr_known filter on the same pass.
 97-    auto addr_already_known = [&peer](const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(NetEventsInterface::g_msgproc_mutex) {
 98+    auto addr_already_known = [&peer](const CAddress& addr) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) {
 99         bool ret = peer.m_addr_known->contains(addr.GetKey());
100         if (!ret) peer.m_addr_known->insert(addr.GetKey());
101         return ret;

ajtowns commented at 4:55 am on September 15, 2022:

Ah, didn’t cross my mind that it wasn’t necessary here. m_last_block_inv_triggering_headers_sync also doesn’t need the class name. I’ve left it in the class since that class essentially defines “message processing” so it seems more appropriate than not.

MarcoFalke commented at 7:37 am on September 15, 2022:

g_msgproc_mutex already clarifies that this is for message processing, so still think the namespace/class can be dropped

vasild commented at 9:00 am on September 15, 2022:

Then, should it be GlobalMutex? I had some plans to eradicate that one.

ajtowns commented at 10:49 am on September 15, 2022:

If it were a normal global rather than a static member of the class it would need to be GlobalMutex (otherwise you get an error that CConnman::ThreadMessageHandler() needs to be annotated with EXCLUSIVE_LOCKS_REQUIRED(!g_msgproc_mutex), and probably similar in the tests). With the lock being a class member, clang assumes functions outside of the class aren’t called with the lock held.

vasild commented at 12:34 pm on September 15, 2022:

Yes, I mean that if “the namespace/class is dropped” then it becomes normal global and thus its type must be changed.

MarcoFalke approved

MarcoFalke commented at 11:04 am on September 14, 2022: member

review ACK 22e7c464db9719ebc8eba0143f5a6024aac658c3 🍡

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3review ACK 22e7c464db9719ebc8eba0143f5a6024aac658c3 🍡
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUhiGQwAkEL9Sna2ocIewx1Haykhnz0jJc7HCkseZAGV6fG6B7XaIvRJzv72iDgu
 8cxRU8pnGwa0GNai6aL6oGVJN61ic6UmAtHjn13T+hLTngOVJKq5G9QfuIAaafyRZ
 9Mv9zQ1n+qbydjD4uBrkkelQc65lX1wASK1q1h4S2iZR1q4oree5rQr9QpOQ7APSX
10hfpqRrkYXkLKdlJ89aeNYHaJ6KFtT0b2oJsCCQpkU/Ou3ywluqnN+7QnkNbIDQmu
11DQl0gTrImpldkLsgqmSE4CDtcy7Oopx0AbbDyn3ClKj2wOnRXAaa0QUmtcYdACF0
123Wv88lr3mEr+SReN7E5CaijpQELQG2wAbHykl1FQ8kPQ8oQkCaA0gA6D4VL5Ol85
13MOAU3gr2mazuo1KKF+tT9HAIb5pFJBzFcFIHpQpKijVWVp6wAqQubQKQOd/qf2Pr
14p7r3kqlbXurQIaEn4KpxbYJ7hg2F5X/q+RdS2vgtPr3dC2MLsEEB4ZqjpPe7MiyH
15YQJlcdGU
16=OIUy
17-----END PGP SIGNATURE-----

net: add NetEventsInterface::g_msgproc_mutex

There are many cases where we assume message processing is
single-threaded in order for how we access node-related memory to be
safe. Add an explicit mutex that we can use to document this, which allows
the compiler to catch any cases where we try to access that memory from
other threads and break that assumption.

1e78f566d5

net: drop cs_sendProcessing

SendMessages() is now protected g_msgproc_mutex; so this additional
per-node mutex is redundant.

bf12abe454

ajtowns force-pushed on Sep 15, 2022

MarcoFalke commented at 7:37 am on September 15, 2022: member

review ACK 60682dc1e506f01446ba54b4c41c8f32a9459235 🤞

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3review ACK 60682dc1e506f01446ba54b4c41c8f32a9459235 🤞
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUgEZgv/f+A8D5L+blHinz1hvX6Lx161mNRj4EjTkO8G6z8T7FAWLAlFjkRUD/px
 8pfjkbSjby0mCk+Qy56q9TnHNeNCvVtl3Qp4bIoQBzG2KfUtkvGHR2c+KsM3XW6F9
 9gaZfzhQBQsieSluEt0PtJMx9jo4zDEUEIcHg2QRtPaPIf631UtO6MeIRSnwr0V3K
102+A706TTlgJgKP9lFyAM0KjhKtQ61CVY0Drh0C7wuErTP+Pb+WiQ8jDNM5UfGjvi
11lF/VqYJXgvAgT/Y6F7ft8BMo39zrlBK9k+/i1KaUOgysp9/TX1e17BWxfFFnBEsh
12hbXcL6bYr5qqw9CaWH3qNWHZV9tTGgdLcypdD2gIAiMaLgZeu05YUOk8pXyp3/0o
13sGLI3ih7yQ3Hy1S5e0+hDvU+drVgVndEyaxyCZtSn4u+4e9UWl9YVnncfdXDTwXy
14KZ1i5SKJbp+pZ3JWBQmJK6HTalIBOC4TDcs/930ym/CChXmVqv4bMAhS1Mkzp8tB
15PGnTV0Eb
16=AnQA
17-----END PGP SIGNATURE-----

in src/net_processing.cpp:754 in 60682dc1e5 outdated

750@@ -751,7 +751,7 @@ class PeerManagerImpl final : public PeerManager
751     int nSyncStarted GUARDED_BY(cs_main) = 0;
752 
753     /** Hash of the last block we received via INV */
754-    uint256 m_last_block_inv_triggering_headers_sync{};
755+    uint256 m_last_block_inv_triggering_headers_sync GUARDED_BY(g_msgproc_mutex){};

vasild commented at 9:40 am on September 15, 2022:

In the previous incarnation of this PR the commit net_processing: add thread safety annotations for PeerManagerImpl members accessed only via the msgproc thread contained just this, 1-line, hunk.

Now it contains many hunks that change NetEventsInterface::g_msgproc_mutex to g_msgproc_mutex. Better to use g_msgproc_mutex initially and not have to modify it in a subsequent commit.

ajtowns commented at 10:45 am on September 15, 2022:

Ugh, yes. Fixed

net_processing: add thread safety annotations for Peer members accessed only via the msgproc thread a66a7ccb82

net_processing: add thread safety annotations for PeerManagerImpl members accessed only via the msgproc thread 0ae7987f68

net_processing: add thread safety annotation for m_highest_fast_announce d575a675cc

ajtowns force-pushed on Sep 15, 2022

w0xlt approved

w0xlt commented at 10:56 am on September 15, 2022: contributor

ACK https://github.com/bitcoin/bitcoin/pull/26036/commits/d575a675cc884b1bebdb6197f2ef45c51788d4a3

vasild approved

vasild commented at 12:56 pm on September 15, 2022: contributor

ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 modulo the missing runtime checks

It would be really nice not to violate the developer notes, or at least if we occasionally do, to be for a better reason than given in #26036 (review).

MarcoFalke commented at 7:49 pm on September 15, 2022: member

review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 📽

Signature:

 0-----BEGIN PGP SIGNED MESSAGE-----
 1Hash: SHA512
 2
 3review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3 📽
 4-----BEGIN PGP SIGNATURE-----
 5
 6iQGzBAEBCgAdFiEE+rVPoUahrI9sLGYTzit1aX5ppUgFAlwqrYAACgkQzit1aX5p
 7pUjnCgv/VwIJXtqBslTMcneH9mic4H9oVu0GmVcAskWGZcUNOMGAFHhtrD8a08IF
 8VRj9QOBUlvJHKo13NpDbctT7GOOQPTuDV0TS7ixote1+yjwknsGpB1MpVgCu1s8M
 9su/9Xx5zdikfQoybb7qbjcYrtaOUQYfoxSsFIGJoiKMTZ1H0D6GjS4D0/XORsMIL
10Lk7UNeLByR5Kq/otSHUEdzu42ToZTGEFEPUvOoDefNdFdbbB9KLT59NYSbYiBJLD
1139C8VPYuu+M8q2i1PaC4Zr9l7ufFvQv09lw9TBFN+ztAz0gnc6sm+kr2b7/yHkDq
12MmDflnhyaMV8XYehtz/SbWiS2QAcNc96ewy5M4pL1V6gNHytzr75qk9oBWpKPSMM
13am2/TiSf+EnKwsU9Quj2XbzYhBweFk0xNpXbAYHeFf7WEH0E/vb/5PudbelwmNRP
14AzaXYbyh4qR/uKyaJ9wlCFMcr0toWPb7koqHOTba1Zq2Kb+JyWRI6cWkB3BZX2EM
15EZQHT3Z2
16=X5uj
17-----END PGP SIGNATURE-----

dergoegge commented at 12:04 pm on September 20, 2022: member

Code review ACK d575a675cc884b1bebdb6197f2ef45c51788d4a3

There are a couple CNodeState members that could also be annotated as guarded by g_msgproc_mutex, see the last couple commits here: https://github.com/dergoegge/bitcoin/commits/202209-msgproc-mutex . Feel free to cherry pick them otherwise i can also open a follow up. (Those CNodeState members should probably move to Peer aswell)

fanquake merged this on Sep 20, 2022

fanquake closed this on Sep 20, 2022

sidhujag referenced this in commit b099ece0a8 on Sep 20, 2022

dergoegge commented at 2:26 pm on September 20, 2022: member

Opened a follow-up (#26140) that includes what i mentioned in my previous comment.

bitcoin locked this on Sep 20, 2023

net: add NetEventsInterface::g_msgproc_mutex #26036

Conflicts