Merge pull request #4085

b39a07d Add missing AssertLockHeld in ConnectBlock (Wladimir J. van der Laan)
41106a5 qt: get required locks upfront in polling functions (Wladimir J. van der Laan)
ed67100 Add required locks in tests (Wladimir J. van der Laan)
This commit is contained in:
Wladimir J. van der Laan 2014-04-23 17:05:36 +02:00
commit 89bbd54fbf
No known key found for this signature in database
GPG Key ID: 74810B012346C9A6
7 changed files with 40 additions and 19 deletions

View File

@ -1727,6 +1727,7 @@ void ThreadScriptCheck() {
bool ConnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck) bool ConnectBlock(CBlock& block, CValidationState& state, CBlockIndex* pindex, CCoinsViewCache& view, bool fJustCheck)
{ {
AssertLockHeld(cs_main);
// Check it again in case a previous version let a bad block in // Check it again in case a previous version let a bad block in
if (!CheckBlock(block, state, !fJustCheck, !fJustCheck)) if (!CheckBlock(block, state, !fJustCheck, !fJustCheck))
return false; return false;

View File

@ -92,6 +92,12 @@ double ClientModel::getVerificationProgress() const
void ClientModel::updateTimer() void ClientModel::updateTimer()
{ {
// Get required lock upfront. This avoids the GUI from getting stuck on
// periodical polls if the core is holding the locks for a longer time -
// for example, during a wallet rescan.
TRY_LOCK(cs_main, lockMain);
if(!lockMain)
return;
// Some quantities (such as number of blocks) change so fast that we don't want to be notified for each change. // Some quantities (such as number of blocks) change so fast that we don't want to be notified for each change.
// Periodically check and update with a timer. // Periodically check and update with a timer.
int newNumBlocks = getNumBlocks(); int newNumBlocks = getNumBlocks();

View File

@ -24,7 +24,6 @@
#include <QDebug> #include <QDebug>
#include <QIcon> #include <QIcon>
#include <QList> #include <QList>
#include <QTimer>
// Amount column is right-aligned it contains numbers // Amount column is right-aligned it contains numbers
static int column_alignments[] = { static int column_alignments[] = {
@ -187,11 +186,18 @@ public:
{ {
TransactionRecord *rec = &cachedWallet[idx]; TransactionRecord *rec = &cachedWallet[idx];
// Get required locks upfront. This avoids the GUI from getting
// stuck if the core is holding the locks for a longer time - for
// example, during a wallet rescan.
//
// If a status update is needed (blocks came in since last check), // If a status update is needed (blocks came in since last check),
// update the status of this transaction from the wallet. Otherwise, // update the status of this transaction from the wallet. Otherwise,
// simply re-use the cached status. // simply re-use the cached status.
LOCK2(cs_main, wallet->cs_wallet); TRY_LOCK(cs_main, lockMain);
if(rec->statusUpdateNeeded()) if(lockMain)
{
TRY_LOCK(wallet->cs_wallet, lockWallet);
if(lockWallet && rec->statusUpdateNeeded())
{ {
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash); std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
@ -200,6 +206,7 @@ public:
rec->updateStatus(mi->second); rec->updateStatus(mi->second);
} }
} }
}
return rec; return rec;
} }
else else

View File

@ -98,18 +98,21 @@ void WalletModel::updateStatus()
void WalletModel::pollBalanceChanged() void WalletModel::pollBalanceChanged()
{ {
bool heightChanged = false; // Get required locks upfront. This avoids the GUI from getting stuck on
{ // periodical polls if the core is holding the locks for a longer time -
LOCK(cs_main); // for example, during a wallet rescan.
TRY_LOCK(cs_main, lockMain);
if(!lockMain)
return;
TRY_LOCK(wallet->cs_wallet, lockWallet);
if(!lockWallet)
return;
if(chainActive.Height() != cachedNumBlocks) if(chainActive.Height() != cachedNumBlocks)
{ {
// Balance and number of transactions might have changed // Balance and number of transactions might have changed
cachedNumBlocks = chainActive.Height(); cachedNumBlocks = chainActive.Height();
heightChanged = true;
}
}
if(heightChanged)
{
checkBalanceChanged(); checkBalanceChanged();
if(transactionTableModel) if(transactionTableModel)
transactionTableModel->updateConfirmations(); transactionTableModel->updateConfirmations();

View File

@ -65,7 +65,7 @@ BOOST_AUTO_TEST_CASE(rpc_wallet)
// Test RPC calls for various wallet statistics // Test RPC calls for various wallet statistics
Value r; Value r;
LOCK(pwalletMain->cs_wallet); LOCK2(cs_main, pwalletMain->cs_wallet);
BOOST_CHECK_NO_THROW(CallRPC("listunspent")); BOOST_CHECK_NO_THROW(CallRPC("listunspent"));
BOOST_CHECK_THROW(CallRPC("listunspent string"), runtime_error); BOOST_CHECK_THROW(CallRPC("listunspent string"), runtime_error);

View File

@ -50,6 +50,7 @@ BOOST_AUTO_TEST_SUITE(script_P2SH_tests)
BOOST_AUTO_TEST_CASE(sign) BOOST_AUTO_TEST_CASE(sign)
{ {
LOCK(cs_main);
// Pay-to-script-hash looks like this: // Pay-to-script-hash looks like this:
// scriptSig: <sig> <sig...> <serialized_script> // scriptSig: <sig> <sig...> <serialized_script>
// scriptPubKey: HASH160 <hash> EQUAL // scriptPubKey: HASH160 <hash> EQUAL
@ -147,6 +148,7 @@ BOOST_AUTO_TEST_CASE(norecurse)
BOOST_AUTO_TEST_CASE(set) BOOST_AUTO_TEST_CASE(set)
{ {
LOCK(cs_main);
// Test the CScript::Set* methods // Test the CScript::Set* methods
CBasicKeyStore keystore; CBasicKeyStore keystore;
CKey key[4]; CKey key[4];
@ -250,6 +252,7 @@ BOOST_AUTO_TEST_CASE(switchover)
BOOST_AUTO_TEST_CASE(AreInputsStandard) BOOST_AUTO_TEST_CASE(AreInputsStandard)
{ {
LOCK(cs_main);
CCoinsView coinsDummy; CCoinsView coinsDummy;
CCoinsViewCache coins(coinsDummy); CCoinsViewCache coins(coinsDummy);
CBasicKeyStore keystore; CBasicKeyStore keystore;

View File

@ -254,6 +254,7 @@ BOOST_AUTO_TEST_CASE(test_Get)
BOOST_AUTO_TEST_CASE(test_IsStandard) BOOST_AUTO_TEST_CASE(test_IsStandard)
{ {
LOCK(cs_main);
CBasicKeyStore keystore; CBasicKeyStore keystore;
CCoinsView coinsDummy; CCoinsView coinsDummy;
CCoinsViewCache coins(coinsDummy); CCoinsViewCache coins(coinsDummy);