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

Solve chainActive-related locking issues

Browse source 
- In wallet and GUI code LOCK cs_main as well as cs_wallet when
  necessary
- In main.cpp SendMessages move the TRY_LOCK(cs_main) up, to encompass the call
  to IsInitialBlockDownload.
- Make ActivateBestChain, AddToBlockIndex, IsInitialBlockDownload,
  InitBlockIndex acquire the cs_main lock

Fixes #3997
This commit is contained in:
Wladimir J. van der Laan 2014-04-15 17:38:25 +02:00
parent e07c943ce8
commit 55a1db4fa2
7 changed files with 269 additions and 272 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

15
src/main.cpp
View file

@ -1310,7 +1310,7 @@ int GetNumBlocksOfPeers()
bool IsInitialBlockDownload() bool IsInitialBlockDownload()
{ {
AssertLockHeld(cs_main); LOCK(cs_main);
if (fImporting || fReindex || chainActive.Height() < Checkpoints::GetTotalBlocksEstimate()) if (fImporting || fReindex || chainActive.Height() < Checkpoints::GetTotalBlocksEstimate())
return true; return true;
static int64_t nLastUpdate; static int64_t nLastUpdate;
@ -2087,7 +2087,7 @@ void static FindMostWorkChain() {
// Try to activate to the most-work chain (thereby connecting it). // Try to activate to the most-work chain (thereby connecting it).
bool ActivateBestChain(CValidationState &state) { bool ActivateBestChain(CValidationState &state) {
AssertLockHeld(cs_main); LOCK(cs_main);
CBlockIndex *pindexOldTip = chainActive.Tip(); CBlockIndex *pindexOldTip = chainActive.Tip();
bool fComplete = false; bool fComplete = false;
while (!fComplete) { while (!fComplete) {
@ -2136,7 +2136,6 @@ bool ActivateBestChain(CValidationState &state) {
bool AddToBlockIndex(CBlock& block, CValidationState& state, const CDiskBlockPos& pos) bool AddToBlockIndex(CBlock& block, CValidationState& state, const CDiskBlockPos& pos)
{ {
AssertLockHeld(cs_main);
// Check for duplicate // Check for duplicate
uint256 hash = block.GetHash(); uint256 hash = block.GetHash();
if (mapBlockIndex.count(hash)) if (mapBlockIndex.count(hash))
@ -2173,6 +2172,7 @@ bool AddToBlockIndex(CBlock& block, CValidationState& state, const CDiskBlockPos
if (!ActivateBestChain(state)) if (!ActivateBestChain(state))
return false; return false;
LOCK(cs_main);
if (pindexNew == chainActive.Tip()) if (pindexNew == chainActive.Tip())
{ {
// Clear fork warning if its no longer applicable // Clear fork warning if its no longer applicable
@ -2962,6 +2962,7 @@ bool LoadBlockIndex()
bool InitBlockIndex() { bool InitBlockIndex() {
LOCK(cs_main);
// Check whether we're already initialized // Check whether we're already initialized
if (chainActive.Genesis() != NULL) if (chainActive.Genesis() != NULL)
return true; return true;
@ -4201,6 +4202,10 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
} }
} }
TRY_LOCK(cs_main, lockMain); // Acquire cs_main for IsInitialBlockDownload() and CNodeState()
if (!lockMain)
return true;
// Address refresh broadcast // Address refresh broadcast
static int64_t nLastRebroadcast; static int64_t nLastRebroadcast;
if (!IsInitialBlockDownload() && (GetTime() - nLastRebroadcast > 24 * 60 * 60)) if (!IsInitialBlockDownload() && (GetTime() - nLastRebroadcast > 24 * 60 * 60))
@ -4251,10 +4256,6 @@ bool SendMessages(CNode* pto, bool fSendTrickle)
pto->PushMessage("addr", vAddr); pto->PushMessage("addr", vAddr);
} }
TRY_LOCK(cs_main, lockMain);
if (!lockMain)
return true;
CNodeState &state = *State(pto->GetId()); CNodeState &state = *State(pto->GetId());
if (state.fShouldBan) { if (state.fShouldBan) {
if (pto->addr.IsLocal()) if (pto->addr.IsLocal())

3
src/qt/clientmodel.cpp
View file

@ -55,6 +55,7 @@ int ClientModel::getNumConnections(unsigned int flags) const
int ClientModel::getNumBlocks() const int ClientModel::getNumBlocks() const
{ {
LOCK(cs_main);
return chainActive.Height(); return chainActive.Height();
} }
@ -76,6 +77,7 @@ quint64 ClientModel::getTotalBytesSent() const
QDateTime ClientModel::getLastBlockDate() const QDateTime ClientModel::getLastBlockDate() const
{ {
LOCK(cs_main);
if (chainActive.Tip()) if (chainActive.Tip())
return QDateTime::fromTime_t(chainActive.Tip()->GetBlockTime()); return QDateTime::fromTime_t(chainActive.Tip()->GetBlockTime());
else else
@ -84,6 +86,7 @@ QDateTime ClientModel::getLastBlockDate() const
double ClientModel::getVerificationProgress() const double ClientModel::getVerificationProgress() const
{ {
LOCK(cs_main);
return Checkpoints::GuessVerificationProgress(chainActive.Tip()); return Checkpoints::GuessVerificationProgress(chainActive.Tip());
} }

7
src/qt/transactiondesc.cpp
View file

@ -46,8 +46,7 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, int vout, int u
{ {
QString strHTML; QString strHTML;
{ LOCK2(cs_main, wallet->cs_wallet);
LOCK(wallet->cs_wallet);
strHTML.reserve(4000); strHTML.reserve(4000);
strHTML += "<html><font face='verdana, arial, helvetica, sans-serif'>"; strHTML += "<html><font face='verdana, arial, helvetica, sans-serif'>";
@ -272,8 +271,6 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, int vout, int u
strHTML += "<br><b>" + tr("Inputs") + ":</b>"; strHTML += "<br><b>" + tr("Inputs") + ":</b>";
strHTML += "<ul>"; strHTML += "<ul>";
{
LOCK(wallet->cs_wallet);
BOOST_FOREACH(const CTxIn& txin, wtx.vin) BOOST_FOREACH(const CTxIn& txin, wtx.vin)
{ {
COutPoint prevout = txin.prevout; COutPoint prevout = txin.prevout;
@ -297,12 +294,10 @@ QString TransactionDesc::toHTML(CWallet *wallet, CWalletTx &wtx, int vout, int u
} }
} }
} }
}
strHTML += "</ul>"; strHTML += "</ul>";
} }
strHTML += "</font></html>"; strHTML += "</font></html>";
}
return strHTML; return strHTML;
} }

21
src/qt/transactiontablemodel.cpp
View file

@ -78,7 +78,7 @@ public:
qDebug() << "TransactionTablePriv::refreshWallet"; qDebug() << "TransactionTablePriv::refreshWallet";
cachedWallet.clear(); cachedWallet.clear();
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
for(std::map<uint256, CWalletTx>::iterator it = wallet->mapWallet.begin(); it != wallet->mapWallet.end(); ++it) for(std::map<uint256, CWalletTx>::iterator it = wallet->mapWallet.begin(); it != wallet->mapWallet.end(); ++it)
{ {
if(TransactionRecord::showTransaction(it->second)) if(TransactionRecord::showTransaction(it->second))
@ -96,7 +96,7 @@ public:
{ {
qDebug() << "TransactionTablePriv::updateWallet : " + QString::fromStdString(hash.ToString()) + " " + QString::number(status); qDebug() << "TransactionTablePriv::updateWallet : " + QString::fromStdString(hash.ToString()) + " " + QString::number(status);
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
// Find transaction in wallet // Find transaction in wallet
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(hash); std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(hash);
@ -190,10 +190,9 @@ public:
// 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);
if(rec->statusUpdateNeeded()) if(rec->statusUpdateNeeded())
{ {
{
LOCK(wallet->cs_wallet);
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash); std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
if(mi != wallet->mapWallet.end()) if(mi != wallet->mapWallet.end())
@ -201,7 +200,6 @@ public:
rec->updateStatus(mi->second); rec->updateStatus(mi->second);
} }
} }
}
return rec; return rec;
} }
else else
@ -213,7 +211,7 @@ public:
QString describe(TransactionRecord *rec, int unit) QString describe(TransactionRecord *rec, int unit)
{ {
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash); std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(rec->hash);
if(mi != wallet->mapWallet.end()) if(mi != wallet->mapWallet.end())
{ {
@ -228,17 +226,12 @@ TransactionTableModel::TransactionTableModel(CWallet* wallet, WalletModel *paren
QAbstractTableModel(parent), QAbstractTableModel(parent),
wallet(wallet), wallet(wallet),
walletModel(parent), walletModel(parent),
priv(new TransactionTablePriv(wallet, this)), priv(new TransactionTablePriv(wallet, this))
cachedNumBlocks(0)
{ {
columns << QString() << tr("Date") << tr("Type") << tr("Address") << tr("Amount"); columns << QString() << tr("Date") << tr("Type") << tr("Address") << tr("Amount");
priv->refreshWallet(); priv->refreshWallet();
QTimer *timer = new QTimer(this);
connect(timer, SIGNAL(timeout()), this, SLOT(updateConfirmations()));
timer->start(MODEL_UPDATE_DELAY);
connect(walletModel->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit())); connect(walletModel->getOptionsModel(), SIGNAL(displayUnitChanged(int)), this, SLOT(updateDisplayUnit()));
} }
@ -257,16 +250,12 @@ void TransactionTableModel::updateTransaction(const QString &hash, int status)
void TransactionTableModel::updateConfirmations() void TransactionTableModel::updateConfirmations()
{ {
if(chainActive.Height() != cachedNumBlocks)
{
cachedNumBlocks = chainActive.Height();
// Blocks came in since last poll. // Blocks came in since last poll.
// Invalidate status (number of confirmations) and (possibly) description // Invalidate status (number of confirmations) and (possibly) description
// for all rows. Qt is smart enough to only actually request the data for the // for all rows. Qt is smart enough to only actually request the data for the
// visible rows. // visible rows.
emit dataChanged(index(0, Status), index(priv->size()-1, Status)); emit dataChanged(index(0, Status), index(priv->size()-1, Status));
emit dataChanged(index(0, ToAddress), index(priv->size()-1, ToAddress)); emit dataChanged(index(0, ToAddress), index(priv->size()-1, ToAddress));
}
} }
int TransactionTableModel::rowCount(const QModelIndex &parent) const int TransactionTableModel::rowCount(const QModelIndex &parent) const

1
src/qt/transactiontablemodel.h
View file

@ -69,7 +69,6 @@ private:
WalletModel *walletModel; WalletModel *walletModel;
QStringList columns; QStringList columns;
TransactionTablePriv *priv; TransactionTablePriv *priv;
int cachedNumBlocks;
QString lookupAddress(const std::string &address, bool tooltip) const; QString lookupAddress(const std::string &address, bool tooltip) const;
QVariant addressColor(const TransactionRecord *wtx) const; QVariant addressColor(const TransactionRecord *wtx) const;

24
src/qt/walletmodel.cpp
View file

@ -98,11 +98,21 @@ void WalletModel::updateStatus()
void WalletModel::pollBalanceChanged() void WalletModel::pollBalanceChanged()
{ {
bool heightChanged = false;
{
LOCK(cs_main);
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)
transactionTableModel->updateConfirmations();
} }
} }
@ -520,7 +530,7 @@ bool WalletModel::getPubKey(const CKeyID &address, CPubKey& vchPubKeyOut) const
// returns a list of COutputs from COutPoints // returns a list of COutputs from COutPoints
void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vector<COutput>& vOutputs) void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vector<COutput>& vOutputs)
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
BOOST_FOREACH(const COutPoint& outpoint, vOutpoints) BOOST_FOREACH(const COutPoint& outpoint, vOutpoints)
{ {
if (!wallet->mapWallet.count(outpoint.hash)) continue; if (!wallet->mapWallet.count(outpoint.hash)) continue;
@ -533,7 +543,7 @@ void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vect
bool WalletModel::isSpent(const COutPoint& outpoint) const bool WalletModel::isSpent(const COutPoint& outpoint) const
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
return wallet->IsSpent(outpoint.hash, outpoint.n); return wallet->IsSpent(outpoint.hash, outpoint.n);
} }
@ -543,7 +553,7 @@ void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins)
std::vector<COutput> vCoins; std::vector<COutput> vCoins;
wallet->AvailableCoins(vCoins); wallet->AvailableCoins(vCoins);
LOCK(wallet->cs_wallet); // ListLockedCoins, mapWallet LOCK2(cs_main, wallet->cs_wallet); // ListLockedCoins, mapWallet
std::vector<COutPoint> vLockedCoins; std::vector<COutPoint> vLockedCoins;
wallet->ListLockedCoins(vLockedCoins); wallet->ListLockedCoins(vLockedCoins);
@ -575,25 +585,25 @@ void WalletModel::listCoins(std::map<QString, std::vector<COutput> >& mapCoins)
bool WalletModel::isLockedCoin(uint256 hash, unsigned int n) const bool WalletModel::isLockedCoin(uint256 hash, unsigned int n) const
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
return wallet->IsLockedCoin(hash, n); return wallet->IsLockedCoin(hash, n);
} }
void WalletModel::lockCoin(COutPoint& output) void WalletModel::lockCoin(COutPoint& output)
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
wallet->LockCoin(output); wallet->LockCoin(output);
} }
void WalletModel::unlockCoin(COutPoint& output) void WalletModel::unlockCoin(COutPoint& output)
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
wallet->UnlockCoin(output); wallet->UnlockCoin(output);
} }
void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts) void WalletModel::listLockedCoins(std::vector<COutPoint>& vOutpts)
{ {
LOCK(wallet->cs_wallet); LOCK2(cs_main, wallet->cs_wallet);
wallet->ListLockedCoins(vOutpts); wallet->ListLockedCoins(vOutpts);
} }

