DragonX/dragonx
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 3 Wiki Activity

miner: fix cs_main/mempool.cs lock leak on the isStake error paths

Browse Source 
CreateNewBlock takes cs_main and mempool.cs unconditionally via
ENTER_CRITICAL_SECTION, and releases them on every return path. Two of those
LEAVE pairs — the notary-pay failure and the TestBlockValidity failure — were
wrapped in `if (!isStake) { LEAVE; LEAVE; }` but still `return(0)`
afterwards, so when isStake is true the function returned with both locks
still held: a lock leak that deadlocks the next cs_main acquirer. The success
and timelock return paths already release unconditionally, so the guard was
simply wrong.

Make both LEAVE pairs unconditional to match the ENTER. Behavior-identical on
DragonX (a RandomX chain where staking/LWMAPOS is off, so isStake is always
false and the LEAVE already ran), and correct for both isStake values.

Verified: a node self-mined thousands of blocks across two mining threads
(each block ENTER/LEAVEs the locks in CreateNewBlock) with height rising
continuously and verifychain=true — a leak would have deadlocked immediately.

The fuller RAII conversion (replace the manual ENTER/LEAVE with a scoped
LOCK2 for exception safety) remains a worthwhile follow-up, but is a larger
change to cs_main handling best done under its own review.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
DanS
2026-08-27 16:05:10 -05:00
parent 81f803948c
commit c92e69a13f
1 changed files with 10 additions and 10 deletions
Download Patch File Download Diff File Expand all files Collapse all files

12
src/miner.cpp
View File

@@ -677,11 +677,11 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32
if ( totalsats == 0 ) if ( totalsats == 0 )
{ {
LogPrintf("Could not create notary payment, trying again.\n"); LogPrintf("Could not create notary payment, trying again.\n");
if ( !isStake ) // Release unconditionally to match the unconditional ENTER above. The old
{ // `if(!isStake)` guard leaked cs_main/mempool.cs on the isStake path (this
// still return(0)s), while the success and timelock paths always release.
LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(cs_main);
LEAVE_CRITICAL_SECTION(mempool.cs); LEAVE_CRITICAL_SECTION(mempool.cs);
}
return(0); return(0);
} }
} else LogPrintf("vout 2 of notarization is not OP_RETURN scriptlen.%i\n", scriptlen); } else LogPrintf("vout 2 of notarization is not OP_RETURN scriptlen.%i\n", scriptlen);
@@ -726,11 +726,11 @@ CBlockTemplate* CreateNewBlock(CPubKey _pk,const CScript& _scriptPubKeyIn, int32
CValidationState state; CValidationState state;
if ( !TestBlockValidity(state, *pblock, pindexPrev, false, false)) // invokes CC checks if ( !TestBlockValidity(state, *pblock, pindexPrev, false, false)) // invokes CC checks
{ {
if ( !isStake ) // Release unconditionally to match the unconditional ENTER above. The old
{ // `if(!isStake)` guard leaked cs_main/mempool.cs on the isStake path (this
// still return(0)s), while the success and timelock paths always release.
LEAVE_CRITICAL_SECTION(cs_main); LEAVE_CRITICAL_SECTION(cs_main);
LEAVE_CRITICAL_SECTION(mempool.cs); LEAVE_CRITICAL_SECTION(mempool.cs);
}
LogPrintf("%s: TestBlockValidity failed!\n", __func__); LogPrintf("%s: TestBlockValidity failed!\n", __func__);
//throw std::runtime_error("CreateNewBlock(): TestBlockValidity failed"); // crashes the node, moved to GetBlockTemplate and issue return. //throw std::runtime_error("CreateNewBlock(): TestBlockValidity failed"); // crashes the node, moved to GetBlockTemplate and issue return.
return(0); return(0);