Add missing locks: validation.cpp + related #11652

pull practicalswift wants to merge 3 commits into bitcoin:master from practicalswift:init-and-validation-locks changing 15 files +127 −63
  1. practicalswift commented at 9:38 AM on November 10, 2017: contributor

    Add missing locks required when accessing:

    int32_t nBlockSequenceId GUARDED_BY(cs_nBlockSequenceId) = 1;
int nLastBlockFile GUARDED_BY(cs_LastBlockFile) = 0;
bool fCheckForPruning GUARDED_BY(cs_LastBlockFile) = false;
CuckooCache::cache<uint256, SignatureCacheHasher> scriptExecutionCache GUARDED_BY(cs_main);
BlockMap& mapBlockIndex GUARDED_BY(cs_main);
std::unique_ptr<CCoinsViewDB> pcoinsdbview GUARDED_BY(cs_main);
std::unique_ptr<CCoinsViewCache> pcoinsTip GUARDED_BY(cs_main);
std::unique_ptr<CBlockTreeDB> pblocktree GUARDED_BY(cs_main);

    Also, add the locking annotations that follow from the requirements above.

  2. practicalswift force-pushed on Nov 10, 2017
  3. fanquake added the label Refactoring on Nov 10, 2017
  4. practicalswift commented at 7:40 AM on November 22, 2017: contributor

    Anyone willing to review? :-)

  5. practicalswift force-pushed on Dec 13, 2017
  6. practicalswift force-pushed on Dec 21, 2017
  7. in src/validation.cpp:3851 in 255b5c5d4d outdated 
    3662 | @@ -3656,8 +3663,8 @@ CBlockIndex * CChainState::InsertBlockIndex(const uint256& hash)
