From 6bc5b7100b1182c4c060c045df9a58dada3d3eab Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 14 Nov 2017 23:35:42 +0100 Subject: Fix missing locking in CTxMemPool::check(const CCoinsViewCache *pcoins) * reading variable 'mapTx' requires holding mutex 'cs' * reading variable 'mapNextTx' requires holding mutex 'cs' * reading variable 'nCheckFrequency' requires holding mutex 'cs' --- src/txmempool.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/txmempool.cpp b/src/txmempool.cpp index d03429ca81..bb585fc075 100644 --- a/src/txmempool.cpp +++ b/src/txmempool.cpp @@ -618,6 +618,7 @@ static void CheckInputsAndUpdateCoins(const CTransaction& tx, CCoinsViewCache& m void CTxMemPool::check(const CCoinsViewCache *pcoins) const { + LOCK(cs); if (nCheckFrequency == 0) return; @@ -632,7 +633,6 @@ void CTxMemPool::check(const CCoinsViewCache *pcoins) const CCoinsViewCache mempoolDuplicate(const_cast(pcoins)); const int64_t spendheight = GetSpendHeight(mempoolDuplicate); - LOCK(cs); std::list waitingOnDependants; for (indexed_transaction_set::const_iterator it = mapTx.begin(); it != mapTx.end(); it++) { unsigned int i = 0; -- cgit v1.2.3 From 0e2dfa8a65091504b27a0b9d66ee4415fe2b7b37 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Thu, 16 Nov 2017 09:54:10 +0100 Subject: Fix missing locking in CTxMemPool::setSanityCheck(double dFrequency) * writing variable 'nCheckFrequency' requires holding mutex 'cs' --- src/txmempool.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src') diff --git a/src/txmempool.h b/src/txmempool.h index 3f9fb4850c..4d06d5bcf9 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -529,7 +529,7 @@ public: * check does nothing. */ void check(const CCoinsViewCache *pcoins) const; - void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = static_cast(dFrequency * 4294967295.0); } + void setSanityCheck(double dFrequency = 1.0) { LOCK(cs); nCheckFrequency = static_cast(dFrequency * 4294967295.0); } // addUnchecked must updated state for all ancestors of a given transaction, // to track size/count of descendant transactions. First version of -- cgit v1.2.3 From 47782b49e67599585cd766c8322ca01764fe5aa7 Mon Sep 17 00:00:00 2001 From: practicalswift Date: Tue, 13 Mar 2018 00:05:53 +0100 Subject: Add Clang thread safety analysis annotations --- src/miner.cpp | 1 - src/miner.h | 7 ++++--- src/test/mempool_tests.cpp | 2 +- src/txmempool.h | 26 +++++++++++++------------- 4 files changed, 18 insertions(+), 18 deletions(-) (limited to 'src') diff --git a/src/miner.cpp b/src/miner.cpp index 0660df928c..d4527a1d67 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -14,7 +14,6 @@ #include #include #include -#include #include #include #include diff --git a/src/miner.h b/src/miner.h index 33a22ba75f..ed1b4434f9 100644 --- a/src/miner.h +++ b/src/miner.h @@ -8,6 +8,7 @@ #include #include +#include #include #include @@ -169,7 +170,7 @@ private: /** Add transactions based on feerate including unconfirmed ancestors * Increments nPackagesSelected / nDescendantsUpdated with corresponding * statistics from the package selection (for logging statistics). */ - void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated); + void addPackageTxs(int &nPackagesSelected, int &nDescendantsUpdated) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); // helper functions for addPackageTxs() /** Remove confirmed (inBlock) entries from given set */ @@ -183,13 +184,13 @@ private: bool TestPackageTransactions(const CTxMemPool::setEntries& package); /** Return true if given transaction from mapTx has already been evaluated, * or if the transaction's cached data in mapTx is incorrect. */ - bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx); + bool SkipMapTxEntry(CTxMemPool::txiter it, indexed_modified_transaction_set &mapModifiedTx, CTxMemPool::setEntries &failedTx) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); /** Sort the package in an order that is valid to appear in a block */ void SortForBlock(const CTxMemPool::setEntries& package, std::vector& sortedEntries); /** Add descendants of given transactions to mapModifiedTx with ancestor * state updated assuming given transactions are inBlock. Returns number * of updated descendants. */ - int UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set &mapModifiedTx); + int UpdatePackagesForAdded(const CTxMemPool::setEntries& alreadyAdded, indexed_modified_transaction_set &mapModifiedTx) EXCLUSIVE_LOCKS_REQUIRED(mempool.cs); }; /** Modify the extranonce in a block */ diff --git a/src/test/mempool_tests.cpp b/src/test/mempool_tests.cpp index c4b18151a7..5ca243f42e 100644 --- a/src/test/mempool_tests.cpp +++ b/src/test/mempool_tests.cpp @@ -106,7 +106,7 @@ BOOST_AUTO_TEST_CASE(MempoolRemoveTest) } template -static void CheckSort(CTxMemPool &pool, std::vector &sortedOrder) +static void CheckSort(CTxMemPool &pool, std::vector &sortedOrder) EXCLUSIVE_LOCKS_REQUIRED(pool.cs) { BOOST_CHECK_EQUAL(pool.size(), sortedOrder.size()); typename CTxMemPool::indexed_transaction_set::index::type::iterator it = pool.mapTx.get().begin(); diff --git a/src/txmempool.h b/src/txmempool.h index 4d06d5bcf9..ca7b1cd4be 100644 --- a/src/txmempool.h +++ b/src/txmempool.h @@ -440,7 +440,7 @@ public: class CTxMemPool { private: - uint32_t nCheckFrequency; //!< Value n means that n times in 2^32 we check. + uint32_t nCheckFrequency GUARDED_BY(cs); //!< Value n means that n times in 2^32 we check. unsigned int nTransactionsUpdated; //!< Used by getblocktemplate to trigger CreateNewBlock() invocation CBlockPolicyEstimator* minerPolicyEstimator; @@ -484,7 +484,7 @@ public: > indexed_transaction_set; mutable CCriticalSection cs; - indexed_transaction_set mapTx; + indexed_transaction_set mapTx GUARDED_BY(cs); typedef indexed_transaction_set::nth_index<0>::type::iterator txiter; std::vector > vTxHashes; //!< All tx witness hashes/entries in mapTx, in random order @@ -496,8 +496,8 @@ public: }; typedef std::set setEntries; - const setEntries & GetMemPoolParents(txiter entry) const; - const setEntries & GetMemPoolChildren(txiter entry) const; + const setEntries & GetMemPoolParents(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); + const setEntries & GetMemPoolChildren(txiter entry) const EXCLUSIVE_LOCKS_REQUIRED(cs); private: typedef std::map cacheMap; @@ -515,7 +515,7 @@ private: std::vector GetSortedDepthAndScore() const EXCLUSIVE_LOCKS_REQUIRED(cs); public: - indirectmap mapNextTx; + indirectmap mapNextTx GUARDED_BY(cs); std::map mapDeltas; /** Create a new CTxMemPool. @@ -547,7 +547,7 @@ public: void removeForBlock(const std::vector& vtx, unsigned int nBlockHeight); void clear(); - void _clear(); //lock free + void _clear() EXCLUSIVE_LOCKS_REQUIRED(cs); //lock free bool CompareDepthAndScore(const uint256& hasha, const uint256& hashb); void queryHashes(std::vector& vtxid); bool isSpent(const COutPoint& outpoint) const; @@ -600,7 +600,7 @@ public: /** Populate setDescendants with all in-mempool descendants of hash. * Assumes that setDescendants includes all in-mempool descendants of anything * already in it. */ - void CalculateDescendants(txiter it, setEntries& setDescendants) const; + void CalculateDescendants(txiter it, setEntries& setDescendants) const EXCLUSIVE_LOCKS_REQUIRED(cs); /** The minimum fee to get into the mempool, which may itself not be enough * for larger-sized transactions. @@ -665,17 +665,17 @@ private: */ void UpdateForDescendants(txiter updateIt, cacheMap &cachedDescendants, - const std::set &setExclude); + const std::set &setExclude) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Update ancestors of hash to add/remove it as a descendant transaction. */ - void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors); + void UpdateAncestorsOf(bool add, txiter hash, setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Set ancestor state for an entry */ - void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors); + void UpdateEntryForAncestors(txiter it, const setEntries &setAncestors) EXCLUSIVE_LOCKS_REQUIRED(cs); /** For each transaction being removed, update ancestors and any direct children. * If updateDescendants is true, then also update in-mempool descendants' * ancestor state. */ - void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants); + void UpdateForRemoveFromMempool(const setEntries &entriesToRemove, bool updateDescendants) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Sever link between specified transaction and direct children. */ - void UpdateChildrenForRemoval(txiter entry); + void UpdateChildrenForRemoval(txiter entry) EXCLUSIVE_LOCKS_REQUIRED(cs); /** Before calling removeUnchecked for a given transaction, * UpdateForRemoveFromMempool must be called on the entire (dependent) set @@ -685,7 +685,7 @@ private: * transactions in a chain before we've updated all the state for the * removal. */ - void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN); + void removeUnchecked(txiter entry, MemPoolRemovalReason reason = MemPoolRemovalReason::UNKNOWN) EXCLUSIVE_LOCKS_REQUIRED(cs); }; /** -- cgit v1.2.3