[net] Thread safety annotations in net

skeees commented at 6:12 pm on June 8, 2018: contributor

(note that this depends on #13417) This commit fully annotates all globals in net_processing with clang thread safety annotations. Subsequent commits will begin transitioning some of this data away from cs_main into locks that are local to net_processing. Static thread safety analysis should it easier to verify correctness of that process.

skeees force-pushed on Jun 8, 2018

fanquake added the label P2P on Jun 9, 2018

fanquake added the label Refactoring on Jun 9, 2018

skeees force-pushed on Jun 9, 2018

skeees force-pushed on Jun 10, 2018

promag commented at 9:12 pm on June 10, 2018: member

Concept ACK.

Well done in 4c0b978, improves thread safety analysis in a lot of places.

DrahtBot added the label Needs rebase on Jun 12, 2018

jnewbery commented at 5:11 pm on June 14, 2018: member

utACK 00c1bae9091447229cd09b5146ee25a357f592fe (although I’m not an expert on cs_main)

skeees force-pushed on Jun 19, 2018

skeees commented at 5:29 pm on June 19, 2018: contributor

@DrahtBot - rebased

DrahtBot removed the label Needs rebase on Jun 19, 2018

DrahtBot commented at 10:46 pm on June 19, 2018: member

#13770 (Use explicit captures in lambda expressions by practicalswift)
#11640 (Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection by ryanofsky)
#11599 (scripted-diff: Small locking rename by ryanofsky)

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.

jnewbery commented at 5:58 pm on June 21, 2018: member

reACK af288e3f09f0724390334d9902742883a9db388e

DrahtBot added the label Needs rebase on Jul 14, 2018

Annotate AssertLockHeld() with ASSERT_CAPABILITY() for thread safety analysis f393a533be

skeees force-pushed on Jul 25, 2018

fanquake removed the label Needs rebase on Jul 25, 2018

skeees force-pushed on Jul 25, 2018

skeees commented at 5:47 pm on July 25, 2018: contributor

rebased now that #13417 has been merged

MarcoFalke added the label Needs rebase on Jul 25, 2018

in src/net_processing.cpp:157 in 6a2e1ee787 outdated

157+    int g_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0;
158 
159     /** When our tip was last updated. */
160     std::atomic<int64_t> g_last_tip_update(0);
161 
162     /** Relay map, protected by cs_main. */

MarcoFalke commented at 6:09 pm on July 25, 2018:

Can remove the now redundant mention of cs_main here?

in src/net_processing.cpp:160 in 6a2e1ee787 outdated

161 
162     /** Relay map, protected by cs_main. */
163     typedef std::map<uint256, CTransactionRef> MapRelay;
164-    MapRelay mapRelay;
165+    MapRelay mapRelay GUARDED_BY(cs_main);
166     /** Expiration-time ordered list of (expire time, relay map entry) pairs, protected by cs_main). */

MarcoFalke commented at 6:09 pm on July 25, 2018:

Same here (and everywhere else)

DrahtBot removed the label Needs rebase on Jul 25, 2018

[net_processing] Add thread safety annotations 1e3bcd2517

skeees force-pushed on Jul 25, 2018

skeees commented at 7:25 pm on July 25, 2018: contributor

@MarcoFalke - done

sipa commented at 9:02 pm on July 25, 2018: member

utACK 1e3bcd251768baeb95e555d51d2dc787a6b2acee

promag commented at 5:06 pm on July 26, 2018: member

utACK 1e3bcd2.

laanwj commented at 5:13 pm on July 26, 2018: member

utACK 1e3bcd251768baeb95e555d51d2dc787a6b2acee

laanwj merged this on Jul 26, 2018

laanwj closed this on Jul 26, 2018

laanwj referenced this in commit f58674a20a on Jul 26, 2018

in src/net_processing.cpp:79 in 1e3bcd2517

75@@ -76,7 +76,7 @@ std::map<uint256, COrphanTx> mapOrphanTransactions GUARDED_BY(g_cs_orphans);
76 void EraseOrphansFor(NodeId peer);
77 
78 /** Increase a node's misbehavior score. */
79-void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="");
80+void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main);

MarcoFalke commented at 5:35 pm on July 26, 2018:

nit: should prefix this with static to make sure the annotation is properly added where this function is declared.

skeees commented at 5:48 pm on July 26, 2018:

sorry - don’t quite understand - this is also used in src/test/denialofservice_tests.cpp wouldn’t declaring static break that?

MarcoFalke commented at 5:51 pm on July 26, 2018:

Ah, I see. Thanks for the clarification. I only built bitcoind to check my patch.

PastaPastaPasta referenced this in commit 614a0bbd78 on Apr 15, 2020

PastaPastaPasta referenced this in commit 76d2e73d79 on Apr 16, 2020

PastaPastaPasta referenced this in commit b4d0e1149f on Apr 16, 2020

PastaPastaPasta referenced this in commit 0fab32de36 on Apr 19, 2020

PastaPastaPasta referenced this in commit c7f131cb64 on Apr 20, 2020

PastaPastaPasta referenced this in commit aeff62a7ad on May 10, 2020

PastaPastaPasta referenced this in commit 10d9608654 on May 12, 2020

PastaPastaPasta referenced this in commit 19ac12e516 on Jun 9, 2020

PastaPastaPasta referenced this in commit 0a2fb48943 on Jun 9, 2020

PastaPastaPasta referenced this in commit bcb03327d1 on Jun 10, 2020

PastaPastaPasta referenced this in commit 646b57c07b on Jun 11, 2020

LarryRuane referenced this in commit b8e1c93deb on Mar 26, 2021

LarryRuane referenced this in commit 33243cda37 on Mar 26, 2021

LarryRuane referenced this in commit 1606c615fb on Apr 26, 2021

LarryRuane referenced this in commit b03729d737 on Apr 26, 2021

LarryRuane referenced this in commit 61f63df0bf on May 27, 2021

LarryRuane referenced this in commit b23bc94f47 on May 27, 2021

str4d referenced this in commit 9cdf39e961 on Sep 23, 2021

str4d referenced this in commit 9121ef4ad3 on Sep 23, 2021

DrahtBot locked this on Feb 15, 2022

[net] Thread safety annotations in net_processing #13423