From 85aa8398f5d13c659299b81cdae377462b4f8316 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 6 Feb 2018 13:51:44 -0500 Subject: [PATCH 1/2] 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 --- src/validation.cpp | 9 +-------- 1 file changed, 1 insertion(+), 8 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 698ef9181d39b..a8b41d16e6a98 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -547,6 +547,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; @@ -581,8 +582,6 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool // Check for conflicts with in-memory transactions std::set setConflicts; - { - LOCK(pool.cs); // protect pool.mapNextTx for (const CTxIn &txin : tx.vin) { auto itConflicting = pool.mapNextTx.find(txin.prevout); @@ -623,15 +622,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); @@ -670,8 +666,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)); @@ -768,7 +762,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) { From 02fc8863630a20e75230f8bc3ba1051c480ae560 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Tue, 6 Feb 2018 14:55:36 -0500 Subject: [PATCH 2/2] Add braces to meet code style on line-after-the-one-changed. --- src/validation.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validation.cpp b/src/validation.cpp index a8b41d16e6a98..eb8599b7692ea 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -548,8 +548,9 @@ static bool AcceptToMemoryPoolWorker(const CChainParams& chainparams, CTxMemPool const uint256 hash = tx.GetHash(); AssertLockHeld(cs_main); LOCK(pool.cs); // mempool "read lock" (held through GetMainSignals().TransactionAddedToMempool()) - if (pfMissingInputs) + if (pfMissingInputs) { *pfMissingInputs = false; + } if (!CheckTransaction(tx, state)) return false; // state filled in by CheckTransaction