[net] Thread safety annotations in net_processing #13423

pull skeees wants to merge 2 commits into bitcoin:master from skeees:net_processing-thread-annotations changing 3 files +50 −47
  1. 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.

  2. skeees force-pushed on Jun 8, 2018
  3. fanquake added the label P2P on Jun 9, 2018
  4. fanquake added the label Refactoring on Jun 9, 2018
  5. skeees force-pushed on Jun 9, 2018
  6. skeees force-pushed on Jun 9, 2018
  7. skeees force-pushed on Jun 10, 2018
  8. skeees force-pushed on Jun 10, 2018
  9. skeees force-pushed on Jun 10, 2018
  10. 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.

  11. DrahtBot added the label Needs rebase on Jun 12, 2018
  12. jnewbery commented at 5:11 PM on June 14, 2018: member

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

  13. skeees force-pushed on Jun 19, 2018
  14. skeees commented at 5:29 PM on June 19, 2018: contributor

    @DrahtBot - rebased

  15. DrahtBot removed the label Needs rebase on Jun 19, 2018
  16. DrahtBot commented at 10:46 PM on June 19, 2018: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->Note to reviewers: This pull request conflicts with the following ones:

    • #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.

  17. DrahtBot cross-referenced this on Jun 19, 2018 from issue [net,mempool] Call AcceptToMemoryPool() asynchronously in p2p by skeees
  18. DrahtBot cross-referenced this on Jun 19, 2018 from issue [refactor, move-only-ish] Refactor mempool accept/reject logic by skeees
  19. jnewbery commented at 5:58 PM on June 21, 2018: member

    reACK af288e3f09f0724390334d9902742883a9db388e

  20. DrahtBot cross-referenced this on Jun 26, 2018 from issue [net] Tighten scope in net_processing by skeees
  21. DrahtBot added the label Needs rebase on Jul 14, 2018
  22. Annotate AssertLockHeld() with ASSERT_CAPABILITY() for thread safety analysis f393a533be
  23. skeees force-pushed on Jul 25, 2018
  24. fanquake removed the label Needs rebase on Jul 25, 2018
  25. skeees force-pushed on Jul 25, 2018
  26. skeees commented at 5:47 PM on July 25, 2018: contributor

    rebased now that #13417 has been merged

  27. MarcoFalke added the label Needs rebase on Jul 25, 2018
  28. 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?

  29. 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)

  30. DrahtBot removed the label Needs rebase on Jul 25, 2018
  31. [net_processing] Add thread safety annotations 1e3bcd2517
  32. skeees force-pushed on Jul 25, 2018
  33. skeees commented at 7:25 PM on July 25, 2018: contributor

    @MarcoFalke - done

  34. sipa commented at 9:02 PM on July 25, 2018: member

    utACK 1e3bcd251768baeb95e555d51d2dc787a6b2acee

  35. DrahtBot cross-referenced this on Jul 26, 2018 from issue Use explicit captures in lambda expressions by practicalswift
  36. promag commented at 5:06 PM on July 26, 2018: member

    utACK 1e3bcd2.

  37. laanwj commented at 5:13 PM on July 26, 2018: member

    utACK 1e3bcd251768baeb95e555d51d2dc787a6b2acee

  38. laanwj merged this on Jul 26, 2018
  39. laanwj closed this on Jul 26, 2018
  40. laanwj referenced this in commit f58674a20a on Jul 26, 2018
  41. 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.

  42. ryanofsky cross-referenced this on Aug 7, 2018 from issue Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection by ryanofsky
  43. Bushstar cross-referenced this on Aug 13, 2018 from issue commits from bitcoin/master by Bushstar
  44. ryanofsky cross-referenced this on Aug 31, 2018 from issue Add AssertLockHeld assertions in CWallet::ListCoins by ryanofsky
  45. sickpig cross-referenced this on Mar 19, 2020 from issue Add missing locks and fix false positive clang warnings - part 3 by sickpig
  46. hebasto cross-referenced this on Apr 14, 2020 from issue Replace -Wthread-safety-analysis with broader -Wthread-safety by hebasto
  47. PastaPastaPasta referenced this in commit 614a0bbd78 on Apr 15, 2020
  48. PastaPastaPasta referenced this in commit 76d2e73d79 on Apr 16, 2020
  49. PastaPastaPasta referenced this in commit b4d0e1149f on Apr 16, 2020
  50. PastaPastaPasta referenced this in commit 0fab32de36 on Apr 19, 2020
  51. PastaPastaPasta referenced this in commit c7f131cb64 on Apr 20, 2020
  52. PastaPastaPasta referenced this in commit aeff62a7ad on May 10, 2020
  53. PastaPastaPasta referenced this in commit 10d9608654 on May 12, 2020
  54. PastaPastaPasta referenced this in commit 19ac12e516 on Jun 9, 2020
  55. PastaPastaPasta referenced this in commit 0a2fb48943 on Jun 9, 2020
  56. PastaPastaPasta referenced this in commit bcb03327d1 on Jun 10, 2020
  57. PastaPastaPasta referenced this in commit 646b57c07b on Jun 11, 2020
  58. ryanofsky cross-referenced this on Sep 14, 2020 from issue scripted-diff: Restore AssertLockHeld after #19668, remove LockAssertion by ryanofsky
  59. LarryRuane cross-referenced this on Mar 26, 2021 from issue Bitcoin 0.17 locking PRs by LarryRuane
  60. LarryRuane referenced this in commit b8e1c93deb on Mar 26, 2021
  61. LarryRuane referenced this in commit 33243cda37 on Mar 26, 2021
  62. str4d cross-referenced this on Apr 12, 2021 from issue Sync backports by str4d
  63. LarryRuane referenced this in commit 1606c615fb on Apr 26, 2021
  64. LarryRuane referenced this in commit b03729d737 on Apr 26, 2021
  65. volbil cross-referenced this on Apr 29, 2021 from issue Fix edge case race condition by volbil
  66. LarryRuane referenced this in commit 61f63df0bf on May 27, 2021
  67. LarryRuane referenced this in commit b23bc94f47 on May 27, 2021
  68. str4d referenced this in commit 9cdf39e961 on Sep 23, 2021
  69. str4d referenced this in commit 9121ef4ad3 on Sep 23, 2021
  70. bitcoin locked this on Feb 15, 2022
Contributors
skeeespromagjnewberyDrahtBotMarcoFalkesipalaanwj
Labels
RefactoringP2P
Linked (view graph)
#10605 Add AssertLockHeld assertions in CWallet::ListCoins#11640 Make LOCK, LOCK2, TRY_LOCK work with CWaitableCriticalSection#13417 [net] Tighten scope in net_processing#18635 Replace -Wthread-safety-analysis with broader -Wthread-safety#19865 scripted-diff: Restore AssertLockHeld after #19668, remove LockAssertion
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-05-20 06:55 UTC