Eliminate all blocking LOCK(cs_wallet) calls from UI thread
Build All Platforms / build-windows-qt (push) Waiting to run
Build All Platforms / build-windows-daemon (push) Waiting to run
Build All Platforms / build-linux-qt (push) Waiting to run
Build All Platforms / build-linux-daemon (push) Waiting to run
Build All Platforms / build-macos (push) Waiting to run
Build All Platforms / release (push) Blocked by required conditions

During sync, NotifyTransactionChanged fires for every wallet tx in
every block, each triggering 3 blocking LOCK(cs_wallet) calls on
the UI thread: updateWallet, GetAllBalances, getNumTransactions.
With the block processing thread holding cs_wallet almost continuously,
the UI thread blocks waiting for the lock - causing "not responding".

Fixes:
- GetAllBalances: LOCK → TRY_LOCK, returns false if busy
- updateWallet (tx table): LOCK → TRY_LOCK, skips if busy
- updateTransaction: removed checkBalanceChanged() call entirely
  (pollBalanceChanged timer handles it every 2.5s with TRY_LOCK)
- getNumTransactions: replaced with rowCount() from cached model

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
2026-03-22 19:32:04 -07:00
parent ed87543153
commit 65b9417c28
4 changed files with 26 additions and 20 deletions
+3 -1
View File
@@ -95,7 +95,9 @@ public:
{
OutputDebugStringF("updateWallet %s %i\n", hash.ToString().c_str(), status);
{
LOCK(wallet->cs_wallet);
TRY_LOCK(wallet->cs_wallet, lockWallet);
if (!lockWallet)
return;
// Find transaction in wallet
std::map<uint256, CWalletTx>::iterator mi = wallet->mapWallet.find(hash);
+17 -16
View File
@@ -78,12 +78,6 @@ void WalletModel::pollBalanceChanged()
{
if(nBestHeight != cachedNumBlocks)
{
// Don't block the UI thread waiting for cs_wallet - skip this
// update cycle if the lock is held by the block processing thread
TRY_LOCK(wallet->cs_wallet, lockWallet);
if(!lockWallet)
return;
// Balance and number of transactions might have changed
cachedNumBlocks = nBestHeight;
checkBalanceChanged();
@@ -92,11 +86,12 @@ void WalletModel::pollBalanceChanged()
void WalletModel::checkBalanceChanged()
{
// Get all balances in a single lock acquisition + single pass
// instead of 4 separate lock+iterate cycles.
// Use int64_t locals to match GetAllBalances signature (qint64 differs on Linux).
// Get all balances in a single lock acquisition + single pass.
// Uses TRY_LOCK internally - if cs_wallet is busy (block processing),
// skip this cycle. The timer will retry in 2.5 seconds.
int64_t newBalance = 0, newStake = 0, newUnconfirmedBalance = 0, newImmatureBalance = 0;
wallet->GetAllBalances(newBalance, newStake, newUnconfirmedBalance, newImmatureBalance);
if (!wallet->GetAllBalances(newBalance, newStake, newUnconfirmedBalance, newImmatureBalance))
return;
if(cachedBalance != newBalance || cachedStake != newStake || cachedUnconfirmedBalance != newUnconfirmedBalance || cachedImmatureBalance != newImmatureBalance)
{
@@ -113,14 +108,20 @@ void WalletModel::updateTransaction(const QString &hash, int status)
if(transactionTableModel)
transactionTableModel->updateTransaction(hash, status);
// Balance and number of transactions might have changed
checkBalanceChanged();
// Don't call checkBalanceChanged() here - it does LOCK(cs_wallet) + iterates
// all wallet transactions, blocking the UI thread. The pollBalanceChanged()
// timer already handles balance updates every 2.5 seconds with TRY_LOCK.
int newNumTransactions = getNumTransactions();
if(cachedNumTransactions != newNumTransactions)
// Same for getNumTransactions() - use cached count from the transaction model
// to avoid another LOCK(cs_wallet) on the UI thread.
if(transactionTableModel)
{
cachedNumTransactions = newNumTransactions;
emit numTransactionsChanged(newNumTransactions);
int newNumTransactions = transactionTableModel->rowCount(QModelIndex());
if(cachedNumTransactions != newNumTransactions)
{
cachedNumTransactions = newNumTransactions;
emit numTransactionsChanged(newNumTransactions);
}
}
}
+5 -2
View File
@@ -1476,13 +1476,15 @@ int64_t CWallet::GetNewMint() const
return nTotal;
}
void CWallet::GetAllBalances(int64_t& nBalance, int64_t& nStake, int64_t& nUnconfirmed, int64_t& nImmature) const
bool CWallet::GetAllBalances(int64_t& nBalance, int64_t& nStake, int64_t& nUnconfirmed, int64_t& nImmature) const
{
nBalance = 0;
nStake = 0;
nUnconfirmed = 0;
nImmature = 0;
LOCK(cs_wallet);
TRY_LOCK(cs_wallet, lockWallet);
if (!lockWallet)
return false;
for (map<uint256, CWalletTx>::const_iterator it = mapWallet.begin(); it != mapWallet.end(); ++it)
{
const CWalletTx& pcoin = (*it).second;
@@ -1499,6 +1501,7 @@ void CWallet::GetAllBalances(int64_t& nBalance, int64_t& nStake, int64_t& nUncon
if (!pcoin.IsFinal() || !pcoin.IsTrusted())
nUnconfirmed += pcoin.GetAvailableCredit();
}
return true;
}
bool CWallet::SelectCoinsMinConf(int64_t nTargetValue, unsigned int nSpendTime, int nConfMine, int nConfTheirs, vector<COutput> vCoins, set<pair<const CWalletTx*,unsigned int> >& setCoinsRet, int64_t& nValueRet) const
+1 -1
View File
@@ -196,7 +196,7 @@ public:
int64_t GetStake() const;
int64_t GetNewMint() const;
// Get all balances in a single lock acquisition + single pass (avoids 4x lock + 4x iteration)
void GetAllBalances(int64_t& nBalance, int64_t& nStake, int64_t& nUnconfirmed, int64_t& nImmature) const;
bool GetAllBalances(int64_t& nBalance, int64_t& nStake, int64_t& nUnconfirmed, int64_t& nImmature) const;
bool CreateTransaction(const std::vector<std::pair<CScript, int64_t> >& vecSend, CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl *coinControl=NULL);
bool CreateTransaction(CScript scriptPubKey, int64_t nValue, std::string& sNarr, CWalletTx& wtxNew, CReserveKey& reservekey, int64_t& nFeeRet, const CCoinControl *coinControl=NULL);
bool CommitTransaction(CWalletTx& wtxNew, CReserveKey& reservekey);