3663 |  
3664 |  bool CChainState::LoadBlockIndex(const Consensus::Params& consensus_params, CBlockTreeDB& blocktree)
3665 |  {
3666 | +    LOCK(cs_main);
3667 |      if (!blocktree.LoadBlockIndexGuts(consensus_params, [this](const uint256& hash){ return this->InsertBlockIndex(hash); }))
3668 | -        return false;
3669 |
    ajtowns commented at 5:33 AM on January 11, 2018:

    Dropping the "return false" doesn't seem right...

    practicalswift commented at 7:19 AM on January 11, 2018:

    Ouch, thanks! Will fix!

  8. in src/validation.cpp:4140 in 255b5c5d4d outdated 
    4123 | +        nLastBlockFile = 0;
4124 | +    }
4125 | +    {
4126 | +        LOCK(cs_nBlockSequenceId);
4127 | +        nBlockSequenceId = 1;
4128 | +    }
    ajtowns commented at 5:54 AM on January 11, 2018:

    This has moved into CChainState::UnloadBlockIndex.

  9. ajtowns changes_requested
  10. TheBlueMatt commented at 5:44 PM on January 11, 2018: member

    Please just put a cs_main at the top of the load-from disk loop thing, there is no reason to lock and unlock cs_main 20 times during single-threaded init.

  11. practicalswift force-pushed on Jan 11, 2018
  12. practicalswift force-pushed on Jan 11, 2018
  13. practicalswift commented at 9:14 PM on January 11, 2018: contributor

    @ajtowns @TheBlueMatt Thanks for reviewing! I've now addressed your feedback. Would you mind re-reviewing? :-)

  14. MarcoFalke commented at 11:20 PM on January 12, 2018: member

    General note: Instead of documenting the locking assumptions in the OP of this pull request, wouldn't it be better to put them in the header files? Imo, this increases review efficiency.

  15. practicalswift commented at 11:21 PM on March 12, 2018: contributor

    @MarcoFalke You mean as an interim before we've added GUARDED_BY(…) annotations for each guarded variable?

  16. MarcoFalke commented at 11:40 PM on March 12, 2018: member

    No, I meant adding the GUARDED_BY to header files whenever possible

  17. practicalswift force-pushed on Mar 14, 2018
  18. practicalswift force-pushed on Mar 14, 2018
  19. practicalswift renamed this:
    Add missing locks to init.cpp (in AppInitMain + ThreadImport) and validation.cpp
    Add missing locks: validation.cpp + related
    on Mar 14, 2018
  20. practicalswift force-pushed on Mar 14, 2018
  21. practicalswift force-pushed on Mar 14, 2018
  22. practicalswift commented at 6:16 PM on March 14, 2018: contributor

    @MarcoFalke Sure! New version with GUARDED_BY + the implied EXCLUSIVE_LOCKS_REQUIRED :-)

    Could you please review? :-)

  23. practicalswift force-pushed on Mar 20, 2018
  24. practicalswift force-pushed on Mar 23, 2018
  25. practicalswift commented at 6:16 PM on March 25, 2018: contributor

    Hrmm, the i686-w64-mingw32 build seems to timeout for some reason.

  26. ryanofsky commented at 1:16 AM on March 27, 2018: member

    Hrmm, the i686-w64-mingw32 build seems to timeout for some reason.

    Should be fixed by rebasing after #12772

  27. practicalswift force-pushed on Mar 27, 2018
  28. practicalswift commented at 11:09 AM on March 27, 2018: contributor

    Rebased!

  29. ajtowns commented at 1:13 AM on March 28, 2018: member

    9bd9d9f8302d4cdde2f25f4d7a008f8f0a0ccea2 looks good to me, fwiw, but it needs another rebase anyway to cope with scoped enums.

  30. practicalswift force-pushed on Mar 28, 2018
  31. practicalswift commented at 2:36 AM on March 28, 2018: contributor

    Rebased! @ajtowns Thanks for the review. Please re-review :-)

  32. practicalswift force-pushed on Apr 3, 2018
  33. practicalswift commented at 11:37 AM on April 3, 2018: contributor

    Rebased! :-)

  34. practicalswift force-pushed on Apr 6, 2018
  35. practicalswift force-pushed on Apr 9, 2018
  36. practicalswift force-pushed on Apr 9, 2018
  37. practicalswift commented at 1:38 PM on April 9, 2018: contributor

    Rebased!

  38. practicalswift force-pushed on Apr 9, 2018
  39. in src/rpc/blockchain.cpp:998 in 220db649a5 outdated 
     963 | @@ -964,6 +964,7 @@ UniValue gettxoutsetinfo(const JSONRPCRequest& request)
 964 |  
 965 |      CCoinsStats stats;
 966 |      FlushStateToDisk();
 967 | +    LOCK(cs_main);
    TheBlueMatt commented at 5:36 PM on April 17, 2018:

    This is incorrect.

    practicalswift commented at 8:06 AM on April 23, 2018:

    Oh, thanks. Switching to UniValue gettxoutsetinfo(const JSONRPCRequest& request) EXCLUSIVE_LOCKS_REQUIRED(cs_main) instead.

    practicalswift commented at 11:25 PM on July 11, 2018:

    @TheBlueMatt I’ve now reverted to the original version after @MarcoFalke’s comment in #11652 (review). Could you two help me resolve this? Why is it incorrect? Doesn’t reading the variable pcoinsdbview require holding the mutex cs_main?

  40. in src/validation.cpp:1337 in 220db649a5 outdated 
    1334 |      // nMaxCacheSize is unsigned. If -maxsigcachesize is set to zero,
