Fix missing locking in CTxMemPool::check(const CCoinsViewCache *pcoins):
mapTx requires holding mutex csmapNextTx requires holding mutex csnCheckFrequency requires holding mutex csFix missing locking in CTxMemPool::setSanityCheck(double dFrequency):
nCheckFrequency requires holding mutex cs
ACK 2abe610.
Unrelated, but should read nCheckFrequency with the lock too?
@promag Yes, nCheckFrequency should probably be guarded by the mutex cs too. Good catch!
I’ll ping in @MarcoFalke and @TheBlueMatt for a comment. What do you say? :-)
0$ git blame src/txmempool.h | grep nCheckFrequency
1fada0c42 (MarcoFalke 2016-04-03 11:49:36 +0200 415) uint32_t nCheckFrequency; //!< Value n means that n times in 2^32 we check.
253a6590f (Matt Corallo 2017-09-11 15:43:49 -0400 510) void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = static_cast<uint32_t>(dFrequency * 4294967295.0); }
3$ git grep -E '(nCheckFrequency|setSanityCheck)'
4src/init.cpp: mempool.setSanityCheck(1.0 / ratio);
5src/qt/test/rpcnestedtests.cpp: //mempool.setSanityCheck(1.0);
6src/test/test_bitcoin.cpp: mempool.setSanityCheck(1.0);
7src/txmempool.cpp: nCheckFrequency = 0;
8src/txmempool.cpp: if (nCheckFrequency != 0) assert(!coin.IsSpent());
9src/txmempool.cpp: if (nCheckFrequency == 0)
10src/txmempool.cpp: if (GetRand(std::numeric_limits<uint32_t>::max()) >= nCheckFrequency)
11src/txmempool.h: uint32_t nCheckFrequency; //!< Value n means that n times in 2^32 we check.
12src/txmempool.h: void setSanityCheck(double dFrequency = 1.0) { nCheckFrequency = static_cast<uint32_t>(dFrequency * 4294967295.0); }
Actually nCheckFrequency doesn’t change, only set in
0src/init.cpp: mempool.setSanityCheck(1.0 / ratio);
nCheckFrequency too. Please re-review :-)
Updated with annotations moved over to the header files.
The EXCLUSIVE_LOCKS_REQUIRED(mempool.cs) annotations for BlockAssembler::SkipMapTxEntry and BlockAssembler::addPackageTxs should be placed in miner.h rather than miner.cpp, but the existence of mempool (and hence mempool.cs) is not currently not known in miner.h. Would extern CTxMemPool mempool; be acceptable in miner.h?
SkipMapTxEntry, addPackageTxs, UpdateChildrenForRemoval and CalculateDescendants. Now fixed! Please re-review :-)
* reading variable 'mapTx' requires holding mutex 'cs'
* reading variable 'mapNextTx' requires holding mutex 'cs'
* reading variable 'nCheckFrequency' requires holding mutex 'cs'
* writing variable 'nCheckFrequency' requires holding mutex 'cs'
