rpc: handle shutdown during long poll and wait methods

The waitTipChanged() now returns nullopt if the node is shutting down.

Previously it would return the last known tip during shutdown, but
this creates an ambiguous circumstance in the scenario where the
node is started and quickly shutdown, before notifications().TipBlock()
is set.

The getblocktemplate, waitfornewblock and waitforblockheight RPC
are updated to handle this. Existing behavior is preserved.

Co-authored-by: Ryan Ofsky <ryan@ofsky.org>
This commit is contained in:
Sjors Provoost 2025-02-19 10:28:43 +01:00
parent a3bf43343f
commit 64a2795fd4
No known key found for this signature in database
GPG Key ID: 57FF9BDBCC301009
4 changed files with 63 additions and 28 deletions

View File

@ -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<BlockRef> waitTipChanged(uint256 current_tip, MillisecondsDouble timeout = MillisecondsDouble::max()) = 0;
/**
* Construct a new block template

View File

@ -1070,22 +1070,34 @@ public:
return BlockRef{tip->GetBlockHash(), tip->nHeight};
}
BlockRef waitTipChanged(uint256 current_tip, MillisecondsDouble timeout) override
std::optional<BlockRef> 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<BlockTemplate> createNewBlock(const BlockCreateOptions& options) override

View File

@ -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<BlockRef> 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<BlockRef> 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<BlockRef> 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;
},
};

View File

@ -46,6 +46,7 @@
#include <memory>
#include <stdint.h>
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<BlockRef> 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)