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/
This commit is contained in:
parent
41496e20f3
commit
de9a1db2ed
2 changed files with 12 additions and 8 deletions
|
@ -364,6 +364,12 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
|
|||
empty_wallet();
|
||||
}
|
||||
|
||||
static void AddKey(CWallet& wallet, const CKey& key)
|
||||
{
|
||||
LOCK(wallet.cs_wallet);
|
||||
wallet.AddKeyPubKey(key, key.GetPubKey());
|
||||
}
|
||||
|
||||
BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
|
||||
{
|
||||
LOCK(cs_main);
|
||||
|
@ -379,8 +385,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
|
|||
// and new block files.
|
||||
{
|
||||
CWallet wallet;
|
||||
LOCK(wallet.cs_wallet);
|
||||
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
|
||||
AddKey(wallet, coinbaseKey);
|
||||
BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip));
|
||||
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN);
|
||||
}
|
||||
|
@ -393,8 +398,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
|
|||
// file.
|
||||
{
|
||||
CWallet wallet;
|
||||
LOCK(wallet.cs_wallet);
|
||||
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
|
||||
AddKey(wallet, coinbaseKey);
|
||||
BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip));
|
||||
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN);
|
||||
}
|
||||
|
@ -599,8 +603,7 @@ public:
|
|||
wallet.reset(new CWallet(std::unique_ptr<CWalletDBWrapper>(new CWalletDBWrapper(&bitdb, "wallet_test.dat"))));
|
||||
bool firstRun;
|
||||
wallet->LoadWallet(firstRun);
|
||||
LOCK(wallet->cs_wallet);
|
||||
wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
|
||||
AddKey(*wallet, coinbaseKey);
|
||||
wallet->ScanForWalletTransactions(chainActive.Genesis());
|
||||
}
|
||||
|
||||
|
@ -635,7 +638,7 @@ public:
|
|||
BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
|
||||
{
|
||||
std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString();
|
||||
LOCK(wallet->cs_wallet);
|
||||
LOCK2(cs_main, wallet->cs_wallet);
|
||||
|
||||
// Confirm ListCoins initially returns 1 coin grouped under coinbaseKey
|
||||
// address.
|
||||
|
|
|
@ -3107,13 +3107,14 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c
|
|||
|
||||
DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
|
||||
{
|
||||
LOCK2(cs_main, cs_wallet);
|
||||
|
||||
fFirstRunRet = false;
|
||||
DBErrors nLoadWalletRet = CWalletDB(*dbw,"cr+").LoadWallet(this);
|
||||
if (nLoadWalletRet == DB_NEED_REWRITE)
|
||||
{
|
||||
if (dbw->Rewrite("\x04pool"))
|
||||
{
|
||||
LOCK(cs_wallet);
|
||||
setInternalKeyPool.clear();
|
||||
setExternalKeyPool.clear();
|
||||
m_pool_key_to_index.clear();
|
||||
|
|
Loading…
Reference in a new issue