From 5b36d7ad245d86a53def79656b57dc75d3a48618 Mon Sep 17 00:00:00 2001 From: DanS Date: Fri, 16 Oct 2026 21:23:01 -0500 Subject: [PATCH] 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 --- src/gtest/test_fd_defense.cpp | 28 +++++++++++++++ src/validationinterface.cpp | 66 +++++++++++++++++++++++++---------- src/validationinterface.h | 5 +++ 3 files changed, 81 insertions(+), 18 deletions(-) diff --git a/src/gtest/test_fd_defense.cpp b/src/gtest/test_fd_defense.cpp index 20c754a2a..c8c9cb28b 100644 --- a/src/gtest/test_fd_defense.cpp +++ b/src/gtest/test_fd_defense.cpp @@ -4,15 +4,23 @@ #include +#include #include +#include #include #include #include +#include #include "chainparams.h" +#include "init.h" #include "main.h" +#include "noui.h" #include "util.h" +#include "validationinterface.h" + +extern std::atomic fRequestShutdown; // 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. @@ -53,3 +61,23 @@ TEST(FdDefense, ReadOnlyBlockFileIsNotWritable) { ClearDatadirCache(); 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()); +} diff --git a/src/validationinterface.cpp b/src/validationinterface.cpp index f1018833f..4de52d6f2 100644 --- a/src/validationinterface.cpp +++ b/src/validationinterface.cpp @@ -86,6 +86,30 @@ struct CachedBlockData { pindex(pindex), oldTrees(oldTrees), txConflicted(txConflicted) {} }; +bool NotifyWalletsOrShutdown(const std::function& 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) { // If pindexLastTip == nullptr, the wallet is at genesis. @@ -208,13 +232,16 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) break; } - // Let wallets know transactions went from 1-confirmed to - // 0-confirmed or conflicted: - for (const CTransaction &tx : block.vtx) { - SyncWithWallets(tx, NULL); - } - // Update cached incremental witnesses - GetMainSignals().ChainTip(pindexLastTip, &block, boost::none); + if (!NotifyWalletsOrShutdown([&] { + // Let wallets know transactions went from 1-confirmed to + // 0-confirmed or conflicted: + for (const CTransaction &tx : block.vtx) { + SyncWithWallets(tx, NULL); + } + // Update cached incremental witnesses + GetMainSignals().ChainTip(pindexLastTip, &block, boost::none); + })) + return; // On to the next block! pindexLastTip = pindexLastTip->pprev; @@ -246,17 +273,20 @@ void ThreadNotifyWallets(CBlockIndex *pindexLastTip) break; } - // Tell wallet about transactions that went from mempool - // to conflicted: - for (const CTransaction &tx : blockData.txConflicted) { - SyncWithWallets(tx, NULL); - } - // ... and about transactions that got confirmed: - for (const CTransaction &tx : block.vtx) { - SyncWithWallets(tx, &block); - } - // Update cached incremental witnesses - GetMainSignals().ChainTip(blockData.pindex, &block, blockData.oldTrees); + if (!NotifyWalletsOrShutdown([&] { + // Tell wallet about transactions that went from mempool + // to conflicted: + for (const CTransaction &tx : blockData.txConflicted) { + SyncWithWallets(tx, NULL); + } + // ... and about transactions that got confirmed: + for (const CTransaction &tx : block.vtx) { + SyncWithWallets(tx, &block); + } + // Update cached incremental witnesses + GetMainSignals().ChainTip(blockData.pindex, &block, blockData.oldTrees); + })) + return; // This block is done! pindexLastTip = blockData.pindex; diff --git a/src/validationinterface.h b/src/validationinterface.h index 3d4352a8b..98b17abdf 100644 --- a/src/validationinterface.h +++ b/src/validationinterface.h @@ -8,6 +8,7 @@ #define HUSH_VALIDATIONINTERFACE_H #include +#include #include "zcash/IncrementalMerkleTree.hpp" class CBlock; @@ -76,4 +77,8 @@ CMainSignals& GetMainSignals(); 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& notify); + #endif // HUSH_VALIDATIONINTERFACE_H