Hold mempool.cs for the duration of ATMP.

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 #12273
This commit is contained in:
Matt Corallo 2018-02-06 13:51:44 -05:00
parent 1462bde767
commit 85aa8398f5

View file

@ -547,6 +547,7 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
const CTransaction& tx = *ptx; const CTransaction& tx = *ptx;
const uint256 hash = tx.GetHash(); const uint256 hash = tx.GetHash();
AssertLockHeld(cs_main); AssertLockHeld(cs_main);
LOCK(pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool())
if (pfMissingInputs) if (pfMissingInputs)
*pfMissingInputs = false; *pfMissingInputs = false;
@ -581,8 +582,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// Check for conflicts with in-memory transactions // Check for conflicts with in-memory transactions
std::set<uint256> setConflicts; std::set<uint256> setConflicts;
{
LOCK(pool.cs); // protect pool.mapNextTx
for (const CTxIn &txin : tx.vin) for (const CTxIn &txin : tx.vin)
{ {
auto itConflicting = pool.mapNextTx.find(txin.prevout); auto itConflicting = pool.mapNextTx.find(txin.prevout);
@ -623,15 +622,12 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
} }
} }
} }
}
{ {
CCoinsView dummy; CCoinsView dummy;
CCoinsViewCache view(&dummy); CCoinsViewCache view(&dummy);
LockPoints lp; LockPoints lp;
{
LOCK(pool.cs);
CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool); CCoinsViewMemPool viewMemPool(pcoinsTip.get(), pool);
view.SetBackend(viewMemPool); view.SetBackend(viewMemPool);
@ -670,8 +666,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp)) if (!CheckSequenceLocks(tx, STANDARD_LOCKTIME_VERIFY_FLAGS, &lp))
return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final"); return state.DoS(0, false, REJECT_NONSTANDARD, "non-BIP68-final");
} // end LOCK(pool.cs)
CAmount nFees = 0; CAmount nFees = 0;
if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) { if (!Consensus::CheckTxInputs(tx, state, view, GetSpendHeight(view), nFees)) {
return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state)); return error("%s: Consensus::CheckTxInputs: %s, %s", __func__, tx.GetHash().ToString(), FormatStateMessage(state));
@ -768,7 +762,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool
// If we don't hold the lock allConflicting might be incomplete; the // If we don't hold the lock allConflicting might be incomplete; the
// subsequent RemoveStaged() and addUnchecked() calls don't guarantee // subsequent RemoveStaged() and addUnchecked() calls don't guarantee
// mempool consistency for us. // mempool consistency for us.
LOCK(pool.cs);
const bool fReplacementTransaction = setConflicts.size(); const bool fReplacementTransaction = setConflicts.size();
if (fReplacementTransaction) if (fReplacementTransaction)
{ {