Skip to content

Commit

Permalink
Hold mempool.cs for the duration of ATMP.
Browse files Browse the repository at this point in the history
This resolves an issue where getrawmempool() can race mempool
notification signals. Intuitively we use mempool.cs as a "read
lock" on the mempool with cs_main being the write lock, so holding
the read lock intermittently while doing write operations is
somewhat strange.
This also avoids the introduction of cs_main in getrawmempool()
which reviewers objected to in the previous fix in bitcoin#12273

Github-Pull: bitcoin#12368
Rebased-From: 85aa839
Tree-SHA512: 90a505a96cecc065e8575d816f3bb35040df8672efc315f45eb3f2ea086e8ea6ee2c99eed03d0fe2215c8d3ee947a7b120e3c57a25185d03550c9075573ab032
  • Loading branch information
TheBlueMatt authored and Michael Polzer committed Mar 16, 2018
1 parent c0696a5 commit b0149c5
Showing 1 changed file with 1 addition and 8 deletions.
9 changes: 1 addition & 8 deletions src/validation.cpp
Expand Up @@ -559,6 +559,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
const CTransaction& tx = *ptx;
const uint256 hash = tx.GetHash();
AssertLockHeld(cs_main);
LOCK(pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool())
if (pfMissingInputs)
*pfMissingInputs = false;

Expand Down Expand Up @@ -615,8 +616,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool

// Check for conflicts with in-memory transactions
std::set<uint256> setConflicts;
{
LOCK(pool.cs); // protect pool.mapNextTx
for (const CTxIn &txin : tx.vin)
{
auto itConflicting = pool.mapNextTx.find(txin.prevout);
Expand Down Expand Up @@ -669,15 +668,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
}
}
}
}

{
CCoinsView dummy;
CCoinsViewCache view(&dummy);

LockPoints lp;
{
LOCK(pool.cs);
CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool);
view.SetBackend(viewMemPool);

Expand Down Expand Up @@ -716,8 +712,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");

} // end LOCK(pool.cs)

CAmount nFees = 0;
if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) {
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
Expand Down Expand Up @@ -814,7 +808,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// If we don't hold the lock allConflicting might be incomplete; the
// subsequent RemoveStaged() and addUnchecked() calls don't guarantee
// mempool consistency for us.
LOCK(pool.cs);
const bool fReplacementTransaction = setConflicts.size();
if (fReplacementTransaction)
{
Expand Down

0 comments on commit b0149c5

Please sign in to comment.