1335 |      // setup_bytes creates the minimum possible cache (2 elements).
1336 |      size_t nMaxCacheSize = std::min(std::max((int64_t)0, gArgs.GetArg("-maxsigcachesize", DEFAULT_MAX_SIG_CACHE_SIZE) / 2), MAX_MAX_SIG_CACHE_SIZE) * ((size_t) 1 << 20);
1337 | -    size_t nElems = scriptExecutionCache.setup_bytes(nMaxCacheSize);
1338 | +    size_t nElems;
1339 | +    {
    TheBlueMatt commented at 5:37 PM on April 17, 2018:

    You dont have to add a new scope for stuff like this - holding cs_main through an extra line of LogPrintf during init is perfectly fine.

    practicalswift commented at 8:07 AM on April 23, 2018:

    Fixed!

  41. in src/validation.cpp:4211 in 220db649a5 outdated 
    4207 | @@ -4198,7 +4208,10 @@ void UnloadBlockIndex()
4208 |      mempool.clear();
4209 |      mapBlocksUnlinked.clear();
4210 |      vinfoBlockFile.clear();
4211 | -    nLastBlockFile = 0;
4212 | +    {
    TheBlueMatt commented at 5:41 PM on April 17, 2018:

    Why the extra scope here? Its during init/deinit so everything is single-threaded anyway.

    practicalswift commented at 9:00 AM on April 23, 2018:

    Fixed!

  42. in src/validation.h:433 in 220db649a5 outdated 
     429 | @@ -430,21 +430,21 @@ class CVerifyDB {
 430 |  /** Replay blocks that aren't fully applied to the database. */
 431 |  bool ReplayBlocks(const CChainParams& params, CCoinsView* view);
 432 |  
 433 | -inline CBlockIndex* LookupBlockIndex(const uint256& hash)
 434 | +inline CBlockIndex* LookupBlockIndex(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
    TheBlueMatt commented at 5:44 PM on April 17, 2018:

    Can we just take cs_main inside instead of requiring it? Seems like that would massively reduce the diff size here.

    practicalswift commented at 9:01 AM on April 23, 2018:

    Oh, very good point! Now fixed!

    promag commented at 10:01 AM on April 23, 2018:

    Don't we want to avoid recursive locks?

  43. practicalswift force-pushed on Apr 23, 2018
  44. practicalswift force-pushed on Apr 23, 2018
  45. practicalswift commented at 9:02 AM on April 23, 2018: contributor

    Thanks to @TheBlueMatt's insightful review I've now managed to reduce the size of this PR significantly (by having LookupBlockIndex perform cs_main locking). Thanks!

    Please re-review :-)

  46. practicalswift force-pushed on Apr 26, 2018
  47. practicalswift commented at 4:30 AM on April 26, 2018: contributor

    Rebased!

  48. practicalswift force-pushed on May 4, 2018
  49. practicalswift commented at 2:14 PM on May 4, 2018: contributor

    Rebased!

  50. practicalswift force-pushed on May 5, 2018
  51. in src/validation.cpp:359 in 781c3b0ed7 outdated 
     355 | @@ -356,7 +356,7 @@ bool TestLockPointValidity(const LockPoints* lp)
 356 |      return true;
 357 |  }
 358 |  
 359 | -bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool useExistingLockPoints)
 360 | +bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp, bool useExistingLockPoints) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
    MarcoFalke commented at 11:44 PM on May 29, 2018:

    Should be in the header file. See #13083

  52. practicalswift force-pushed on May 30, 2018
  53. practicalswift commented at 3:37 AM on May 30, 2018: contributor

    @MarcoFalke Thanks for reviewing! Now moved to the header file. Please re-review :-)

  54. DrahtBot added the label Needs rebase on Jun 7, 2018
  55. practicalswift force-pushed on Jun 7, 2018
  56. practicalswift commented at 5:48 PM on June 7, 2018: contributor

    Rebased! Please re-review :-)

  57. DrahtBot removed the label Needs rebase on Jun 7, 2018
  58. DrahtBot added the label Needs rebase on Jul 9, 2018
  59. rickblackjr commented at 11:32 PM on July 9, 2018: none

    What is this and how to stop getting these constant notifications?

    Sent via the Samsung GALAXY S® 5, an AT&T 4G LTE smartphone

    -------- Original message -------- From: DrahtBot notifications@github.com Date: 7/9/18 19:29 (GMT-05:00) To: bitcoin/bitcoin bitcoin@noreply.github.com Cc: Subscribed subscribed@noreply.github.com Subject: Re: [bitcoin/bitcoin] Add missing locks: validation.cpp + related (#11652)

    Needs rebase

    — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHubhttps://github.com/bitcoin/bitcoin/pull/11652#issuecomment-403651158, or mute the threadhttps://github.com/notifications/unsubscribe-auth/AmaOR_mP_fTsER1n5gCk1geOakOHbxsjks5uE-dcgaJpZM4QZTJU.

  60. practicalswift force-pushed on Jul 10, 2018
  61. practicalswift commented at 4:42 PM on July 10, 2018: contributor

    Rebased! Please re-review :-)

  62. DrahtBot removed the label Needs rebase on Jul 10, 2018
  63. in src/rpc/blockchain.cpp:953 in eeb5e8e443 outdated 
     949 | @@ -953,7 +950,7 @@ static UniValue pruneblockchain(const JSONRPCRequest& request)
 950 |      return uint64_t(height);
 951 |  }
 952 |  
 953 | -static UniValue gettxoutsetinfo(const JSONRPCRequest& request)
 954 | +static UniValue gettxoutsetinfo(const JSONRPCRequest& request) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
    MarcoFalke commented at 4:47 PM on July 10, 2018:

    This is incorrect and ignored by clang anyway

    practicalswift commented at 8:18 PM on July 11, 2018:

    @MarcoFalke In what way? Doesn’t reading the variable pcoinsdbview require holding the mutex cs_main?

    MarcoFalke commented at 11:02 PM on July 11, 2018:

    All the rpc calls are called through function pointers, which breaks clangs static analyser. Clearly there is no way to acquire this lock in the caller of gettxoutsetinfo. If the lock is missing, it should be added...

  64. in src/wallet/wallet.h:42 in eeb5e8e443 outdated 
      37 | @@ -38,6 +38,8 @@ bool HasWallets();
  38 |  std::vector<std::shared_ptr<CWallet>> GetWallets();
  39 |  std::shared_ptr<CWallet> GetWallet(const std::string& name);
  40 |  
  41 | +extern CCriticalSection cs_main;
  42 | +
    MarcoFalke commented at 4:48 PM on July 10, 2018:

    Unrelated addition?

  65. in src/validation.cpp:3370 in eeb5e8e443 outdated 
    3366 | @@ -3362,7 +3367,7 @@ static bool ContextualCheckBlock(const CBlock& block, CValidationState& state, c
3367 |      return true;
3368 |  }
3369 |  
3370 | -bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex)
3371 | +bool CChainState::AcceptBlockHeader(const CBlockHeader& block, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
    MarcoFalke commented at 4:50 PM on July 10, 2018:

    Those annotations are ignored by clang if they are at the implementation, please add them to where the function signature is defined.

  66. in src/validation.cpp:3472 in eeb5e8e443 outdated 
    3468 | @@ -3464,7 +3469,7 @@ static CDiskBlockPos SaveBlockToDisk(const CBlock& block, int nHeight, const CCh
3469 |  }
3470 |  
3471 |  /** Store block on disk. If dbp is non-nullptr, the file is known to already reside on disk */
3472 | -bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock)
3473 | +bool CChainState::AcceptBlock(const std::shared_ptr<const CBlock>& pblock, CValidationState& state, const CChainParams& chainparams, CBlockIndex** ppindex, bool fRequested, const CDiskBlockPos* dbp, bool* fNewBlock) EXCLUSIVE_LOCKS_REQUIRED(cs_main)
    MarcoFalke commented at 4:50 PM on July 10, 2018:

    Same here. (this comment holds for all other instances as well)

    MarcoFalke commented at 5:07 PM on July 10, 2018:

    Probably best to fix those in #13083 as well

  67. MarcoFalke added the label Needs rebase on Jul 10, 2018
  68. DrahtBot removed the label Needs rebase on Jul 10, 2018
  69. practicalswift force-pushed on Jul 11, 2018
  70. practicalswift force-pushed on Jul 11, 2018
  71. practicalswift force-pushed on Jul 11, 2018
  72. practicalswift force-pushed on Jul 11, 2018
  73. practicalswift force-pushed on Jul 11, 2018
  74. practicalswift commented at 11:14 PM on July 11, 2018: contributor

    @MarcoFalke Thanks for reviewing! Feedback addressed. Let me know if it looks good now. I’ll implement the validation.cpp changes into #13083 when the changes introduced here has been reviewed.

  75. practicalswift force-pushed on Jul 11, 2018
  76. in src/validation.h:455 in 43c9703998 outdated 
     468 | @@ -469,13 +469,13 @@ bool ResetBlockFailureFlags(CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_mai
 469 |  extern CChain& chainActive;
    MarcoFalke commented at 1:49 PM on July 14, 2018:

    Missing GUARDED_BY(cs_main).

    practicalswift commented at 10:31 PM on August 27, 2018:

    @MarcoFalke Now fixed. See separate commit.

    promag commented at 11:25 PM on August 27, 2018:

    Missing :-) in the above comment.

  77. in src/validation.h:471 in 43c9703998 outdated 
     479 |  /** Global variable that points to the active block tree (protected by cs_main) */
 480 | -extern std::unique_ptr<CBlockTreeDB> pblocktree;
 481 | +extern std::unique_ptr<CBlockTreeDB> pblocktree GUARDED_BY(cs_main);
 482 |  
 483 |  /**
 484 |   * Return the spend height, which is one more than the inputs.GetBestBlock().
    MarcoFalke commented at 1:49 PM on July 14, 2018:

    Missing annotation for this function

    practicalswift commented at 10:30 PM on August 27, 2018:

    @MarcoFalke Do you mean GetSpendHeight? GetSpendHeight locks cs_main itself :-)

  78. in src/validation.h:350 in 43c9703998 outdated 
     360 | @@ -361,7 +361,7 @@ bool TestLockPointValidity(const LockPoints* lp);
 361 |   *
 362 |   * See consensus/consensus.h for flag definitions.
 363 |   */
 364 | -bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false);
 365 | +bool CheckSequenceLocks(const CTransaction &tx, int flags, LockPoints* lp = nullptr, bool useExistingLockPoints = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
    MarcoFalke commented at 1:10 PM on July 18, 2018:

    Also requires ::mempool.cs to be held, no?

    practicalswift commented at 10:32 PM on August 27, 2018:

    @MarcoFalke Due to what access? :-)

  79. DrahtBot added the label Needs rebase on Jul 22, 2018
  80. practicalswift force-pushed on Aug 15, 2018
  81. DrahtBot removed the label Needs rebase on Aug 15, 2018
  82. DrahtBot added the label Needs rebase on Aug 25, 2018
  83. practicalswift force-pushed on Aug 25, 2018
  84. DrahtBot removed the label Needs rebase on Aug 26, 2018
  85. DrahtBot added the label Needs rebase on Aug 27, 2018
  86. practicalswift force-pushed on Aug 27, 2018
  87. DrahtBot removed the label Needs rebase on Aug 27, 2018
  88. practicalswift commented at 10:33 PM on August 27, 2018: contributor

    @MarcoFalke Pushed an updated version. See the latest commit which adds extern CChain& chainActive GUARDED_BY(cs_main) (validation.h) plus required locking. Please re-review :-)

  89. practicalswift force-pushed on Aug 28, 2018
  90. practicalswift force-pushed on Aug 30, 2018
  91. in src/validation.h:433 in 78533020e8 outdated 
     429 | @@ -430,7 +430,7 @@ bool ReplayBlocks(const CChainParams& params, CCoinsView* view);
 430 |  
 431 |  inline CBlockIndex* LookupBlockIndex(const uint256& hash)
 432 |  {
 433 | -    AssertLockHeld(cs_main);
 434 | +    LOCK(cs_main);
    MarcoFalke commented at 8:20 PM on September 20, 2018:

    Why this change?

  92. DrahtBot commented at 1:29 PM on September 21, 2018: member

    <!--e57a25ab6845829454e8d69fc972939a-->

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

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #14711 (Remove uses of chainActive and mapBlockIndex in wallet code by ryanofsky)
    • #14193 (validation: Add missing mempool locks by MarcoFalke)
    • #13204 (Faster sigcache nonce by JeremyRubin)
    • #9381 (Remove CWalletTx merging logic from AddToWallet 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.

  93. practicalswift force-pushed on Oct 7, 2018
  94. practicalswift force-pushed on Oct 8, 2018
  95. practicalswift force-pushed on Oct 8, 2018
  96. practicalswift force-pushed on Oct 8, 2018
  97. practicalswift force-pushed on Oct 8, 2018
  98. practicalswift commented at 4:24 AM on October 9, 2018: contributor

    Updated! @ajtowns @TheBlueMatt @MarcoFalke @ryanofsky Please re-review :-)

  99. TheBlueMatt commented at 2:51 AM on October 10, 2018: member

    Please just put a cs_main at the top of the load-from disk loop thing, there is no reason to lock and unlock cs_main 20 times during single-threaded init.

    Looks like you never did this.

  100. practicalswift force-pushed on Oct 11, 2018
  101. practicalswift force-pushed on Oct 11, 2018
  102. practicalswift commented at 8:41 AM on October 24, 2018: contributor

    @TheBlueMatt Can you confirm that your suggestion is addressed in the latest version? :-) If not I might need help identifying the "load-from disk loop thing" :-)

  103. practicalswift commented at 11:11 PM on November 6, 2018: contributor

    @TheBlueMatt Friendly ping :-)

  104. DrahtBot added the label Needs rebase on Nov 9, 2018
  105. practicalswift closed this on Nov 12, 2018
  106. practicalswift commented at 9:31 AM on November 27, 2018: contributor

    @MarcoFalke I'll do that once my other two open locking PRs #13128 and #13123 have been processed (either merged or closed). I try to limit my work in progress in the form of open pull requests :-)

  107. MarcoFalke commented at 5:15 PM on January 10, 2019: member

    I think this can be reopened and rebased.

  108. practicalswift commented at 5:35 PM on January 10, 2019: contributor

    @MarcoFalke I force-pushed before re-opening, so now I'm unable to re-open. Could you re-open? :-)

  109. MarcoFalke commented at 5:59 PM on January 10, 2019: member

    I can't force push 7e11716932b8a1f6f1aa2c06e1990b64950a89bc to your branch either, since the pull request is closed.

    Try

    git push origin 7e11716932b8a1f6f1aa2c06e1990b64950a89bc:init-and-validation-locks -f
