Merge #11126: Acquire cs_main lock before cs_wallet during wallet initialization

de9a1db Acquire cs_main lock before cs_wallet during wallet initialization (Russell Yanofsky)

Pull request description:

  `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/

Tree-SHA512: 353fe21bc0a4a2828b41876897001a3c414d4b115ee7430925bd391d8bc396fca81661145d00996c1ba1a01516d9acf8b89fb5c3da27092f5f3aa7e37ef26ffa
This commit is contained in:
Wladimir J. van der Laan 2017-08-28 10:47:07 +02:00 committed by Pasta
parent bcef238d07
commit 2f8512b923
No known key found for this signature in database
GPG Key ID: D362C9F7142766AE
2 changed files with 12 additions and 8 deletions

View File

@ -363,6 +363,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)
{
// Cap last block file size, and mine new block in a new block file.
@ -378,8 +384,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(), 1000 * COIN);
}
@ -392,8 +397,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(), 500 * COIN);
}
@ -598,8 +602,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());
}
@ -633,7 +636,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.

View File

@ -3952,13 +3952,14 @@ CAmount CWallet::GetMinimumFee(unsigned int nTxBytes, unsigned int nConfirmTarge
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();
nKeysLeftSinceAutoBackup = 0;