Add AssertLockHeld assertions in CWallet::ListCoins #10605

pull ryanofsky wants to merge 2 commits into bitcoin:master from ryanofsky:pr/listlock changing 3 files +16 −14
  1. ryanofsky commented at 7:44 PM on June 15, 2017: contributor

    Fixes TODO from #10295

  2. fanquake added the label Wallet on Jun 16, 2017
  3. sipa commented at 9:59 PM on June 16, 2017: member

    utACK 5c7a7de7b38a84a3ba1e6c26dd1af67236dad891

  4. laanwj renamed this:
    Add AssertLockHeld assertions in CWallet::ListCoins
    [dotnotmerge] Add AssertLockHeld assertions in CWallet::ListCoins
    on Jun 24, 2017
  5. ryanofsky force-pushed on Aug 29, 2017
  6. ryanofsky force-pushed on Aug 29, 2017
  7. promag cross-referenced this on Feb 6, 2018 from issue Make CWallet::ListCoins atomic by promag
  8. ryanofsky force-pushed on Feb 9, 2018
  9. ryanofsky force-pushed on Apr 6, 2018
  10. ryanofsky renamed this:
    [dotnotmerge] Add AssertLockHeld assertions in CWallet::ListCoins
    Add AssertLockHeld assertions in CWallet::ListCoins
    on Apr 6, 2018
  11. ryanofsky commented at 5:58 PM on April 6, 2018: contributor

    Be careful here: this is dependent on a PR that has not been merged yet. Do not merge.

    Dependent PR #10244 got merged, so this is now safe to merge too.


    Rebased 5c7a7de7b38a84a3ba1e6c26dd1af67236dad891 -> 8392051595c615216f3857d54a80cf708ed1f29e (pr/listlock.1 -> pr/listlock.2) due to conflict with #11126 Rebased 8392051595c615216f3857d54a80cf708ed1f29e -> cc69e88921725bed4b1e11823914aa8e418b7220 (pr/listlock.2 -> pr/listlock.3) Rebased cc69e88921725bed4b1e11823914aa8e418b7220 -> a6c634013b7cf5d74bea68e51c011effcd7fc11d (pr/listlock.3 -> pr/listlock.4) due to conflicts with #10295 and #12333 Rebased a6c634013b7cf5d74bea68e51c011effcd7fc11d -> 8740c61a334b3774984cfd1a3d67e33e68adb96a (pr/listlock.4 -> pr/listlock.5)

  12. promag commented at 9:19 PM on April 6, 2018: member

    utACK 8740c61.

  13. Add AssertLockHeld assertions in CWallet::ListCoins 545e85eccc
  14. ryanofsky force-pushed on Apr 11, 2018
  15. ryanofsky commented at 3:45 PM on April 11, 2018: contributor

    Rebased 8740c61a334b3774984cfd1a3d67e33e68adb96a -> 545e85eccc2441c6d7745bb90d88dc14718455a2 (pr/listlock.5 -> pr/listlock.6) due to conflict with #12920

  16. MarcoFalke commented at 8:21 PM on April 27, 2018: member

    Would you mind adding the clang lock annotations here as well?

  17. ryanofsky cross-referenced this on May 8, 2018 from issue Refactor: separate wallet from node by ryanofsky
  18. DrahtBot closed this on Jul 21, 2018

  19. DrahtBot commented at 5:29 PM on July 21, 2018: contributor

    <!--5d09a71f8925f3f132321140b44b946d-->The last travis run for this pull request was 101 days ago and is thus outdated. To trigger a fresh travis build, this pull request should be closed and re-opened.

  20. DrahtBot reopened this on Jul 21, 2018

  21. MarcoFalke commented at 5:51 PM on July 21, 2018: member

    @ryanofsky Are you still working on this? If so, mind to respond to my previous question from April?

  22. MarcoFalke commented at 1:16 AM on July 22, 2018: member

    Missing locking annotations:

    wallet/wallet.cpp:2362:5: error: calling function 'AvailableCoins' requires holding mutex 'cs_wallet' exclusively [-Werror,-Wthread-safety-analysis]
        AvailableCoins(availableCoins);
        ^
    wallet/wallet.cpp:2373:5: error: calling function 'ListLockedCoins' requires holding mutex 'cs_wallet' exclusively [-Werror,-Wthread-safety-analysis]
        ListLockedCoins(lockedCoins);
        ^
    
  23. laanwj commented at 10:38 AM on August 31, 2018: member

    Closing as this seems to be inactive, let me know if you start work on this again and I should reopen it.

  24. laanwj closed this on Aug 31, 2018

  25. laanwj added the label Up for grabs on Aug 31, 2018
  26. ryanofsky commented at 11:53 AM on August 31, 2018: contributor

    Closing as this seems to be inactive, let me know if you start work on this again and I should reopen it.

    This PR can be reopened. There isn't actually any work that needs to be done here since the clang errors @MarcoFalke reported were fixed by @skeees in #13423 (f393a533bebc088985f94c725b9af881500ba998).

  27. MarcoFalke reopened this on Aug 31, 2018

  28. MarcoFalke removed the label Up for grabs on Aug 31, 2018
  29. MarcoFalke commented at 12:04 PM on August 31, 2018: member

    @ryanofsky What I mean with locking annotations is the clang-annotations in the header file:

    diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
    index 05ae9a1bbf..da326517c0 100644
    --- a/src/wallet/wallet.h
    +++ b/src/wallet/wallet.h
    @@ -764,7 +764,7 @@ public:
         /**
          * Return list of available coins and locked coins grouped by non-change output address.
          */
    -    std::map<CTxDestination, std::vector<COutput>> ListCoins() const;
    +    std::map<CTxDestination, std::vector<COutput>> ListCoins() const EXCLUSIVE_LOCKS_REQUIRED(cs_main, cs_wallet);
     
         /**
          * Find non-change parent output.
    

    Without these, it is only a best-guess runtime-check that the correct locks are taken for this method.

  30. Add EXCLUSIVE_LOCKS_REQUIRED to CWallet::ListCoins
    Suggested by MarcoFalke <falke.marco@gmail.com> in
    https://github.com/bitcoin/bitcoin/pull/10605#issuecomment-417643535
    62b6f0f21e
  31. MarcoFalke commented at 12:13 PM on August 31, 2018: member

    utACK 545e85eccc2441c6d7745bb90d88dc14718455a2 (given that the clang lock annotations are included in this commit)

  32. MarcoFalke commented at 12:25 PM on August 31, 2018: member

    utACK 62b6f0f21e24ff367d096c80ebdf398de4a98163

  33. promag commented at 12:46 PM on August 31, 2018: member

    utACK 62b6f0f.

  34. MarcoFalke merged this on Aug 31, 2018
  35. MarcoFalke closed this on Aug 31, 2018

  36. MarcoFalke referenced this in commit b012bbe358 on Aug 31, 2018
  37. Bushstar referenced this in commit 3102eba576 on Sep 4, 2018
  38. Bushstar cross-referenced this on Sep 4, 2018 from issue commits from bitcoin/master by Bushstar
  39. jfhk referenced this in commit e66674943d on Nov 14, 2018
  40. HashUnlimited referenced this in commit f6072117b2 on Nov 26, 2018
  41. jasonbcox referenced this in commit b660a2d9f5 on Oct 11, 2019
  42. jonspock referenced this in commit 0c6abd5881 on Dec 25, 2019
  43. jonspock referenced this in commit 6c6044d2fc on Dec 26, 2019
  44. jonspock referenced this in commit 623b2e06a5 on Dec 27, 2019
  45. PastaPastaPasta referenced this in commit 3ed39fb7e8 on Jun 27, 2021
  46. PastaPastaPasta referenced this in commit 3639112c81 on Jun 28, 2021
  47. PastaPastaPasta referenced this in commit 1bbf70330f on Jun 29, 2021
  48. PastaPastaPasta referenced this in commit c079031e99 on Jun 29, 2021
  49. PastaPastaPasta referenced this in commit 017708af12 on Jun 29, 2021
  50. PastaPastaPasta referenced this in commit f411978924 on Jun 29, 2021
  51. PastaPastaPasta referenced this in commit 4c80a5f989 on Jun 29, 2021
  52. PastaPastaPasta referenced this in commit 7c120a7124 on Jul 1, 2021
  53. Munkybooty referenced this in commit 90c89bbfe8 on Jul 1, 2021
  54. bitcoin locked this on Sep 8, 2021

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