LBRYCommunity/lbrycrd
1
0
Fork 0
Code Issues 29 Pull requests 1 Projects Releases 32 Packages Wiki Activity

Acquire cs_main lock before cs_wallet during wallet initialization

Browse source 
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:
Russell Yanofsky 2017-08-24 14:12:21 -04:00
parent 41496e20f3
commit de9a1db2ed
2 changed files with 12 additions and 8 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

17
src/wallet/test/wallet_tests.cpp
View file

@ -364,6 +364,12 @@ BOOST_AUTO_TEST_CASE(ApproximateBestSubset)
empty_wallet(); 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) BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
{ {
LOCK(cs_main); LOCK(cs_main);
@ -379,8 +385,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
// and new block files. // and new block files.
{ {
CWallet wallet; CWallet wallet;
LOCK(wallet.cs_wallet); AddKey(wallet, coinbaseKey);
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip)); BOOST_CHECK_EQUAL(nullBlock, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN); BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 100 * COIN);
} }
@ -393,8 +398,7 @@ BOOST_FIXTURE_TEST_CASE(rescan, TestChain100Setup)
// file. // file.
{ {
CWallet wallet; CWallet wallet;
LOCK(wallet.cs_wallet); AddKey(wallet, coinbaseKey);
wallet.AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip)); BOOST_CHECK_EQUAL(oldTip, wallet.ScanForWalletTransactions(oldTip));
BOOST_CHECK_EQUAL(wallet.GetImmatureBalance(), 50 * COIN); 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")))); wallet.reset(new CWallet(std::unique_ptr<CWalletDBWrapper>(new CWalletDBWrapper(&bitdb, "wallet_test.dat"))));
bool firstRun; bool firstRun;
wallet->LoadWallet(firstRun); wallet->LoadWallet(firstRun);
LOCK(wallet->cs_wallet); AddKey(*wallet, coinbaseKey);
wallet->AddKeyPubKey(coinbaseKey, coinbaseKey.GetPubKey());
wallet->ScanForWalletTransactions(chainActive.Genesis()); wallet->ScanForWalletTransactions(chainActive.Genesis());
} }
@ -635,7 +638,7 @@ public:
BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup) BOOST_FIXTURE_TEST_CASE(ListCoins, ListCoinsTestingSetup)
{ {
std::string coinbaseAddress = coinbaseKey.GetPubKey().GetID().ToString(); 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 // Confirm ListCoins initially returns 1 coin grouped under coinbaseKey
// address. // address.

3
src/wallet/wallet.cpp
View file

@ -3107,13 +3107,14 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, const CCoinControl& coin_c
DBErrors CWallet::LoadWallet(bool& fFirstRunRet) DBErrors CWallet::LoadWallet(bool& fFirstRunRet)
{ {
LOCK2(cs_main, cs_wallet);
fFirstRunRet = false; fFirstRunRet = false;
DBErrors nLoadWalletRet = CWalletDB(*dbw,"cr+").LoadWallet(this); DBErrors nLoadWalletRet = CWalletDB(*dbw,"cr+").LoadWallet(this);
if (nLoadWalletRet == DB_NEED_REWRITE) if (nLoadWalletRet == DB_NEED_REWRITE)
{ {
if (dbw->Rewrite("\x04pool")) if (dbw->Rewrite("\x04pool"))
{ {
LOCK(cs_wallet);
setInternalKeyPool.clear(); setInternalKeyPool.clear();
setExternalKeyPool.clear(); setExternalKeyPool.clear();
m_pool_key_to_index.clear(); m_pool_key_to_index.clear();