validationinterface: shut down cleanly when a wallet block notification throws
ThreadNotifyWallets' block disconnect and connect loops had no try/catch (only its mempool loop does). A wallet database failure inside SyncWithWallets or ChainTip escaped the thread, and TraceThread logs and rethrows, so the node died in std::terminate with nothing flushed. One example: CWalletDB::ListAccountCreditDebit's "error scanning DB" after BDB lost its file to the wolfSSL CloseSocket bug. That is how the pool's node died on 2026-10-12 and 2026-10-16. Run each block's wallet notifications through NotifyWalletsOrShutdown. On an exception it logs the error, requests a shutdown, shows the usual fatal-error message and stops the notifier, leaving pindexLastTip at the last block the wallet fully processed. Carrying on instead would leave the wallet behind the chain, missing the block it failed on. Thread interruption still propagates. The shutdown is requested before the fatal-error message is shown. FdDefense.WalletNotifyFailureShutsDownCleanly: - a throwing notification requests a shutdown, a clean one doesn't; - a non-std exception is handled the same way; - boost::thread_interrupted still propagates. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
@@ -4,15 +4,23 @@
|
|||||||
|
|
||||||
#include <gtest/gtest.h>
|
#include <gtest/gtest.h>
|
||||||
|
|
||||||
|
#include <atomic>
|
||||||
#include <errno.h>
|
#include <errno.h>
|
||||||
|
#include <stdexcept>
|
||||||
#include <stdio.h>
|
#include <stdio.h>
|
||||||
#include <unistd.h>
|
#include <unistd.h>
|
||||||
|
|
||||||
#include <boost/filesystem.hpp>
|
#include <boost/filesystem.hpp>
|
||||||
|
#include <boost/thread.hpp>
|
||||||
|
|
||||||
#include "chainparams.h"
|
#include "chainparams.h"
|
||||||
|
#include "init.h"
|
||||||
#include "main.h"
|
#include "main.h"
|
||||||
|
#include "noui.h"
|
||||||
#include "util.h"
|
#include "util.h"
|
||||||
|
#include "validationinterface.h"
|
||||||
|
|
||||||
|
extern std::atomic<bool> fRequestShutdown;
|
||||||
|
|
||||||
// A block file opened for reading must not be writable through its fd: a stray write through a reused fd number
|
// A block file opened for reading must not be writable through its fd: a stray write through a reused fd number
|
||||||
// (the wolfSSL CloseSocket macro let the wallet's BDB fd be closed behind its back) used to land in it.
|
// (the wolfSSL CloseSocket macro let the wallet's BDB fd be closed behind its back) used to land in it.
|
||||||
@@ -53,3 +61,23 @@ TEST(FdDefense, ReadOnlyBlockFileIsNotWritable) {
|
|||||||
ClearDatadirCache();
|
ClearDatadirCache();
|
||||||
boost::filesystem::remove_all(dir);
|
boost::filesystem::remove_all(dir);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// A wallet notification that throws (a wallet DB failure) requests a clean shutdown instead of escaping the
|
||||||
|
// notifier thread into std::terminate; a successful one doesn't; thread interruption still propagates.
|
||||||
|
TEST(FdDefense, WalletNotifyFailureShutsDownCleanly) {
|
||||||
|
noui_connect(); // the daemon connects these UI signals at startup; the error path shows a message box
|
||||||
|
fRequestShutdown = false;
|
||||||
|
EXPECT_TRUE(NotifyWalletsOrShutdown([] {}));
|
||||||
|
EXPECT_FALSE(ShutdownRequested());
|
||||||
|
|
||||||
|
EXPECT_FALSE(NotifyWalletsOrShutdown([] { throw std::runtime_error("CWalletDB::ListAccountCreditDebit(): error scanning DB"); }));
|
||||||
|
EXPECT_TRUE(ShutdownRequested());
|
||||||
|
fRequestShutdown = false;
|
||||||
|
|
||||||
|
EXPECT_FALSE(NotifyWalletsOrShutdown([] { throw 42; }));
|
||||||
|
EXPECT_TRUE(ShutdownRequested());
|
||||||
|
fRequestShutdown = false;
|
||||||
|
|
||||||
|
EXPECT_THROW(NotifyWalletsOrShutdown([] { throw boost::thread_interrupted(); }), boost::thread_interrupted);
|
||||||
|
EXPECT_FALSE(ShutdownRequested());
|
||||||
|
}
|
||||||
|
|||||||
+48
-18
@@ -86,6 +86,30 @@ struct CachedBlockData {
|
|||||||
pindex(pindex), oldTrees(oldTrees), txConflicted(txConflicted) {}
|
pindex(pindex), oldTrees(oldTrees), txConflicted(txConflicted) {}
|
||||||
};
|
};
|
||||||
|
|
||||||
|
bool NotifyWalletsOrShutdown(const std::function<void()>& notify)
|
||||||
|
{
|
||||||
|
// The wallet can throw here on a database failure, e.g. CWalletDB::ListAccountCreditDebit's "error scanning
|
||||||
|
// DB" after BDB lost its file. Escaping this thread, that std::terminate()d the node (TraceThread logs and
|
||||||
|
// rethrows) with nothing flushed. Stop cleanly instead: going on would leave the wallet behind the chain,
|
||||||
|
// missing the block it failed on.
|
||||||
|
try {
|
||||||
|
notify();
|
||||||
|
return true;
|
||||||
|
} catch (const boost::thread_interrupted&) {
|
||||||
|
throw;
|
||||||
|
} catch (const std::exception& e) {
|
||||||
|
PrintExceptionContinue(&e, "ThreadNotifyWallets()");
|
||||||
|
} catch (...) {
|
||||||
|
PrintExceptionContinue(NULL, "ThreadNotifyWallets()");
|
||||||
|
}
|
||||||
|
LogPrintf("*** %s\n", "Wallet failed to process a block notification; shutting down");
|
||||||
|
StartShutdown();
|
||||||
|
uiInterface.ThreadSafeMessageBox(
|
||||||
|
_("Error: A fatal internal error occurred, see debug.log for details"),
|
||||||
|
"", CClientUIInterface::MSG_ERROR);
|
||||||
|
return false;
|
||||||
|
}
|
||||||
|
|
||||||
void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
|
void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
|
||||||
{
|
{
|
||||||
// If pindexLastTip == nullptr, the wallet is at genesis.
|
// If pindexLastTip == nullptr, the wallet is at genesis.
|
||||||
@@ -208,13 +232,16 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Let wallets know transactions went from 1-confirmed to
|
if (!NotifyWalletsOrShutdown([&] {
|
||||||
// 0-confirmed or conflicted:
|
// Let wallets know transactions went from 1-confirmed to
|
||||||
for (const CTransaction &tx : block.vtx) {
|
// 0-confirmed or conflicted:
|
||||||
SyncWithWallets(tx, NULL);
|
for (const CTransaction &tx : block.vtx) {
|
||||||
}
|
SyncWithWallets(tx, NULL);
|
||||||
// Update cached incremental witnesses
|
}
|
||||||
GetMainSignals().ChainTip(pindexLastTip, &block, boost::none);
|
// Update cached incremental witnesses
|
||||||
|
GetMainSignals().ChainTip(pindexLastTip, &block, boost::none);
|
||||||
|
}))
|
||||||
|
return;
|
||||||
|
|
||||||
// On to the next block!
|
// On to the next block!
|
||||||
pindexLastTip = pindexLastTip->pprev;
|
pindexLastTip = pindexLastTip->pprev;
|
||||||
@@ -246,17 +273,20 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip)
|
|||||||
break;
|
break;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Tell wallet about transactions that went from mempool
|
if (!NotifyWalletsOrShutdown([&] {
|
||||||
// to conflicted:
|
// Tell wallet about transactions that went from mempool
|
||||||
for (const CTransaction &tx : blockData.txConflicted) {
|
// to conflicted:
|
||||||
SyncWithWallets(tx, NULL);
|
for (const CTransaction &tx : blockData.txConflicted) {
|
||||||
}
|
SyncWithWallets(tx, NULL);
|
||||||
// ... and about transactions that got confirmed:
|
}
|
||||||
for (const CTransaction &tx : block.vtx) {
|
// ... and about transactions that got confirmed:
|
||||||
SyncWithWallets(tx, &block);
|
for (const CTransaction &tx : block.vtx) {
|
||||||
}
|
SyncWithWallets(tx, &block);
|
||||||
// Update cached incremental witnesses
|
}
|
||||||
GetMainSignals().ChainTip(blockData.pindex, &block, blockData.oldTrees);
|
// Update cached incremental witnesses
|
||||||
|
GetMainSignals().ChainTip(blockData.pindex, &block, blockData.oldTrees);
|
||||||
|
}))
|
||||||
|
return;
|
||||||
|
|
||||||
// This block is done!
|
// This block is done!
|
||||||
pindexLastTip = blockData.pindex;
|
pindexLastTip = blockData.pindex;
|
||||||
|
|||||||
@@ -8,6 +8,7 @@
|
|||||||
#define HUSH_VALIDATIONINTERFACE_H
|
#define HUSH_VALIDATIONINTERFACE_H
|
||||||
|
|
||||||
#include <boost/signals2/signal.hpp>
|
#include <boost/signals2/signal.hpp>
|
||||||
|
#include <functional>
|
||||||
#include "zcash/IncrementalMerkleTree.hpp"
|
#include "zcash/IncrementalMerkleTree.hpp"
|
||||||
|
|
||||||
class CBlock;
|
class CBlock;
|
||||||
@@ -76,4 +77,8 @@ CMainSignals& GetMainSignals();
|
|||||||
|
|
||||||
void ThreadNotifyWallets(CBlockIndex *pindexLastTip);
|
void ThreadNotifyWallets(CBlockIndex *pindexLastTip);
|
||||||
|
|
||||||
|
/** Runs one block's wallet notifications for ThreadNotifyWallets. Returns false, after logging the error and
|
||||||
|
* requesting a shutdown, if they threw (e.g. a wallet database failure); thread interruption propagates. */
|
||||||
|
bool NotifyWalletsOrShutdown(const std::function<void()>& notify);
|
||||||
|
|
||||||
#endif // HUSH_VALIDATIONINTERFACE_H
|
#endif // HUSH_VALIDATIONINTERFACE_H
|
||||||
|
|||||||
Reference in New Issue
Block a user