diff --git a/src/Makefile.am b/src/Makefile.am index 85c3eaf08d..4e9c161c57 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -205,6 +205,7 @@ BITCOIN_CORE_H = \ netbase.h \ netgroup.h \ netmessagemaker.h \ + node/abort.h \ node/blockmanager_args.h \ node/blockstorage.h \ node/caches.h \ @@ -310,6 +311,7 @@ BITCOIN_CORE_H = \ util/readwritefile.h \ util/result.h \ util/serfloat.h \ + util/signalinterrupt.h \ util/sock.h \ util/spanparsing.h \ util/string.h \ @@ -399,6 +401,7 @@ libbitcoin_node_a_SOURCES = \ net.cpp \ net_processing.cpp \ netgroup.cpp \ + node/abort.cpp \ node/blockmanager_args.cpp \ node/blockstorage.cpp \ node/caches.cpp \ @@ -733,6 +736,7 @@ libbitcoin_util_a_SOURCES = \ util/moneystr.cpp \ util/rbf.cpp \ util/readwritefile.cpp \ + util/signalinterrupt.cpp \ util/thread.cpp \ util/threadinterrupt.cpp \ util/threadnames.cpp \ @@ -933,7 +937,6 @@ libbitcoinkernel_la_SOURCES = \ logging.cpp \ node/blockstorage.cpp \ node/chainstate.cpp \ - node/interface_ui.cpp \ node/utxo_snapshot.cpp \ policy/feerate.cpp \ policy/fees.cpp \ @@ -972,6 +975,7 @@ libbitcoinkernel_la_SOURCES = \ util/moneystr.cpp \ util/rbf.cpp \ util/serfloat.cpp \ + util/signalinterrupt.cpp \ util/strencodings.cpp \ util/string.cpp \ util/syserror.cpp \ diff --git a/src/bitcoin-chainstate.cpp b/src/bitcoin-chainstate.cpp index 432bdc8e33..c36233207e 100644 --- a/src/bitcoin-chainstate.cpp +++ b/src/bitcoin-chainstate.cpp @@ -97,6 +97,15 @@ int main(int argc, char* argv[]) { std::cout << "Warning: " << warning.original << std::endl; } + void flushError(const std::string& debug_message) override + { + std::cerr << "Error flushing block data to disk: " << debug_message << std::endl; + } + void fatalError(const std::string& debug_message, const bilingual_str& user_message) override + { + std::cerr << "Error: " << debug_message << std::endl; + std::cerr << (user_message.empty() ? "A fatal internal error occurred." : user_message.original) << std::endl; + } }; auto notifications = std::make_unique(); @@ -112,8 +121,9 @@ int main(int argc, char* argv[]) const node::BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = abs_datadir / "blocks", + .notifications = chainman_opts.notifications, }; - ChainstateManager chainman{chainman_opts, blockman_opts}; + ChainstateManager chainman{kernel_context.interrupt, chainman_opts, blockman_opts}; node::CacheSizes cache_sizes; cache_sizes.block_tree_db = 2 << 20; diff --git a/src/index/base.cpp b/src/index/base.cpp index ec23cc1247..cf07cae286 100644 --- a/src/index/base.cpp +++ b/src/index/base.cpp @@ -8,6 +8,7 @@ #include #include #include +#include #include #include #include @@ -30,9 +31,10 @@ constexpr auto SYNC_LOG_INTERVAL{30s}; constexpr auto SYNC_LOCATOR_WRITE_INTERVAL{30s}; template -static void FatalError(const char* fmt, const Args&... args) +void BaseIndex::FatalErrorf(const char* fmt, const Args&... args) { - AbortNode(tfm::format(fmt, args...)); + auto message = tfm::format(fmt, args...); + node::AbortNode(m_chain->context()->exit_status, message); } CBlockLocator GetLocator(interfaces::Chain& chain, const uint256& block_hash) @@ -197,7 +199,7 @@ void BaseIndex::ThreadSync() break; } if (pindex_next->pprev != pindex && !Rewind(pindex, pindex_next->pprev)) { - FatalError("%s: Failed to rewind index %s to a previous chain tip", + FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); return; } @@ -221,14 +223,14 @@ void BaseIndex::ThreadSync() CBlock block; interfaces::BlockInfo block_info = kernel::MakeBlockInfo(pindex); if (!m_chainstate->m_blockman.ReadBlockFromDisk(block, *pindex)) { - FatalError("%s: Failed to read block %s from disk", + FatalErrorf("%s: Failed to read block %s from disk", __func__, pindex->GetBlockHash().ToString()); return; } else { block_info.data = █ } if (!CustomAppend(block_info)) { - FatalError("%s: Failed to write block %s to index database", + FatalErrorf("%s: Failed to write block %s to index database", __func__, pindex->GetBlockHash().ToString()); return; } @@ -294,7 +296,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr& block, const const CBlockIndex* best_block_index = m_best_block_index.load(); if (!best_block_index) { if (pindex->nHeight != 0) { - FatalError("%s: First block connected is not the genesis block (height=%d)", + FatalErrorf("%s: First block connected is not the genesis block (height=%d)", __func__, pindex->nHeight); return; } @@ -312,7 +314,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr& block, const return; } if (best_block_index != pindex->pprev && !Rewind(best_block_index, pindex->pprev)) { - FatalError("%s: Failed to rewind index %s to a previous chain tip", + FatalErrorf("%s: Failed to rewind index %s to a previous chain tip", __func__, GetName()); return; } @@ -325,7 +327,7 @@ void BaseIndex::BlockConnected(const std::shared_ptr& block, const // processed, and the index object being safe to delete. SetBestBlockIndex(pindex); } else { - FatalError("%s: Failed to write block %s to index", + FatalErrorf("%s: Failed to write block %s to index", __func__, pindex->GetBlockHash().ToString()); return; } @@ -345,7 +347,7 @@ void BaseIndex::ChainStateFlushed(const CBlockLocator& locator) } if (!locator_tip_index) { - FatalError("%s: First block (hash=%s) in locator was not found", + FatalErrorf("%s: First block (hash=%s) in locator was not found", __func__, locator_tip_hash.ToString()); return; } diff --git a/src/index/base.h b/src/index/base.h index 231f36b605..8affee90f8 100644 --- a/src/index/base.h +++ b/src/index/base.h @@ -94,6 +94,9 @@ private: virtual bool AllowPrune() const = 0; + template + void FatalErrorf(const char* fmt, const Args&... args); + protected: std::unique_ptr m_chain; Chainstate* m_chainstate{nullptr}; diff --git a/src/init.cpp b/src/init.cpp index c38352ee38..988976028d 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -812,10 +812,6 @@ bool AppInitBasicSetup(const ArgsManager& args, std::atomic& exit_status) // Enable heap terminate-on-corruption HeapSetInformation(nullptr, HeapEnableTerminationOnCorruption, nullptr, 0); #endif - if (!InitShutdownState(exit_status)) { - return InitError(Untranslated("Initializing wait-for-shutdown state failed.")); - } - if (!SetupNetworking()) { return InitError(Untranslated("Initializing networking failed.")); } @@ -988,7 +984,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) // Also report errors from parsing before daemonization { - KernelNotifications notifications{}; + kernel::Notifications notifications{}; ChainstateManager::Options chainman_opts_dummy{ .chainparams = chainparams, .datadir = args.GetDataDirNet(), @@ -1001,6 +997,7 @@ bool AppInitParameterInteraction(const ArgsManager& args) BlockManager::Options blockman_opts_dummy{ .chainparams = chainman_opts_dummy.chainparams, .blocks_dir = args.GetBlocksDirPath(), + .notifications = chainman_opts_dummy.notifications, }; auto blockman_result{ApplyArgsManOptions(args, blockman_opts_dummy)}; if (!blockman_result) { @@ -1411,7 +1408,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) // ********************************************************* Step 7: load block chain - node.notifications = std::make_unique(); + node.notifications = std::make_unique(node.exit_status); fReindex = args.GetBoolArg("-reindex", false); bool fReindexChainState = args.GetBoolArg("-reindex-chainstate", false); ChainstateManager::Options chainman_opts{ @@ -1425,6 +1422,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = args.GetBlocksDirPath(), + .notifications = chainman_opts.notifications, }; Assert(ApplyArgsManOptions(args, blockman_opts)); // no error can happen, already checked in AppInitParameterInteraction @@ -1464,7 +1462,7 @@ bool AppInitMain(NodeContext& node, interfaces::BlockAndHeaderTipInfo* tip_info) for (bool fLoaded = false; !fLoaded && !ShutdownRequested();) { node.mempool = std::make_unique(mempool_opts); - node.chainman = std::make_unique(chainman_opts, blockman_opts); + node.chainman = std::make_unique(node.kernel->interrupt, chainman_opts, blockman_opts); ChainstateManager& chainman = *node.chainman; node::ChainstateLoadOptions options; diff --git a/src/kernel/blockmanager_opts.h b/src/kernel/blockmanager_opts.h index 8f26422f72..0251bbb10a 100644 --- a/src/kernel/blockmanager_opts.h +++ b/src/kernel/blockmanager_opts.h @@ -5,6 +5,7 @@ #ifndef BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H #define BITCOIN_KERNEL_BLOCKMANAGER_OPTS_H +#include #include #include @@ -25,6 +26,7 @@ struct BlockManagerOpts { bool fast_prune{false}; bool stop_after_block_import{DEFAULT_STOPAFTERBLOCKIMPORT}; const fs::path blocks_dir; + Notifications& notifications; }; } // namespace kernel diff --git a/src/kernel/context.cpp b/src/kernel/context.cpp index 1205da869e..3f4a423531 100644 --- a/src/kernel/context.cpp +++ b/src/kernel/context.cpp @@ -14,9 +14,12 @@ namespace kernel { +Context* g_context; Context::Context() { + assert(!g_context); + g_context = this; std::string sha256_algo = SHA256AutoDetect(); LogPrintf("Using the '%s' SHA256 implementation\n", sha256_algo); RandomInit(); @@ -26,6 +29,8 @@ Context::Context() Context::~Context() { ECC_Stop(); + assert(g_context); + g_context = nullptr; } } // namespace kernel diff --git a/src/kernel/context.h b/src/kernel/context.h index f11b7b54f0..e74e0a6f08 100644 --- a/src/kernel/context.h +++ b/src/kernel/context.h @@ -5,6 +5,8 @@ #ifndef BITCOIN_KERNEL_CONTEXT_H #define BITCOIN_KERNEL_CONTEXT_H +#include + #include namespace kernel { @@ -16,12 +18,24 @@ namespace kernel { //! State stored directly in this struct should be simple. More complex state //! should be stored to std::unique_ptr members pointing to opaque types. struct Context { + //! Interrupt object that can be used to stop long-running kernel operations. + util::SignalInterrupt interrupt; + //! Declare default constructor and destructor that are not inline, so code //! instantiating the kernel::Context struct doesn't need to #include class //! definitions for all the unique_ptr members. Context(); ~Context(); }; + +//! Global pointer to kernel::Context for legacy code. New code should avoid +//! using this, and require state it needs to be passed to it directly. +//! +//! Having this pointer is useful because it allows state be moved out of global +//! variables into the kernel::Context struct before all global references to +//! that state are removed. This allows the global references to be removed +//! incrementally, instead of all at once. +extern Context* g_context; } // namespace kernel #endif // BITCOIN_KERNEL_CONTEXT_H diff --git a/src/kernel/mempool_persist.cpp b/src/kernel/mempool_persist.cpp index 71f3aac366..d060e45af3 100644 --- a/src/kernel/mempool_persist.cpp +++ b/src/kernel/mempool_persist.cpp @@ -9,13 +9,13 @@ #include #include #include -#include #include #include #include #include #include #include +#include #include #include @@ -95,7 +95,7 @@ bool LoadMempool(CTxMemPool& pool, const fs::path& load_path, Chainstate& active } else { ++expired; } - if (ShutdownRequested()) + if (active_chainstate.m_chainman.m_interrupt) return false; } std::map mapDeltas; diff --git a/src/kernel/notifications_interface.h b/src/kernel/notifications_interface.h index 48248e9aa0..e596a144a8 100644 --- a/src/kernel/notifications_interface.h +++ b/src/kernel/notifications_interface.h @@ -5,12 +5,13 @@ #ifndef BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H #define BITCOIN_KERNEL_NOTIFICATIONS_INTERFACE_H +#include + #include #include class CBlockIndex; enum class SynchronizationState; -struct bilingual_str; namespace kernel { @@ -27,6 +28,23 @@ public: virtual void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) {} virtual void progress(const bilingual_str& title, int progress_percent, bool resume_possible) {} virtual void warning(const bilingual_str& warning) {} + + //! The flush error notification is sent to notify the user that an error + //! occurred while flushing block data to disk. Kernel code may ignore flush + //! errors that don't affect the immediate operation it is trying to + //! perform. Applications can choose to handle the flush error notification + //! by logging the error, or notifying the user, or triggering an early + //! shutdown as a precaution against causing more errors. + virtual void flushError(const std::string& debug_message) {} + + //! The fatal error notification is sent to notify the user when an error + //! occurs in kernel code that can't be recovered from. After this + //! notification is sent, whatever function triggered the error should also + //! return an error code or raise an exception. Applications can choose to + //! handle the fatal error notification by logging the error, or notifying + //! the user, or triggering an early shutdown as a precaution against + //! causing more errors. + virtual void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) {} }; } // namespace kernel diff --git a/src/node/abort.cpp b/src/node/abort.cpp new file mode 100644 index 0000000000..a554126b86 --- /dev/null +++ b/src/node/abort.cpp @@ -0,0 +1,27 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#include +#include +#include +#include +#include + +#include +#include +#include + +namespace node { + +void AbortNode(std::atomic& exit_status, const std::string& debug_message, const bilingual_str& user_message, bool shutdown) +{ + SetMiscWarning(Untranslated(debug_message)); + LogPrintf("*** %s\n", debug_message); + InitError(user_message.empty() ? _("A fatal internal error occurred, see debug.log for details") : user_message); + exit_status.store(EXIT_FAILURE); + if (shutdown) StartShutdown(); +} +} // namespace node diff --git a/src/node/abort.h b/src/node/abort.h new file mode 100644 index 0000000000..d6bb0c14d5 --- /dev/null +++ b/src/node/abort.h @@ -0,0 +1,17 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_NODE_ABORT_H +#define BITCOIN_NODE_ABORT_H + +#include + +#include +#include + +namespace node { +void AbortNode(std::atomic& exit_status, const std::string& debug_message, const bilingual_str& user_message = {}, bool shutdown = true); +} // namespace node + +#endif // BITCOIN_NODE_ABORT_H diff --git a/src/node/blockstorage.cpp b/src/node/blockstorage.cpp index 223b0e6a17..191b6dc2f5 100644 --- a/src/node/blockstorage.cpp +++ b/src/node/blockstorage.cpp @@ -13,12 +13,12 @@ #include #include #include -#include #include #include #include #include #include +#include #include #include @@ -250,7 +250,8 @@ CBlockIndex* BlockManager::InsertBlockIndex(const uint256& hash) bool BlockManager::LoadBlockIndex() { - if (!m_block_tree_db->LoadBlockIndexGuts(GetConsensus(), [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); })) { + if (!m_block_tree_db->LoadBlockIndexGuts( + GetConsensus(), [this](const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return this->InsertBlockIndex(hash); }, m_interrupt)) { return false; } @@ -260,7 +261,7 @@ bool BlockManager::LoadBlockIndex() CBlockIndexHeightOnlyComparator()); for (CBlockIndex* pindex : vSortedByHeight) { - if (ShutdownRequested()) return false; + if (m_interrupt) return false; pindex->nChainWork = (pindex->pprev ? pindex->pprev->nChainWork : 0) + GetBlockProof(*pindex); pindex->nTimeMax = (pindex->pprev ? std::max(pindex->pprev->nTimeMax, pindex->nTime) : pindex->nTime); @@ -527,7 +528,7 @@ void BlockManager::FlushUndoFile(int block_file, bool finalize) { FlatFilePos undo_pos_old(block_file, m_blockfile_info[block_file].nUndoSize); if (!UndoFileSeq().Flush(undo_pos_old, finalize)) { - AbortNode("Flushing undo file to disk failed. This is likely the result of an I/O error."); + m_opts.notifications.flushError("Flushing undo file to disk failed. This is likely the result of an I/O error."); } } @@ -546,7 +547,7 @@ void BlockManager::FlushBlockFile(bool fFinalize, bool finalize_undo) FlatFilePos block_pos_old(m_last_blockfile, m_blockfile_info[m_last_blockfile].nSize); if (!BlockFileSeq().Flush(block_pos_old, fFinalize)) { - AbortNode("Flushing block file to disk failed. This is likely the result of an I/O error."); + m_opts.notifications.flushError("Flushing block file to disk failed. This is likely the result of an I/O error."); } // we do not always flush the undo file, as the chain tip may be lagging behind the incoming blocks, // e.g. during IBD or a sync after a node going offline @@ -658,7 +659,8 @@ bool BlockManager::FindBlockPos(FlatFilePos& pos, unsigned int nAddSize, unsigne bool out_of_space; size_t bytes_allocated = BlockFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode("Disk space is too low!", _("Disk space is too low!")); + m_opts.notifications.fatalError("Disk space is too low!", _("Disk space is too low!")); + return false; } if (bytes_allocated != 0 && IsPruneMode()) { m_check_for_pruning = true; @@ -682,7 +684,7 @@ bool BlockManager::FindUndoPos(BlockValidationState& state, int nFile, FlatFileP bool out_of_space; size_t bytes_allocated = UndoFileSeq().Allocate(pos, nAddSize, out_of_space); if (out_of_space) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_opts.notifications, state, "Disk space is too low!", _("Disk space is too low!")); } if (bytes_allocated != 0 && IsPruneMode()) { m_check_for_pruning = true; @@ -724,7 +726,7 @@ bool BlockManager::WriteUndoDataForBlock(const CBlockUndo& blockundo, BlockValid return error("ConnectBlock(): FindUndoPos failed"); } if (!UndoWriteToDisk(blockundo, _pos, block.pprev->GetBlockHash(), GetParams().MessageStart())) { - return AbortNode(state, "Failed to write undo data"); + return FatalError(m_opts.notifications, state, "Failed to write undo data"); } // rev files are written in block height order, whereas blk files are written as blocks come in (often out of order) // we want to flush the rev (undo) file once we've written the last block, which is indicated by the last height @@ -842,7 +844,7 @@ FlatFilePos BlockManager::SaveBlockToDisk(const CBlock& block, int nHeight, CCha } if (!position_known) { if (!WriteBlockToDisk(block, blockPos, GetParams().MessageStart())) { - AbortNode("Failed to write block"); + m_opts.notifications.fatalError("Failed to write block"); return FlatFilePos(); } } @@ -890,8 +892,8 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile } LogPrintf("Reindexing block file blk%05u.dat...\n", (unsigned int)nFile); chainman.ActiveChainstate().LoadExternalBlockFile(file, &pos, &blocks_with_unknown_parent); - if (ShutdownRequested()) { - LogPrintf("Shutdown requested. Exit %s\n", __func__); + if (chainman.m_interrupt) { + LogPrintf("Interrupt requested. Exit %s\n", __func__); return; } nFile++; @@ -909,8 +911,8 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile if (file) { LogPrintf("Importing blocks file %s...\n", fs::PathToString(path)); chainman.ActiveChainstate().LoadExternalBlockFile(file); - if (ShutdownRequested()) { - LogPrintf("Shutdown requested. Exit %s\n", __func__); + if (chainman.m_interrupt) { + LogPrintf("Interrupt requested. Exit %s\n", __func__); return; } } else { @@ -926,7 +928,7 @@ void ThreadImport(ChainstateManager& chainman, std::vector vImportFile for (Chainstate* chainstate : WITH_LOCK(::cs_main, return chainman.GetAll())) { BlockValidationState state; if (!chainstate->ActivateBestChain(state, nullptr)) { - AbortNode(strprintf("Failed to connect best block (%s)", state.ToString())); + chainman.GetNotifications().fatalError(strprintf("Failed to connect best block (%s)", state.ToString())); return; } } diff --git a/src/node/blockstorage.h b/src/node/blockstorage.h index a1ebb0df0a..36b8aa4806 100644 --- a/src/node/blockstorage.h +++ b/src/node/blockstorage.h @@ -33,6 +33,9 @@ struct FlatFilePos; namespace Consensus { struct Params; } +namespace util { +class SignalInterrupt; +} // namespace util namespace node { @@ -153,10 +156,12 @@ private: public: using Options = kernel::BlockManagerOpts; - explicit BlockManager(Options opts) + explicit BlockManager(const util::SignalInterrupt& interrupt, Options opts) : m_prune_mode{opts.prune_target > 0}, - m_opts{std::move(opts)} {}; + m_opts{std::move(opts)}, + m_interrupt{interrupt} {}; + const util::SignalInterrupt& m_interrupt; std::atomic m_importing{false}; BlockMap m_block_index GUARDED_BY(cs_main); diff --git a/src/node/kernel_notifications.cpp b/src/node/kernel_notifications.cpp index 926b157f3b..0be7d51afb 100644 --- a/src/node/kernel_notifications.cpp +++ b/src/node/kernel_notifications.cpp @@ -10,7 +10,12 @@ #include #include +#include +#include +#include #include +#include +#include #include #include #include @@ -72,4 +77,14 @@ void KernelNotifications::warning(const bilingual_str& warning) DoWarning(warning); } +void KernelNotifications::flushError(const std::string& debug_message) +{ + AbortNode(m_exit_status, debug_message); +} + +void KernelNotifications::fatalError(const std::string& debug_message, const bilingual_str& user_message) +{ + node::AbortNode(m_exit_status, debug_message, user_message, m_shutdown_on_fatal_error); +} + } // namespace node diff --git a/src/node/kernel_notifications.h b/src/node/kernel_notifications.h index 3e665bbf14..d35b6ecca5 100644 --- a/src/node/kernel_notifications.h +++ b/src/node/kernel_notifications.h @@ -7,6 +7,7 @@ #include +#include #include #include @@ -18,6 +19,8 @@ namespace node { class KernelNotifications : public kernel::Notifications { public: + KernelNotifications(std::atomic& exit_status) : m_exit_status{exit_status} {} + void blockTip(SynchronizationState state, CBlockIndex& index) override; void headerTip(SynchronizationState state, int64_t height, int64_t timestamp, bool presync) override; @@ -25,6 +28,15 @@ public: void progress(const bilingual_str& title, int progress_percent, bool resume_possible) override; void warning(const bilingual_str& warning) override; + + void flushError(const std::string& debug_message) override; + + void fatalError(const std::string& debug_message, const bilingual_str& user_message = {}) override; + + //! Useful for tests, can be set to false to avoid shutdown on fatal error. + bool m_shutdown_on_fatal_error{true}; +private: + std::atomic& m_exit_status; }; } // namespace node diff --git a/src/qt/test/apptests.cpp b/src/qt/test/apptests.cpp index e918e84184..fb8029cb65 100644 --- a/src/qt/test/apptests.cpp +++ b/src/qt/test/apptests.cpp @@ -86,7 +86,6 @@ void AppTests::appTests() // Reset global state to avoid interfering with later tests. LogInstance().DisconnectTestLogger(); - AbortShutdown(); } //! Entry point for BitcoinGUI tests. diff --git a/src/shutdown.cpp b/src/shutdown.cpp index d70017d734..fc18ccd207 100644 --- a/src/shutdown.cpp +++ b/src/shutdown.cpp @@ -5,107 +5,40 @@ #include -#if defined(HAVE_CONFIG_H) -#include -#endif - +#include #include -#include #include -#include -#include +#include #include -#include -#ifdef WIN32 -#include -#endif - -static std::atomic* g_exit_status{nullptr}; - -bool AbortNode(const std::string& strMessage, bilingual_str user_message) -{ - SetMiscWarning(Untranslated(strMessage)); - LogPrintf("*** %s\n", strMessage); - if (user_message.empty()) { - user_message = _("A fatal internal error occurred, see debug.log for details"); - } - InitError(user_message); - Assert(g_exit_status)->store(EXIT_FAILURE); - StartShutdown(); - return false; -} - -static std::atomic fRequestShutdown(false); -#ifdef WIN32 -/** On windows it is possible to simply use a condition variable. */ -std::mutex g_shutdown_mutex; -std::condition_variable g_shutdown_cv; -#else -/** On UNIX-like operating systems use the self-pipe trick. - */ -static TokenPipeEnd g_shutdown_r; -static TokenPipeEnd g_shutdown_w; -#endif - -bool InitShutdownState(std::atomic& exit_status) -{ - g_exit_status = &exit_status; -#ifndef WIN32 - std::optional pipe = TokenPipe::Make(); - if (!pipe) return false; - g_shutdown_r = pipe->TakeReadEnd(); - g_shutdown_w = pipe->TakeWriteEnd(); -#endif - return true; -} +#include void StartShutdown() { -#ifdef WIN32 - std::unique_lock lk(g_shutdown_mutex); - fRequestShutdown = true; - g_shutdown_cv.notify_one(); -#else - // This must be reentrant and safe for calling in a signal handler, so using a condition variable is not safe. - // Make sure that the token is only written once even if multiple threads call this concurrently or in - // case of a reentrant signal. - if (!fRequestShutdown.exchange(true)) { - // Write an arbitrary byte to the write end of the shutdown pipe. - int res = g_shutdown_w.TokenWrite('x'); - if (res != 0) { - LogPrintf("Sending shutdown token failed\n"); - assert(0); - } + try { + Assert(kernel::g_context)->interrupt(); + } catch (const std::system_error&) { + LogPrintf("Sending shutdown token failed\n"); + assert(0); } -#endif } void AbortShutdown() { - if (fRequestShutdown) { - // Cancel existing shutdown by waiting for it, this will reset condition flags and remove - // the shutdown token from the pipe. - WaitForShutdown(); - } - fRequestShutdown = false; + Assert(kernel::g_context)->interrupt.reset(); } bool ShutdownRequested() { - return fRequestShutdown; + return bool{Assert(kernel::g_context)->interrupt}; } void WaitForShutdown() { -#ifdef WIN32 - std::unique_lock lk(g_shutdown_mutex); - g_shutdown_cv.wait(lk, [] { return fRequestShutdown.load(); }); -#else - int res = g_shutdown_r.TokenRead(); - if (res != 'x') { + try { + Assert(kernel::g_context)->interrupt.wait(); + } catch (const std::system_error&) { LogPrintf("Reading shutdown token failed\n"); assert(0); } -#endif } diff --git a/src/shutdown.h b/src/shutdown.h index c119bee96f..2d6ace8d93 100644 --- a/src/shutdown.h +++ b/src/shutdown.h @@ -6,18 +6,6 @@ #ifndef BITCOIN_SHUTDOWN_H #define BITCOIN_SHUTDOWN_H -#include // For bilingual_str - -#include - -/** Abort with a message */ -bool AbortNode(const std::string& strMessage, bilingual_str user_message = bilingual_str{}); - -/** Initialize shutdown state. This must be called before using either StartShutdown(), - * AbortShutdown() or WaitForShutdown(). Calling ShutdownRequested() is always safe. - */ -bool InitShutdownState(std::atomic& exit_status); - /** Request shutdown of the application. */ void StartShutdown(); diff --git a/src/test/blockmanager_tests.cpp b/src/test/blockmanager_tests.cpp index f094766886..2ab2fa55f0 100644 --- a/src/test/blockmanager_tests.cpp +++ b/src/test/blockmanager_tests.cpp @@ -5,14 +5,16 @@ #include #include #include +#include #include #include #include #include -using node::BlockManager; using node::BLOCK_SERIALIZATION_HEADER_SIZE; +using node::BlockManager; +using node::KernelNotifications; using node::MAX_BLOCKFILE_SIZE; // use BasicTestingSetup here for the data directory configuration, setup, and cleanup @@ -21,11 +23,13 @@ BOOST_FIXTURE_TEST_SUITE(blockmanager_tests, BasicTestingSetup) BOOST_AUTO_TEST_CASE(blockmanager_find_block_pos) { const auto params {CreateChainParams(ArgsManager{}, ChainType::MAIN)}; + KernelNotifications notifications{m_node.exit_status}; const BlockManager::Options blockman_opts{ .chainparams = *params, .blocks_dir = m_args.GetBlocksDirPath(), + .notifications = notifications, }; - BlockManager blockman{blockman_opts}; + BlockManager blockman{m_node.kernel->interrupt, blockman_opts}; CChain chain {}; // simulate adding a genesis block normally BOOST_CHECK_EQUAL(blockman.SaveBlockToDisk(params->GenesisBlock(), 0, chain, nullptr).nPos, BLOCK_SERIALIZATION_HEADER_SIZE); diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp index 93a60db832..3e2f0ab88d 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -184,7 +184,7 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto m_cache_sizes = CalculateCacheSizes(m_args); - m_node.notifications = std::make_unique(); + m_node.notifications = std::make_unique(m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = chainparams, @@ -196,8 +196,9 @@ ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vecto const BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), + .notifications = chainman_opts.notifications, }; - m_node.chainman = std::make_unique(chainman_opts, blockman_opts); + m_node.chainman = std::make_unique(m_node.kernel->interrupt, chainman_opts, blockman_opts); m_node.chainman->m_blockman.m_block_tree_db = std::make_unique(DBParams{ .path = m_args.GetDataDirNet() / "blocks" / "index", .cache_bytes = static_cast(m_cache_sizes.block_tree_db), diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp index b797de46af..8f46d54621 100644 --- a/src/test/validation_chainstatemanager_tests.cpp +++ b/src/test/validation_chainstatemanager_tests.cpp @@ -379,7 +379,7 @@ struct SnapshotTestSetup : TestChain100Setup { LOCK(::cs_main); chainman.ResetChainstates(); BOOST_CHECK_EQUAL(chainman.GetAll().size(), 0); - m_node.notifications = std::make_unique(); + m_node.notifications = std::make_unique(m_node.exit_status); const ChainstateManager::Options chainman_opts{ .chainparams = ::Params(), .datadir = chainman.m_options.datadir, @@ -389,11 +389,12 @@ struct SnapshotTestSetup : TestChain100Setup { const BlockManager::Options blockman_opts{ .chainparams = chainman_opts.chainparams, .blocks_dir = m_args.GetBlocksDirPath(), + .notifications = chainman_opts.notifications, }; // For robustness, ensure the old manager is destroyed before creating a // new one. m_node.chainman.reset(); - m_node.chainman = std::make_unique(chainman_opts, blockman_opts); + m_node.chainman = std::make_unique(m_node.kernel->interrupt, chainman_opts, blockman_opts); } return *Assert(m_node.chainman); } @@ -563,7 +564,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup auto db_cache_before_complete = active_cs.m_coinsdb_cache_size_bytes; SnapshotCompletionResult res; - auto mock_shutdown = [](bilingual_str msg) {}; + m_node.notifications->m_shutdown_on_fatal_error = false; fs::path snapshot_chainstate_dir = *node::FindSnapshotChainstateDir(chainman.m_options.datadir); BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); @@ -573,8 +574,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup const uint256 snapshot_tip_hash = WITH_LOCK(chainman.GetMutex(), return chainman.ActiveTip()->GetBlockHash()); - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SUCCESS); WITH_LOCK(::cs_main, BOOST_CHECK(chainman.IsSnapshotValidated())); @@ -590,8 +590,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion, SnapshotTestSetup BOOST_CHECK_EQUAL(all_chainstates[0], &active_cs); // Trying completion again should return false. - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::SKIPPED); // The invalid snapshot path should not have been used. @@ -644,7 +643,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna Chainstate& validation_chainstate = *std::get<0>(chainstates); ChainstateManager& chainman = *Assert(m_node.chainman); SnapshotCompletionResult res; - auto mock_shutdown = [](bilingual_str msg) {}; + m_node.notifications->m_shutdown_on_fatal_error = false; // Test tampering with the IBD UTXO set with an extra coin to ensure it causes // snapshot completion to fail. @@ -660,8 +659,7 @@ BOOST_FIXTURE_TEST_CASE(chainstatemanager_snapshot_completion_hash_mismatch, Sna fs::path snapshot_chainstate_dir = gArgs.GetDataDirNet() / "chainstate_snapshot"; BOOST_CHECK(fs::exists(snapshot_chainstate_dir)); - res = WITH_LOCK(::cs_main, - return chainman.MaybeCompleteSnapshotValidation(mock_shutdown)); + res = WITH_LOCK(::cs_main, return chainman.MaybeCompleteSnapshotValidation()); BOOST_CHECK_EQUAL(res, SnapshotCompletionResult::HASH_MISMATCH); auto all_chainstates = chainman.GetAll(); diff --git a/src/txdb.cpp b/src/txdb.cpp index b2095bd4a3..e5b5e0b8a1 100644 --- a/src/txdb.cpp +++ b/src/txdb.cpp @@ -9,8 +9,8 @@ #include #include #include -#include #include +#include #include #include @@ -291,7 +291,7 @@ bool CBlockTreeDB::ReadFlag(const std::string &name, bool &fValue) { return true; } -bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex) +bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex, const util::SignalInterrupt& interrupt) { AssertLockHeld(::cs_main); std::unique_ptr pcursor(NewIterator()); @@ -299,7 +299,7 @@ bool CBlockTreeDB::LoadBlockIndexGuts(const Consensus::Params& consensusParams, // Load m_block_index while (pcursor->Valid()) { - if (ShutdownRequested()) return false; + if (interrupt) return false; std::pair key; if (pcursor->GetKey(key) && key.first == DB_BLOCK_INDEX) { CDiskBlockIndex diskindex; diff --git a/src/txdb.h b/src/txdb.h index 04d0ecb39f..6405437be9 100644 --- a/src/txdb.h +++ b/src/txdb.h @@ -29,6 +29,9 @@ class uint256; namespace Consensus { struct Params; }; +namespace util { +class SignalInterrupt; +} // namespace util //! -dbcache default (MiB) static const int64_t nDefaultDbCache = 450; @@ -98,7 +101,7 @@ public: void ReadReindexing(bool &fReindexing); bool WriteFlag(const std::string &name, bool fValue); bool ReadFlag(const std::string &name, bool &fValue); - bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex) + bool LoadBlockIndexGuts(const Consensus::Params& consensusParams, std::function insertBlockIndex, const util::SignalInterrupt& interrupt) EXCLUSIVE_LOCKS_REQUIRED(::cs_main); }; diff --git a/src/util/signalinterrupt.cpp b/src/util/signalinterrupt.cpp new file mode 100644 index 0000000000..c551ba8044 --- /dev/null +++ b/src/util/signalinterrupt.cpp @@ -0,0 +1,74 @@ +// Copyright (c) 2022 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#include + +#ifdef WIN32 +#include +#else +#include +#endif + +#include +#include + +namespace util { + +SignalInterrupt::SignalInterrupt() : m_flag{false} +{ +#ifndef WIN32 + std::optional pipe = TokenPipe::Make(); + if (!pipe) throw std::ios_base::failure("Could not create TokenPipe"); + m_pipe_r = pipe->TakeReadEnd(); + m_pipe_w = pipe->TakeWriteEnd(); +#endif +} + +SignalInterrupt::operator bool() const +{ + return m_flag; +} + +void SignalInterrupt::reset() +{ + // Cancel existing interrupt by waiting for it, this will reset condition flags and remove + // the token from the pipe. + if (*this) wait(); + m_flag = false; +} + +void SignalInterrupt::operator()() +{ +#ifdef WIN32 + std::unique_lock lk(m_mutex); + m_flag = true; + m_cv.notify_one(); +#else + // This must be reentrant and safe for calling in a signal handler, so using a condition variable is not safe. + // Make sure that the token is only written once even if multiple threads call this concurrently or in + // case of a reentrant signal. + if (!m_flag.exchange(true)) { + // Write an arbitrary byte to the write end of the pipe. + int res = m_pipe_w.TokenWrite('x'); + if (res != 0) { + throw std::ios_base::failure("Could not write interrupt token"); + } + } +#endif +} + +void SignalInterrupt::wait() +{ +#ifdef WIN32 + std::unique_lock lk(m_mutex); + m_cv.wait(lk, [this] { return m_flag.load(); }); +#else + int res = m_pipe_r.TokenRead(); + if (res != 'x') { + throw std::ios_base::failure("Did not read expected interrupt token"); + } +#endif +} + +} // namespace util diff --git a/src/util/signalinterrupt.h b/src/util/signalinterrupt.h new file mode 100644 index 0000000000..ca02feda91 --- /dev/null +++ b/src/util/signalinterrupt.h @@ -0,0 +1,52 @@ +// Copyright (c) 2023 The Bitcoin Core developers +// Distributed under the MIT software license, see the accompanying +// file COPYING or http://www.opensource.org/licenses/mit-license.php. + +#ifndef BITCOIN_UTIL_SIGNALINTERRUPT_H +#define BITCOIN_UTIL_SIGNALINTERRUPT_H + +#ifdef WIN32 +#include +#include +#else +#include +#endif + +#include +#include + +namespace util { +/** + * Helper class that manages an interrupt flag, and allows a thread or + * signal to interrupt another thread. + * + * This class is safe to be used in a signal handler. If sending an interrupt + * from a signal handler is not necessary, the more lightweight \ref + * CThreadInterrupt class can be used instead. + */ + +class SignalInterrupt +{ +public: + SignalInterrupt(); + explicit operator bool() const; + void operator()(); + void reset(); + void wait(); + +private: + std::atomic m_flag; + +#ifndef WIN32 + // On UNIX-like operating systems use the self-pipe trick. + TokenPipeEnd m_pipe_r; + TokenPipeEnd m_pipe_w; +#else + // On windows use a condition variable, since we don't have any signals there + std::mutex m_mutex; + std::condition_variable m_cv; +#endif +}; +} // namespace util + +#endif // BITCOIN_UTIL_SIGNALINTERRUPT_H diff --git a/src/util/threadinterrupt.h b/src/util/threadinterrupt.h index ccc053f576..0b79b38276 100644 --- a/src/util/threadinterrupt.h +++ b/src/util/threadinterrupt.h @@ -12,11 +12,17 @@ #include #include -/* - A helper class for interruptible sleeps. Calling operator() will interrupt - any current sleep, and after that point operator bool() will return true - until reset. -*/ +/** + * A helper class for interruptible sleeps. Calling operator() will interrupt + * any current sleep, and after that point operator bool() will return true + * until reset. + * + * This class should not be used in a signal handler. It uses thread + * synchronization primitives that are not safe to use with signals. If sending + * an interrupt from a signal handler is necessary, the \ref SignalInterrupt + * class can be used instead. + */ + class CThreadInterrupt { public: diff --git a/src/validation.cpp b/src/validation.cpp index 6836498a64..be6afbd320 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -50,6 +50,7 @@ #include #include #include +#include #include #include #include @@ -1841,9 +1842,9 @@ bool CheckInputScripts(const CTransaction& tx, TxValidationState& state, return true; } -bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage) +bool FatalError(Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage) { - AbortNode(strMessage, userMessage); + notifications.fatalError(strMessage, userMessage); return state.Error(strMessage); } @@ -2078,7 +2079,7 @@ bool Chainstate::ConnectBlock(const CBlock& block, BlockValidationState& state, // We don't write down blocks to disk if they may have been // corrupted, so this should be impossible unless we're having hardware // problems. - return AbortNode(state, "Corrupt block found indicating potential hardware failure; shutting down"); + return FatalError(m_chainman.GetNotifications(), state, "Corrupt block found indicating potential hardware failure; shutting down"); } return error("%s: Consensus::CheckBlock: %s", __func__, state.ToString()); } @@ -2498,7 +2499,7 @@ bool Chainstate::FlushStateToDisk( if (fDoFullFlush || fPeriodicWrite) { // Ensure we can write block index if (!CheckDiskSpace(m_blockman.m_opts.blocks_dir)) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); } { LOG_TIME_MILLIS_WITH_CATEGORY("write block and undo data to disk", BCLog::BENCH); @@ -2512,7 +2513,7 @@ bool Chainstate::FlushStateToDisk( LOG_TIME_MILLIS_WITH_CATEGORY("write block index to disk", BCLog::BENCH); if (!m_blockman.WriteBlockIndexDB()) { - return AbortNode(state, "Failed to write to block index database"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to write to block index database"); } } // Finally remove any pruned files @@ -2534,11 +2535,11 @@ bool Chainstate::FlushStateToDisk( // an overestimation, as most will delete an existing entry or // overwrite one. Still, use a conservative safety factor of 2. if (!CheckDiskSpace(m_chainman.m_options.datadir, 48 * 2 * 2 * CoinsTip().GetCacheSize())) { - return AbortNode(state, "Disk space is too low!", _("Disk space is too low!")); + return FatalError(m_chainman.GetNotifications(), state, "Disk space is too low!", _("Disk space is too low!")); } // Flush the chainstate (which may refer to block index entries). if (!CoinsTip().Flush()) - return AbortNode(state, "Failed to write to coin database"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to write to coin database"); m_last_flush = nNow; full_flush_completed = true; TRACE5(utxocache, flush, @@ -2554,7 +2555,7 @@ bool Chainstate::FlushStateToDisk( GetMainSignals().ChainStateFlushed(m_chain.GetLocator()); } } catch (const std::runtime_error& e) { - return AbortNode(state, std::string("System error while flushing: ") + e.what()); + return FatalError(m_chainman.GetNotifications(), state, std::string("System error while flushing: ") + e.what()); } return true; } @@ -2790,7 +2791,7 @@ bool Chainstate::ConnectTip(BlockValidationState& state, CBlockIndex* pindexNew, if (!pblock) { std::shared_ptr pblockNew = std::make_shared(); if (!m_blockman.ReadBlockFromDisk(*pblockNew, *pindexNew)) { - return AbortNode(state, "Failed to read block"); + return FatalError(m_chainman.GetNotifications(), state, "Failed to read block"); } pthisBlock = pblockNew; } else { @@ -2974,7 +2975,7 @@ bool Chainstate::ActivateBestChainStep(BlockValidationState& state, CBlockIndex* // If we're unable to disconnect a block during normal operation, // then that is a failure of our local system -- we should abort // rather than stay on a less work chain. - AbortNode(state, "Failed to disconnect block; see debug.log for details"); + FatalError(m_chainman.GetNotifications(), state, "Failed to disconnect block; see debug.log for details"); return false; } fBlocksDisconnected = true; @@ -3183,11 +3184,11 @@ bool Chainstate::ActivateBestChain(BlockValidationState& state, std::shared_ptr< break; } - // We check shutdown only after giving ActivateBestChainStep a chance to run once so that we - // never shutdown before connecting the genesis block during LoadChainTip(). Previously this - // caused an assert() failure during shutdown in such cases as the UTXO DB flushing checks + // We check interrupt only after giving ActivateBestChainStep a chance to run once so that we + // never interrupt before connecting the genesis block during LoadChainTip(). Previously this + // caused an assert() failure during interrupt in such cases as the UTXO DB flushing checks // that the best block hash is non-null. - if (ShutdownRequested()) break; + if (m_chainman.m_interrupt) break; } while (pindexNewTip != pindexMostWork); CheckBlockIndex(); @@ -3277,7 +3278,7 @@ bool Chainstate::InvalidateBlock(BlockValidationState& state, CBlockIndex* pinde // Disconnect (descendants of) pindex, and mark them invalid. while (true) { - if (ShutdownRequested()) break; + if (m_chainman.m_interrupt) break; // Make sure the queue of validation callbacks doesn't grow unboundedly. LimitValidationInterfaceQueue(); @@ -3969,7 +3970,7 @@ bool Chainstate::AcceptBlock(const std::shared_ptr& pblock, BlockV } ReceivedBlockTransactions(block, pindex, blockPos); } catch (const std::runtime_error& e) { - return AbortNode(state, std::string("System error: ") + e.what()); + return FatalError(m_chainman.GetNotifications(), state, std::string("System error: ") + e.what()); } FlushStateToDisk(state, FlushStateMode::NONE); @@ -4079,7 +4080,7 @@ void Chainstate::LoadMempool(const fs::path& load_path, FopenFn mockable_fopen_f { if (!m_mempool) return; ::LoadMempool(*m_mempool, load_path, *this, mockable_fopen_function); - m_mempool->SetLoadTried(!ShutdownRequested()); + m_mempool->SetLoadTried(!m_chainman.m_interrupt); } bool Chainstate::LoadChainTip() @@ -4212,7 +4213,7 @@ VerifyDBResult CVerifyDB::VerifyDB( skipped_l3_checks = true; } } - if (ShutdownRequested()) return VerifyDBResult::INTERRUPTED; + if (chainstate.m_chainman.m_interrupt) return VerifyDBResult::INTERRUPTED; } if (pindexFailure) { LogPrintf("Verification error: coin database inconsistencies found (last %i blocks, %i good transactions before that)\n", chainstate.m_chain.Height() - pindexFailure->nHeight + 1, nGoodTransactions); @@ -4245,7 +4246,7 @@ VerifyDBResult CVerifyDB::VerifyDB( LogPrintf("Verification error: found unconnectable block at %d, hash=%s (%s)\n", pindex->nHeight, pindex->GetBlockHash().ToString(), state.ToString()); return VerifyDBResult::CORRUPTED_BLOCK_DB; } - if (ShutdownRequested()) return VerifyDBResult::INTERRUPTED; + if (chainstate.m_chainman.m_interrupt) return VerifyDBResult::INTERRUPTED; } } @@ -4413,7 +4414,7 @@ bool ChainstateManager::LoadBlockIndex() } for (CBlockIndex* pindex : vSortedByHeight) { - if (ShutdownRequested()) return false; + if (m_interrupt) return false; if (pindex->IsAssumedValid() || (pindex->IsValid(BLOCK_VALID_TRANSACTIONS) && (pindex->HaveTxsDownloaded() || pindex->pprev == nullptr))) { @@ -4519,7 +4520,7 @@ void Chainstate::LoadExternalBlockFile( // such as a block fails to deserialize. uint64_t nRewind = blkdat.GetPos(); while (!blkdat.eof()) { - if (ShutdownRequested()) return; + if (m_chainman.m_interrupt) return; blkdat.SetPos(nRewind); nRewind++; // start one byte further next time, in case of failure @@ -4660,7 +4661,7 @@ void Chainstate::LoadExternalBlockFile( } } } catch (const std::runtime_error& e) { - AbortNode(std::string("System error: ") + e.what()); + m_chainman.GetNotifications().fatalError(std::string("System error: ") + e.what()); } LogPrintf("Loaded %i blocks from external file in %dms\n", nLoaded, Ticks(SteadyClock::now() - start)); } @@ -5112,7 +5113,7 @@ bool ChainstateManager::ActivateSnapshot( snapshot_chainstate.reset(); bool removed = DeleteCoinsDBFromDisk(*snapshot_datadir, /*is_snapshot=*/true); if (!removed) { - AbortNode(strprintf("Failed to remove snapshot chainstate dir (%s). " + GetNotifications().fatalError(strprintf("Failed to remove snapshot chainstate dir (%s). " "Manually remove it before restarting.\n", fs::PathToString(*snapshot_datadir))); } } @@ -5152,13 +5153,13 @@ struct StopHashingException : public std::exception { const char* what() const throw() override { - return "ComputeUTXOStats interrupted by shutdown."; + return "ComputeUTXOStats interrupted."; } }; -static void SnapshotUTXOHashBreakpoint() +static void SnapshotUTXOHashBreakpoint(const util::SignalInterrupt& interrupt) { - if (ShutdownRequested()) throw StopHashingException(); + if (interrupt) throw StopHashingException(); } bool ChainstateManager::PopulateAndValidateSnapshot( @@ -5235,7 +5236,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // If our average Coin size is roughly 41 bytes, checking every 120,000 coins // means <5MB of memory imprecision. if (coins_processed % 120000 == 0) { - if (ShutdownRequested()) { + if (m_interrupt) { return false; } @@ -5292,7 +5293,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( try { maybe_stats = ComputeUTXOStats( - CoinStatsHashType::HASH_SERIALIZED, snapshot_coinsdb, m_blockman, SnapshotUTXOHashBreakpoint); + CoinStatsHashType::HASH_SERIALIZED, snapshot_coinsdb, m_blockman, [&interrupt = m_interrupt] { SnapshotUTXOHashBreakpoint(interrupt); }); } catch (StopHashingException const&) { return false; } @@ -5377,8 +5378,7 @@ bool ChainstateManager::PopulateAndValidateSnapshot( // through IsUsable() checks, or // // (ii) giving each chainstate its own lock instead of using cs_main for everything. -SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( - std::function shutdown_fnc) +SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation() { AssertLockHeld(cs_main); if (m_ibd_chainstate.get() == &this->ActiveChainstate() || @@ -5430,7 +5430,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( user_error = strprintf(Untranslated("%s\n%s"), user_error, util::ErrorString(rename_result)); } - shutdown_fnc(user_error); + GetNotifications().fatalError(user_error.original, user_error); }; if (index_new.GetBlockHash() != snapshot_blockhash) { @@ -5470,7 +5470,7 @@ SnapshotCompletionResult ChainstateManager::MaybeCompleteSnapshotValidation( CoinStatsHashType::HASH_SERIALIZED, &ibd_coins_db, m_blockman, - SnapshotUTXOHashBreakpoint); + [&interrupt = m_interrupt] { SnapshotUTXOHashBreakpoint(interrupt); }); } catch (StopHashingException const&) { return SnapshotCompletionResult::STATS_FAILED; } @@ -5579,9 +5579,10 @@ static ChainstateManager::Options&& Flatten(ChainstateManager::Options&& opts) return std::move(opts); } -ChainstateManager::ChainstateManager(Options options, node::BlockManager::Options blockman_options) - : m_options{Flatten(std::move(options))}, - m_blockman{std::move(blockman_options)} {} +ChainstateManager::ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options) + : m_interrupt{interrupt}, + m_options{Flatten(std::move(options))}, + m_blockman{interrupt, std::move(blockman_options)} {} ChainstateManager::~ChainstateManager() { @@ -5726,13 +5727,13 @@ bool ChainstateManager::ValidatedSnapshotCleanup() fs::path tmp_old{ibd_chainstate_path + "_todelete"}; - auto rename_failed_abort = []( + auto rename_failed_abort = [this]( fs::path p_old, fs::path p_new, const fs::filesystem_error& err) { LogPrintf("%s: error renaming file (%s): %s\n", __func__, fs::PathToString(p_old), err.what()); - AbortNode(strprintf( + GetNotifications().fatalError(strprintf( "Rename of '%s' -> '%s' failed. " "Cannot clean up the background chainstate leveldb directory.", fs::PathToString(p_old), fs::PathToString(p_new))); @@ -5757,7 +5758,7 @@ bool ChainstateManager::ValidatedSnapshotCleanup() } if (!DeleteCoinsDBFromDisk(tmp_old, /*is_snapshot=*/false)) { - // No need to AbortNode because once the unneeded bg chainstate data is + // No need to FatalError because once the unneeded bg chainstate data is // moved, it will not interfere with subsequent initialization. LogPrintf("Deletion of %s failed. Please remove it manually, as the " /* Continued */ "directory is now unnecessary.\n", diff --git a/src/validation.h b/src/validation.h index 8bc8842c54..66bc036e66 100644 --- a/src/validation.h +++ b/src/validation.h @@ -62,6 +62,9 @@ class SnapshotMetadata; namespace Consensus { struct Params; } // namespace Consensus +namespace util { +class SignalInterrupt; +} // namespace util /** Maximum number of dedicated script-checking threads allowed */ static const int MAX_SCRIPTCHECK_THREADS = 15; @@ -103,7 +106,7 @@ void StopScriptCheckWorkerThreads(); CAmount GetBlockSubsidy(int nHeight, const Consensus::Params& consensusParams); -bool AbortNode(BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = bilingual_str{}); +bool FatalError(kernel::Notifications& notifications, BlockValidationState& state, const std::string& strMessage, const bilingual_str& userMessage = {}); /** Guess verification progress (as a fraction between 0.0=genesis and 1.0=current tip). */ double GuessVerificationProgress(const ChainTxData& data, const CBlockIndex* pindex); @@ -959,7 +962,7 @@ private: public: using Options = kernel::ChainstateManagerOpts; - explicit ChainstateManager(Options options, node::BlockManager::Options blockman_options); + explicit ChainstateManager(const util::SignalInterrupt& interrupt, Options options, node::BlockManager::Options blockman_options); const CChainParams& GetParams() const { return m_options.chainparams; } const Consensus::Params& GetConsensus() const { return m_options.chainparams.GetConsensus(); } @@ -982,6 +985,7 @@ public: */ RecursiveMutex& GetMutex() const LOCK_RETURNED(::cs_main) { return ::cs_main; } + const util::SignalInterrupt& m_interrupt; const Options m_options; std::thread m_load_block; //! A single BlockManager instance is shared across each constructed @@ -1052,10 +1056,7 @@ public: //! If the coins match (expected), then mark the validation chainstate for //! deletion and continue using the snapshot chainstate as active. //! Otherwise, revert to using the ibd chainstate and shutdown. - SnapshotCompletionResult MaybeCompleteSnapshotValidation( - std::function shutdown_fnc = - [](bilingual_str msg) { AbortNode(msg.original, msg); }) - EXCLUSIVE_LOCKS_REQUIRED(::cs_main); + SnapshotCompletionResult MaybeCompleteSnapshotValidation() EXCLUSIVE_LOCKS_REQUIRED(::cs_main); //! The most-work chain. Chainstate& ActiveChainstate() const; diff --git a/test/lint/lint-format-strings.py b/test/lint/lint-format-strings.py index f54126e023..43addab2f3 100755 --- a/test/lint/lint-format-strings.py +++ b/test/lint/lint-format-strings.py @@ -16,7 +16,7 @@ import re import sys FUNCTION_NAMES_AND_NUMBER_OF_LEADING_ARGUMENTS = [ - 'FatalError,0', + 'FatalErrorf,0', 'fprintf,1', 'tfm::format,1', # Assuming tfm::::format(std::ostream&, ... 'LogConnectFailure,1', diff --git a/test/lint/run-lint-format-strings.py b/test/lint/run-lint-format-strings.py index d1896dba84..ed98b1b2f8 100755 --- a/test/lint/run-lint-format-strings.py +++ b/test/lint/run-lint-format-strings.py @@ -14,7 +14,8 @@ import sys FALSE_POSITIVES = [ ("src/dbwrapper.cpp", "vsnprintf(p, limit - p, format, backup_ap)"), - ("src/index/base.cpp", "FatalError(const char* fmt, const Args&... args)"), + ("src/index/base.cpp", "FatalErrorf(const char* fmt, const Args&... args)"), + ("src/index/base.h", "FatalErrorf(const char* fmt, const Args&... args)"), ("src/netbase.cpp", "LogConnectFailure(bool manual_connection, const char* fmt, const Args&... args)"), ("src/clientversion.cpp", "strprintf(_(COPYRIGHT_HOLDERS).translated, COPYRIGHT_HOLDERS_SUBSTITUTION)"), ("src/test/translation_tests.cpp", "strprintf(format, arg)"),