ARCHIVE/xahaud
3
0
Fork 0
mirror of https://github.com/Xahau/xahaud.git synced 2026-09-28 16:08:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(consensus): hold the consensus mutex around accept-time extension prep

Browse Source 
doAccept reacquires the consensus mutex for the live-build view, the
ordering salt, and the replay or pre-build block. Ledger building,
open-ledger locks, and endConsensus stay outside it.
This commit is contained in:
Nicholas Dudfield
2026-09-23 09:17:16 +07:00
parent bc5b90282a
commit 04a1d8a7b1
2 changed files with 43 additions and 27 deletions
Download Patch File Download Diff File Expand all files Collapse all files

52
src/xrpld/app/consensus/RCLConsensus.cpp
View File

@@ -77,6 +77,7 @@ RCLConsensus::RCLConsensus(
beast::Journal journal)
: adaptor_(
app,
mutex_,
std::move(feeVote),
ledgerMaster,
localTxs,
@@ -92,6 +93,7 @@ RCLConsensus::~RCLConsensus() = default;
RCLConsensus::Adaptor::Adaptor(
Application& app,
std::recursive_mutex& consensusMutex,
std::unique_ptr<FeeVote>&& feeVote,
LedgerMaster& ledgerMaster,
LocalTxs& localTxs,
@@ -99,6 +101,7 @@ RCLConsensus::Adaptor::Adaptor(
ValidatorKeys const& validatorKeys,
beast::Journal journal)
: app_(app)
, consensusMutex_(consensusMutex)
, feeVote_(std::move(feeVote))
, ledgerMaster_(ledgerMaster)
, localTxs_(localTxs)
@@ -518,11 +521,9 @@ RCLConsensus::Adaptor::onAccept(
"acceptLedger",
[=, this, cj = std::move(consensusJson)]() mutable {
//@@start do-accept-freeze-contract
// Note that no lock is held or acquired during this job.
// This is because generic Consensus guarantees that once a ledger
// is accepted, the consensus results and capture by reference state
// will not change until startRound is called (which happens via
// endConsensus).
// The job locks only for extension preparation, not ledger
// building. The accepted result must remain frozen until
// startRound, reached through endConsensus after doAccept returns.
//@@end do-accept-freeze-contract
RclConsensusLogger clog("onAccept", validating, j_);
this->doAccept(
@@ -587,10 +588,12 @@ RCLConsensus::Adaptor::doAccept(
// influence fallback entropy, transaction ordering, or ledger state.
auto replayData = ledgerMaster_.releaseReplay();
auto const consensusTxSetHash = result.txns.id();
auto const liveBuild = replayData
? std::optional<ConsensusExtensions::LiveBuildTxSet>{}
: std::optional<ConsensusExtensions::LiveBuildTxSet>{
ce().makeLiveBuildTxSet(result.txns)};
auto const liveBuild = [&] {
std::lock_guard lock{consensusMutex_};
return replayData ? std::optional<ConsensusExtensions::LiveBuildTxSet>{}
: std::optional<ConsensusExtensions::LiveBuildTxSet>{
ce().makeLiveBuildTxSet(result.txns)};
}();
auto const& buildTxs = liveBuild ? liveBuild->txns : result.txns;
auto const buildTxSetHash = buildTxs.id();
@@ -615,7 +618,11 @@ RCLConsensus::Adaptor::doAccept(
// FIXME: Use a std::vector and a custom sorter instead of CanonicalTXSet?
//@@start txn-ordering-salt-build-inputs
auto const buildSeq = prevLedger.seq() + 1;
CanonicalTXSet retriableTxs{ce().txnOrderingSalt(buildTxSetHash, buildSeq)};
auto const orderingSalt = [&] {
std::lock_guard lock{consensusMutex_};
return ce().txnOrderingSalt(buildTxSetHash, buildSeq);
}();
CanonicalTXSet retriableTxs{orderingSalt};
JLOG(j_.debug()) << "Building canonical tx set: " << retriableTxs.key();
@@ -641,17 +648,22 @@ RCLConsensus::Adaptor::doAccept(
// Export witness injection are independently gated inside onPreBuild;
// export-only rounds still need this hook even when RNG is off.
//@@start accept-time-cleanup-disabled
if (replayData)
{
ce().onReplayBuild();
}
else if (ce().rngEnabled() || ce().exportEnabled())
{
ce().onPreBuild(retriableTxs, buildSeq, buildTxSetHash);
}
else
{
ce().clearRngState();
// Match consensus-side readers/writers. Never extend this scope across
// buildLCL, the open-ledger locks, or endConsensus.
std::lock_guard lock{consensusMutex_};
if (replayData)
{
ce().onReplayBuild();
}
else if (ce().rngEnabled() || ce().exportEnabled())
{
ce().onPreBuild(retriableTxs, buildSeq, buildTxSetHash);
}
else
{
ce().clearRngState();
}
}
//@@end accept-time-cleanup-disabled

18
src/xrpld/app/consensus/RCLConsensus.h
View File

@@ -69,6 +69,7 @@ class RCLConsensus
class Adaptor
{
Application& app_;
std::recursive_mutex& consensusMutex_;
std::unique_ptr<FeeVote> feeVote_;
LedgerMaster& ledgerMaster_;
LocalTxs& localTxs_;
@@ -114,6 +115,7 @@ class RCLConsensus
Adaptor(
Application& app,
std::recursive_mutex& consensusMutex,
std::unique_ptr<FeeVote>&& feeVote,
LedgerMaster& ledgerMaster,
LocalTxs& localTxs,
@@ -210,10 +212,10 @@ class RCLConsensus
// Consensus<Adaptor> methods and since RCLConsensus::consensus_ should
// only be accessed under lock, these will only be called under lock.
//
// In general, the idea is that there is only ONE thread that is running
// consensus code at anytime. The only special case is the dispatched
// onAccept call, which does not take a lock and relies on Consensus not
// changing state until a future call to startRound.
// Normally only one thread runs consensus code at a time. The
// dispatched accept job builds the ledger outside the lock, but
// reacquires it for extension preparation. The accepted result must
// still remain unchanged until a future call to startRound.
friend class Consensus<Adaptor>;
/** Attempt to acquire a specific ledger.
@@ -552,9 +554,11 @@ public:
}
private:
// Since Consensus does not provide intrinsic thread-safety, this mutex
// guards all calls to consensus_. adaptor_ uses atomics internally
// to allow concurrent access of its data members that have getters.
// Guards mutable consensus state and accept-job round-state access.
// Atomic-only status polls and the extension's independently synchronized
// cross-thread APIs are exempt. Constructed before adaptor_.
// Lock order: C before LedgerMaster, never the reverse; C before busyMu_
// before the collector.
mutable std::recursive_mutex mutex_;
Adaptor adaptor_;