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