diff --git a/src/interfaces/mining.h b/src/interfaces/mining.h index 249b9cefd4a..1e84969bc78 100644 --- a/src/interfaces/mining.h +++ b/src/interfaces/mining.h @@ -89,15 +89,16 @@ public: /** * Waits for the connected tip to change. During node initialization, this will - * wait until the tip is connected. + * wait until the tip is connected (regardless of `timeout`). * * @param[in] current_tip block hash of the current chain tip. Function waits * for the chain tip to differ from this. * @param[in] timeout how long to wait for a new tip (default is forever) * - * @returns Hash and height of the current chain tip after this call. + * @retval BlockRef hash and height of the current chain tip after this call. + * @retval std::nullopt if the node is shut down. */ - virtual BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0; + virtual std::optional waitTipChanged(uint256 current_tip, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0; /** * Construct a new block template diff --git a/src/node/interfaces.cpp b/src/node/interfaces.cpp index 59639414212..e2507211327 100644 --- a/src/node/interfaces.cpp +++ b/src/node/interfaces.cpp @@ -1070,22 +1070,34 @@ public: return BlockRef{tip->GetBlockHash(), tip->nHeight}; } - BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override + std::optional waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override { Assume(timeout >= 0ms); // No internal callers should use a negative timeout if (timeout < 0ms) timeout = 0ms; if (timeout > std::chrono::years{100}) timeout = std::chrono::years{100}; // Upper bound to avoid UB in std::chrono + auto deadline{std::chrono::steady_clock::now() + timeout}; { WAIT_LOCK(notifications().m_tip_block_mutex, lock); - notifications().m_tip_block_cv.wait_for(lock, timeout, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { - // We need to wait for m_tip_block to be set AND for the value - // to differ from the current_tip value. - return (notifications().TipBlock() && notifications().TipBlock() != current_tip) || chainman().m_interrupt; + // For callers convenience, wait longer than the provided timeout + // during startup for the tip to be non-null. That way this function + // always returns valid tip information when possible and only + // returns null when shutting down, not when timing out. + notifications().m_tip_block_cv.wait(lock, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { + return notifications().TipBlock() || chainman().m_interrupt; + }); + if (chainman().m_interrupt) return {}; + // At this point TipBlock is set, so continue to wait until it is + // different then `current_tip` provided by caller. + notifications().m_tip_block_cv.wait_until(lock, deadline, [&]() EXCLUSIVE_LOCKS_REQUIRED(notifications().m_tip_block_mutex) { + return Assume(notifications().TipBlock()) != current_tip || chainman().m_interrupt; }); } - // Must release m_tip_block_mutex before locking cs_main, to avoid deadlocks. - LOCK(::cs_main); - return BlockRef{chainman().ActiveChain().Tip()->GetBlockHash(), chainman().ActiveChain().Tip()->nHeight}; + + if (chainman().m_interrupt) return {}; + + // Must release m_tip_block_mutex before getTip() locks cs_main, to + // avoid deadlocks. + return getTip(); } std::unique_ptr createNewBlock(const BlockCreateOptions& options) override diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp index f7f082af39a..6e5c656f3d8 100644 --- a/src/rpc/blockchain.cpp +++ b/src/rpc/blockchain.cpp @@ -64,6 +64,7 @@ using kernel::CCoinsStats; using kernel::CoinStatsHashType; +using interfaces::BlockRef; using interfaces::Mining; using node::BlockManager; using node::NodeContext; @@ -286,12 +287,17 @@ static RPCHelpMan waitfornewblock() NodeContext& node = EnsureAnyNodeContext(request.context); Mining& miner = EnsureMining(node); - auto block{CHECK_NONFATAL(miner.getTip()).value()}; - block = timeout ? miner.waitTipChanged(block.hash, std::chrono::milliseconds(timeout)) : miner.waitTipChanged(block.hash); + // Abort if RPC came out of warmup too early + BlockRef current_block{CHECK_NONFATAL(miner.getTip()).value()}; + std::optional block = timeout ? miner.waitTipChanged(current_block.hash, std::chrono::milliseconds(timeout)) : + miner.waitTipChanged(current_block.hash); + + // Return current block upon shutdown + if (block) current_block = *block; UniValue ret(UniValue::VOBJ); - ret.pushKV("hash", block.hash.GetHex()); - ret.pushKV("height", block.height); + ret.pushKV("hash", current_block.hash.GetHex()); + ret.pushKV("height", current_block.height); return ret; }, }; @@ -330,22 +336,28 @@ static RPCHelpMan waitforblock() NodeContext& node = EnsureAnyNodeContext(request.context); Mining& miner = EnsureMining(node); - auto block{CHECK_NONFATAL(miner.getTip()).value()}; + // Abort if RPC came out of warmup too early + BlockRef current_block{CHECK_NONFATAL(miner.getTip()).value()}; + const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout}; - while (block.hash != hash) { + while (current_block.hash != hash) { + std::optional block; if (timeout) { auto now{std::chrono::steady_clock::now()}; if (now >= deadline) break; const MillisecondsDouble remaining{deadline - now}; - block = miner.waitTipChanged(block.hash, remaining); + block = miner.waitTipChanged(current_block.hash, remaining); } else { - block = miner.waitTipChanged(block.hash); + block = miner.waitTipChanged(current_block.hash); } + // Return current block upon shutdown + if (!block) break; + current_block = *block; } UniValue ret(UniValue::VOBJ); - ret.pushKV("hash", block.hash.GetHex()); - ret.pushKV("height", block.height); + ret.pushKV("hash", current_block.hash.GetHex()); + ret.pushKV("height", current_block.height); return ret; }, }; @@ -385,23 +397,29 @@ static RPCHelpMan waitforblockheight() NodeContext& node = EnsureAnyNodeContext(request.context); Mining& miner = EnsureMining(node); - auto block{CHECK_NONFATAL(miner.getTip()).value()}; + // Abort if RPC came out of warmup too early + BlockRef current_block{CHECK_NONFATAL(miner.getTip()).value()}; + const auto deadline{std::chrono::steady_clock::now() + 1ms * timeout}; - while (block.height < height) { + while (current_block.height < height) { + std::optional block; if (timeout) { auto now{std::chrono::steady_clock::now()}; if (now >= deadline) break; const MillisecondsDouble remaining{deadline - now}; - block = miner.waitTipChanged(block.hash, remaining); + block = miner.waitTipChanged(current_block.hash, remaining); } else { - block = miner.waitTipChanged(block.hash); + block = miner.waitTipChanged(current_block.hash); } + // Return current block on shutdown + if (!block) break; + current_block = *block; } UniValue ret(UniValue::VOBJ); - ret.pushKV("hash", block.hash.GetHex()); - ret.pushKV("height", block.height); + ret.pushKV("hash", current_block.hash.GetHex()); + ret.pushKV("height", current_block.height); return ret; }, }; diff --git a/src/rpc/mining.cpp b/src/rpc/mining.cpp index b527e686a46..ada1a153214 100644 --- a/src/rpc/mining.cpp +++ b/src/rpc/mining.cpp @@ -46,6 +46,7 @@ #include #include +using interfaces::BlockRef; using interfaces::BlockTemplate; using interfaces::Mining; using node::BlockAssembler; @@ -801,7 +802,10 @@ static RPCHelpMan getblocktemplate() { MillisecondsDouble checktxtime{std::chrono::minutes(1)}; while (tip == hashWatchedChain && IsRPCRunning()) { - tip = miner.waitTipChanged(hashWatchedChain, checktxtime).hash; + std::optional maybe_tip{miner.waitTipChanged(hashWatchedChain, checktxtime)}; + // Node is shutting down + if (!maybe_tip) break; + tip = maybe_tip->hash; // Timeout: Check transactions for update // without holding the mempool lock to avoid deadlocks if (mempool.GetTransactionsUpdated() != nTransactionsUpdatedLastLP)