Acquire cs_main lock before cs_wallet during wallet initialization #11126

1. 
   ryanofsky commented at 6:29 pm on August 24, 2017: member

   CWallet::MarkConflicted may acquire the cs_main lock after CWalletDB::LoadWallet acquires the cs_wallet lock during wallet initialization. (CWalletDB::LoadWallet calls ReadKeyValue which calls CWallet::LoadToWallet which calls CWallet::MarkConflicted). This is the opposite order that cs_main and cs_wallet locks are acquired in the rest of the code, and so leads to POTENTIAL DEADLOCK DETECTED errors if bitcoin is built with -DDEBUG_LOCKORDER.

   This commit changes CWallet::LoadWallet (which calls CWalletDB::LoadWallet) to acquire both locks in the standard order.

   Error was reported by @luke-jr in https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/

2. Acquire cs_main lock before cs_wallet during wallet initialization

   CWallet::MarkConflicted may acquire the cs_main lock after
   CWalletDB::LoadWallet acquires the cs_wallet lock during wallet initialization.
   (CWalletDB::LoadWallet calls ReadKeyValue which calls CWallet::LoadToWallet
   which calls CWallet::MarkConflicted). This is the opposite order that cs_main
   and cs_wallet locks are acquired in the rest of the code, and so leads to
   POTENTIAL DEADLOCK DETECTED errors if bitcoin is built with -DDEBUG_LOCKORDER.
   
   This commit changes CWallet::LoadWallet (which calls CWalletDB::LoadWallet) to
   acquire both locks in the standard order. It also fixes some tests that were
   acquiring wallet and main locks out of order and failed with the new locking in
   CWallet::LoadWallet.
   
   Error was reported by Luke Dashjr <luke-jr@utopios.org> in
   https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/

   de9a1db2ed
3. laanwj added the label Wallet on Aug 24, 2017
4. ryanofsky force-pushed on Aug 24, 2017
5. ryanofsky force-pushed on Aug 24, 2017
6. in src/wallet/wallet.cpp:3110 in fef5c2c433 outdated

   3106@@ -3107,6 +3107,8 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c
   3107 
   3108 DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
   3109 {
   3110+    LOCK2(cs_main, cs_wallet);
   

   ---

   ---

   

   jnewbery commented at 6:32 pm on August 25, 2017:

   Looks good. You can remove the LOCK(cs_wallet); from further down in this function (currently L3118)

   ---

   

   ryanofsky commented at 7:00 pm on August 25, 2017:

   Done in 09ee3faaf631fb78150c2725775c65ede66d3a77
7. 
   jnewbery commented at 6:35 pm on August 25, 2017: member

   Looks good. Tested ACK fef5c2c433231bbf6baff8ad6747d7579d35fd7c.

   One nit inline.

8. ryanofsky force-pushed on Aug 25, 2017
9. 
   ryanofsky commented at 7:01 pm on August 25, 2017: member

   Added commit fef5c2c433231bbf6baff8ad6747d7579d35fd7c -> 09ee3faaf631fb78150c2725775c65ede66d3a77 (pr/loadlock2.3 -> pr/loadlock2.4, compare)

   Squashed 09ee3faaf631fb78150c2725775c65ede66d3a77 -> de9a1db2ed14e0c75ffd82dc031f7ad30c56d195 (pr/loadlock2.4 -> pr/loadlock2.5)

10. 
    jnewbery commented at 7:32 pm on August 25, 2017: member

    Tested ACK de9a1db2ed14e0c75ffd82dc031f7ad30c56d195
11. 
    laanwj commented at 8:56 am on August 28, 2017: member

    utACK de9a1db
12. laanwj merged this on Aug 28, 2017
13. laanwj closed this on Aug 28, 2017

    ---

14. laanwj referenced this in commit df91e11ae1 on Aug 28, 2017
15. MarcoFalke added this to the milestone 0.15.1 on Aug 28, 2017
16. MarcoFalke added the label Needs backport on Aug 28, 2017
17. MarcoFalke referenced this in commit 2cb720ae61 on Oct 3, 2017
18. MarcoFalke removed the label Needs backport on Oct 4, 2017
19. UdjinM6 referenced this in commit 32d709f332 on Jul 12, 2019
20. PastaPastaPasta referenced this in commit 2f8512b923 on Jul 12, 2019
21. barrystyle referenced this in commit 85a82c96ba on Jan 22, 2020
22. random-zebra referenced this in commit 4715915d4c on Aug 2, 2020
23. LarryRuane referenced this in commit eff86656f7 on Mar 3, 2021
24. LarryRuane referenced this in commit e0a1266024 on Apr 13, 2021
25. zkbot referenced this in commit 0e36226271 on Apr 17, 2021
26. MarcoFalke locked this on Sep 8, 2021

Contributors
ryanofsky jnewbery laanwj

Labels
Wallet

Milestone
0.15.2