Give CBlockPolicyEstimator it's own lock

morcos · morcos · commit dbb9e3699b8e · 2017-04-10T13:51:51.000-04:00
diff --git a/src/policy/fees.cpp b/src/policy/fees.cpp
@@ -290,6 +290,7 @@ void TxConfirmStats::removeTx(unsigned int entryHeight, unsigned int nBestSeenHe
 // of no harm to try to remove them again.
 bool CBlockPolicyEstimator::removeTx(uint256 hash)
 {
+    LOCK(cs_feeEstimator);
     std::map<uint256, TxStatsInfo>::iterator pos = mapMemPoolTxs.find(hash);
     if (pos != mapMemPoolTxs.end()) {
         feeStats.removeTx(pos->second.blockHeight, nBestSeenHeight, pos->second.bucketIndex);
@@ -315,6 +316,7 @@ CBlockPolicyEstimator::CBlockPolicyEstimator()
 
 void CBlockPolicyEstimator::processTransaction(const CTxMemPoolEntry& entry, bool validFeeEstimate)
 {
+    LOCK(cs_feeEstimator);
     unsigned int txHeight = entry.GetHeight();
     uint256 hash = entry.GetTx().GetHash();
     if (mapMemPoolTxs.count(hash)) {
@@ -374,6 +376,7 @@ bool CBlockPolicyEstimator::processBlockTx(unsigned int nBlockHeight, const CTxM
 void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
                                          std::vector<const CTxMemPoolEntry*>& entries)
 {
+    LOCK(cs_feeEstimator);
     if (nBlockHeight <= nBestSeenHeight) {
         // Ignore side chains and re-orgs; assuming they are random
         // they don't affect the estimate.
@@ -410,6 +413,7 @@ void CBlockPolicyEstimator::processBlock(unsigned int nBlockHeight,
 
 CFeeRate CBlockPolicyEstimator::estimateFee(int confTarget)
 {
+    LOCK(cs_feeEstimator);
     // Return failure if trying to analyze a target we're not tracking
     // It's not possible to get reasonable estimates for confTarget of 1
     if (confTarget <= 1 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
@@ -427,18 +431,24 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun
 {
     if (answerFoundAtTarget)
         *answerFoundAtTarget = confTarget;
-    // Return failure if trying to analyze a target we're not tracking
-    if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
-        return CFeeRate(0);
-
-    // It's not possible to get reasonable estimates for confTarget of 1
-    if (confTarget == 1)
-        confTarget = 2;
 
     double median = -1;
-    while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) {
-        median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight);
-    }
+
+    {
+        LOCK(cs_feeEstimator);
+
+        // Return failure if trying to analyze a target we're not tracking
+        if (confTarget <= 0 || (unsigned int)confTarget > feeStats.GetMaxConfirms())
+            return CFeeRate(0);
+
+        // It's not possible to get reasonable estimates for confTarget of 1
+        if (confTarget == 1)
+            confTarget = 2;
+
+        while (median < 0 && (unsigned int)confTarget <= feeStats.GetMaxConfirms()) {
+            median = feeStats.EstimateMedianVal(confTarget++, SUFFICIENT_FEETXS, MIN_SUCCESS_PCT, true, nBestSeenHeight);
+        }
+    } // Must unlock cs_feeEstimator before taking mempool locks
 
     if (answerFoundAtTarget)
         *answerFoundAtTarget = confTarget - 1;
@@ -456,12 +466,14 @@ CFeeRate CBlockPolicyEstimator::estimateSmartFee(int confTarget, int *answerFoun
 
 void CBlockPolicyEstimator::Write(CAutoFile& fileout)
 {
+    LOCK(cs_feeEstimator);
     fileout << nBestSeenHeight;
     feeStats.Write(fileout);
 }
 
 void CBlockPolicyEstimator::Read(CAutoFile& filein, int nFileVersion)
 {
+    LOCK(cs_feeEstimator);
     int nFileBestSeenHeight;
     filein >> nFileBestSeenHeight;
     feeStats.Read(filein);
diff --git a/src/policy/fees.h b/src/policy/fees.h
@@ -8,6 +8,7 @@
 #include "amount.h"
 #include "uint256.h"
 #include "random.h"
+#include "sync.h"
 
 #include <map>
 #include <string>
@@ -249,6 +250,8 @@ class CBlockPolicyEstimator
     unsigned int trackedTxs;
     unsigned int untrackedTxs;
 
+    mutable CCriticalSection cs_feeEstimator;
+
     /** Process a transaction confirmed in a block*/
     bool processBlockTx(unsigned int nBlockHeight, const CTxMemPoolEntry* entry);
 
diff --git a/src/txmempool.cpp b/src/txmempool.cpp
@@ -845,20 +845,17 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const
 
 CFeeRate CTxMemPool::estimateFee(int nBlocks) const
 {
-    LOCK(cs);
     return minerPolicyEstimator->estimateFee(nBlocks);
 }
 CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const
 {
-    LOCK(cs);
     return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this);
 }
 
 bool
 CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const
 {
     try {
-        LOCK(cs);
         fileout << 139900; // version required to read: 0.13.99 or later
         fileout << CLIENT_VERSION; // version that wrote the file
         minerPolicyEstimator->Write(fileout);
@@ -878,7 +875,6 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein)
         filein >> nVersionRequired >> nVersionThatWrote;
         if (nVersionRequired > CLIENT_VERSION)
             return error("CTxMemPool::ReadFeeEstimates(): up-version (%d) fee estimate file", nVersionRequired);
-        LOCK(cs);
         minerPolicyEstimator->Read(filein, nVersionThatWrote);
     }
     catch (const std::exception&) {

Original file line number	Diff line number	Diff line change
`@@ -845,20 +845,17 @@ TxMempoolInfo CTxMemPool::info(const uint256& hash) const`
`845`	`845`
`846`	`846`	`CFeeRate CTxMemPool::estimateFee(int nBlocks) const`
`847`	`847`	`{`
`848`		`- LOCK(cs);`
`849`	`848`	`return minerPolicyEstimator->estimateFee(nBlocks);`
`850`	`849`	`}`
`851`	`850`	`CFeeRate CTxMemPool::estimateSmartFee(int nBlocks, int *answerFoundAtBlocks) const`
`852`	`851`	`{`
`853`		`- LOCK(cs);`
`854`	`852`	`return minerPolicyEstimator->estimateSmartFee(nBlocks, answerFoundAtBlocks, *this);`
`855`	`853`	`}`
`856`	`854`
`857`	`855`	`bool`
`858`	`856`	`CTxMemPool::WriteFeeEstimates(CAutoFile& fileout) const`
`859`	`857`	`{`
`860`	`858`	`try {`
`861`		`- LOCK(cs);`
`862`	`859`	`fileout << 139900; // version required to read: 0.13.99 or later`
`863`	`860`	`fileout << CLIENT_VERSION; // version that wrote the file`
`864`	`861`	`minerPolicyEstimator->Write(fileout);`
`@@ -878,7 +875,6 @@ CTxMemPool::ReadFeeEstimates(CAutoFile& filein)`
`878`	`875`	`filein >> nVersionRequired >> nVersionThatWrote;`
`879`	`876`	`if (nVersionRequired > CLIENT_VERSION)`
`880`	`877`	`return error("CTxMemPool::ReadFeeEstimates(): up-version (%d) fee estimate file", nVersionRequired);`
`881`		`- LOCK(cs);`
`882`	`878`	`minerPolicyEstimator->Read(filein, nVersionThatWrote);`
`883`	`879`	`}`
`884`	`880`	`catch (const std::exception&) {`