From 675ab11cf40b1dfa3d52e2b405f181743e014caa Mon Sep 17 00:00:00 2001 From: random-zebra Date: Sat, 1 Aug 2020 17:27:11 +0200 Subject: [PATCH] Acquire cs_main lock before cs_wallet during wallet initialization >>> backports bitcoin/bitcoin@de9a1db2ed14e0c75ffd82dc031f7ad30c56d195 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 in https://botbot.me/freenode/bitcoin-core-dev/msg/90244330/ --- src/wallet/wallet.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4606621c195c..e7d5ea2ddbe7 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3042,13 +3042,14 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarge DBErrors CWallet::LoadWallet(bool& fFirstRunRet) { + LOCK2(cs_main, cs_wallet); + if (!fFileBacked) return DB_LOAD_OK; DBErrors nLoadWalletRet = CWalletDB(strWalletFile, "cr+").LoadWallet(this); if (nLoadWalletRet == DB_NEED_REWRITE) { if (CDB::Rewrite(strWalletFile, "\x04pool")) { - LOCK(cs_wallet); // TODO: Implement spk_man->RewriteDB(). m_spk_man->set_pre_split_keypool.clear(); // Note: can't top-up keypool here, because wallet is locked.