# reopen
git push origin init-and-validation-locks -f
  110. practicalswift reopened this on Jan 10, 2019
  111. practicalswift force-pushed on Jan 10, 2019
  112. practicalswift commented at 6:05 PM on January 10, 2019: contributor

    @MarcoFalke Thanks! Now re-opened and force-pushed. WIP for now.

  113. DrahtBot removed the label Needs rebase on Jan 10, 2019
  114. practicalswift commented at 7:12 PM on January 10, 2019: contributor

    @MarcoFalke I'm a bit unsure how to resolve the locking errors below (see Travis OS X output). Do you have any suggestion?

    If m_wallet.chain().lock(…) succeeds then we have acquired cs_main and access to chainActive should be OK?

    interfaces/wallet.cpp:335:22: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        num_blocks = ::chainActive.Height();
                     ^
interfaces/wallet.cpp:349:26: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
            num_blocks = ::chainActive.Height();
                         ^
interfaces/wallet.cpp:380:22: error: reading variable 'chainActive' requires holding mutex 'cs_main' [-Werror,-Wthread-safety-analysis]
        num_blocks = ::chainActive.Height();
                     ^
  115. MarcoFalke commented at 7:15 PM on January 10, 2019: member

    The compiler doesn't understand that, though. You'd have to add a temporary annotation or use the interface (c.f. #14711)

  116. ryanofsky commented at 7:39 PM on January 10, 2019: member

    re: #11652 (comment)

    I'm a bit unsure how to resolve the locking errors below (see Travis OS X output). Do you have any suggestion?

    You could use the LockAnnotation class to resolve these errors. See 081accb875412f38718f2e40ed7bbdf15e6f4ef8 from #14437 for examples. Rebasing on top of #14711 would also resolve them.

  117. practicalswift commented at 9:04 PM on January 10, 2019: contributor

    @MarcoFalke Would you mind taking over this PR if it put it up for grabs? At the moment I'm afraid I don't have time to resolve the remaining issues.

  118. ryanofsky commented at 9:31 PM on January 10, 2019: member

    At the moment I'm afraid I don't have time to resolve the remaining issues.

    Are there other issues besides the need for lock annotations in the wallet? If not, we could just treat this PR as blocked until #14711 is merged, since it removes the need for them.

  119. DrahtBot added the label Needs rebase on Jan 15, 2019
  120. Add missing locks in tests 5f47f07042
  121. Add missing locks f2ce4650ba
  122. Add locking annotations a04dd77fe8
  123. MarcoFalke force-pushed on Jan 15, 2019
  124. DrahtBot removed the label Needs rebase on Jan 15, 2019
  125. practicalswift closed this on Jan 17, 2019
  126. practicalswift commented at 9:33 PM on January 17, 2019: contributor

    Closing in favour of PR #15191 and #15192. They cover everything in this PR except the chainActive locks.

    I'll revisit the missing cs_main locks required for chainActive once #14711 is merged.

  127. practicalswift deleted the branch on Apr 10, 2021
  128. DrahtBot locked this on Aug 18, 2022
Contributors
practicalswiftajtownsTheBlueMattMarcoFalkeryanofskypromagrickblackjrDrahtBot
Labels
Refactoring
Linked (view graph)
#13129 Meta-issue: Add Clang thread safety analysis annotations
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-04-21 15:15 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me