12
src/wallet.cpp
View file

@ -606,7 +606,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const uint256 &hash, const CTransaction&
void CWallet::SyncTransaction(const uint256 &hash, const CTransaction& tx, const CBlock* pblock) void CWallet::SyncTransaction(const uint256 &hash, const CTransaction& tx, const CBlock* pblock)
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
if (!AddToWalletIfInvolvingMe(hash, tx, pblock, true)) if (!AddToWalletIfInvolvingMe(hash, tx, pblock, true))
return; // Not one of ours return; // Not one of ours
@ -834,7 +834,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
CBlockIndex* pindex = pindexStart; CBlockIndex* pindex = pindexStart;
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
// no need to read and scan block, if block was created before // no need to read and scan block, if block was created before
// our wallet birthday (as adjusted for block time variability) // our wallet birthday (as adjusted for block time variability)
@ -869,7 +869,7 @@ int CWallet::ScanForWalletTransactions(CBlockIndex* pindexStart, bool fUpdate)
void CWallet::ReacceptWalletTransactions() void CWallet::ReacceptWalletTransactions()
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet) BOOST_FOREACH(PAIRTYPE(const uint256, CWalletTx)& item, mapWallet)
{ {
const uint256& wtxid = item.first; const uint256& wtxid = item.first;
@ -964,7 +964,7 @@ int64_t CWallet::GetBalance() const
{ {
int64_t nTotal = 0; int64_t nTotal = 0;
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it)
{ {
const CWalletTx* pcoin = &(*it).second; const CWalletTx* pcoin = &(*it).second;
@ -980,7 +980,7 @@ int64_t CWallet::GetUnconfirmedBalance() const
{ {
int64_t nTotal = 0; int64_t nTotal = 0;
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it)
{ {
const CWalletTx* pcoin = &(*it).second; const CWalletTx* pcoin = &(*it).second;
@ -995,7 +995,7 @@ int64_t CWallet::GetImmatureBalance() const
{ {
int64_t nTotal = 0; int64_t nTotal = 0;
{ {
LOCK(cs_wallet); LOCK2(cs_main, cs_wallet);
for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it) for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it)
{ {
const CWalletTx* pcoin = &(*it).second; const CWalletTx* pcoin = &(*it).second;