wallet: Drain validation interface queue after notifications disconnect
What the developer wrote
Authored by Ava Chow
wallet: Drain validation interface queue after notifications disconnect
When unloading a wallet, there may be unexecuted callbacks in the
validation interface queue that can still execute after we have
completed all of the other wallet shutdown tasks. Instead of letting
these run in the background, once the notifications are disconnected,
wait for the queue to drain before continuing with wallet shutdown.
What changed, and why it matters
This change fixes a wallet shutdown timing issue in Bitcoin Core. Previously, when a wallet was unloaded, some background blockchain notifications could still run after the wallet had already finished shutting down. The patch makes the wallet explicitly wait for those pending notifications to finish before completing shutdown, preventing potential crashes or use-after-free problems.
Apply the patch. It is a defensive correctness fix for wallet unload and creation failure paths. No immediate emergency response is indicated, but users running wallet unload/load operations should upgrade to a version containing this fix to avoid potential instability.
Security signals we found
Use-after-free / lifetime bug prevention during asynchronous shutdown
Race condition between wallet teardown and validation interface callbacks
Background queued callbacks executing after wallet shutdown tasks complete
Evidence from the diff
The commit introduces CWallet::DisconnectChainNotifications(), which calls disconnect() on the validation interface notification handler, invokes chain().waitForNotifications() to drain the validation interface queue, and only then resets the handler pointer. It replaces three direct resets of m_chain_notifications_handler in RemoveWallet, CreateNew, and LoadExisting. This ensures no queued validation callbacks execute after wallet shutdown state has been torn down.
Changed components
src/wallet/wallet.cppsrc/wallet/wallet.hCWallet shutdown / unload pathValidation interface notification queueInspect captured patch +16 / −3
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
index 63dab299..1e7ad3fc 100644
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -169,7 +169,7 @@ bool RemoveWallet(WalletContext& context, const std::shared_ptr<CWallet>& wallet
WITH_LOCK(wallet->cs_wallet, wallet->WriteBestBlock());
// Unregister with the validation interface which also drops shared pointers.
- wallet->m_chain_notifications_handler.reset();
+ wallet->DisconnectChainNotifications();
{
LOCK(context.wallets_mutex);
std::vector<std::shared_ptr<CWallet>>::iterator i = std::find(context.wallets.begin(), context.wallets.end(), wallet);
@@ -3117,7 +3117,7 @@ std::shared_ptr<CWallet> CWallet::CreateNew(WalletContext& context, const std::s
walletInstance->TopUpKeyPool();
if (chain && !AttachChain(walletInstance, *chain, /*rescan_required=*/false, error, warnings)) {
- walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded
+ walletInstance->DisconnectChainNotifications();
return nullptr;
}
@@ -3158,7 +3158,7 @@ std::shared_ptr<CWallet> CWallet::LoadExisting(WalletContext& context, const std
walletInstance->TopUpKeyPool();
if (chain && !AttachChain(walletInstance, *chain, rescan_required, error, warnings)) {
- walletInstance->m_chain_notifications_handler.reset(); // Reset this pointer so that the wallet will actually be unloaded
+ walletInstance->DisconnectChainNotifications();
return nullptr;
}
@@ -4577,4 +4577,14 @@ std::optional<WalletTXO> CWallet::GetTXO(const COutPoint& outpoint) const
}
return it->second;
}
+
+void CWallet::DisconnectChainNotifications()
+{
+ if (m_chain_notifications_handler) {
+ m_chain_notifications_handler->disconnect();
+ chain().waitForNotifications();
+ m_chain_notifications_handler.reset();
+ }
+}
+
} // namespace wallet
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
index e3d36236..fd4b368d 100644
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -1072,6 +1072,9 @@ public:
//! Find the private key for the given key id from the wallet's descriptors, if available
//! Returns nullopt when no descriptor has the key or if the wallet is locked.
std::optional<CKey> GetKey(const CKeyID& keyid) const;
+
+ //! Disconnect chain notifications and wait for all notifications to be processed
+ void DisconnectChainNotifications();
};
/**
Why this scored 40/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.