refactor: Add thread safety annotations to Mempool{Info}ToJSON()

hebasto · hebasto · commit 09b1ef2262e7 · 2020-12-07T16:02:52.000+02:00
This change avoids locking CTxMemPool::cs recursively in
Mempool{Info}ToJSON() functions.
diff --git a/src/bench/rpc_mempool.cpp b/src/bench/rpc_mempool.cpp
@@ -18,6 +18,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& poo
 static void RpcMempool(benchmark::Bench& bench)
 {
     CTxMemPool pool;
+    {
     LOCK2(cs_main, pool.cs);
 
     for (int i = 0; i < 1000; ++i) {
@@ -31,6 +32,7 @@ static void RpcMempool(benchmark::Bench& bench)
         const CTransactionRef tx_r{MakeTransactionRef(tx)};
         AddTx(tx_r, /* fee */ i, pool);
     }
+    }
 
     bench.run([&] {
         (void)MempoolToJSON(pool, /*verbose*/ true);
diff --git a/src/rpc/blockchain.cpp b/src/rpc/blockchain.cpp
@@ -512,6 +512,8 @@ static void entryToJSON(const CTxMemPool& pool, UniValue& info, const CTxMemPool
 
 UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose, bool include_mempool_sequence)
 {
+    AssertLockNotHeld(pool.cs);
+
     if (verbose) {
         if (include_mempool_sequence) {
             throw JSONRPCError(RPC_INVALID_PARAMETER, "Verbose results cannot contain mempool sequence values.");
@@ -1487,6 +1489,8 @@ static RPCHelpMan getchaintips()
 
 UniValue MempoolInfoToJSON(const CTxMemPool& pool)
 {
+    AssertLockNotHeld(pool.cs);
+
     // Make sure this call is atomic in the pool.
     LOCK(pool.cs);
     UniValue ret(UniValue::VOBJ);
diff --git a/src/rpc/blockchain.h b/src/rpc/blockchain.h
@@ -7,6 +7,7 @@
 
 #include <amount.h>
 #include <sync.h>
+#include <txmempool.h>
 
 #include <stdint.h>
 #include <vector>
@@ -16,7 +17,6 @@ extern RecursiveMutex cs_main;
 class CBlock;
 class CBlockIndex;
 class CBlockPolicyEstimator;
-class CTxMemPool;
 class ChainstateManager;
 class UniValue;
 struct NodeContext;
@@ -41,10 +41,10 @@ void RPCNotifyBlockChange(const CBlockIndex*);
 UniValue blockToJSON(const CBlock& block, const CBlockIndex* tip, const CBlockIndex* blockindex, bool txDetails = false) LOCKS_EXCLUDED(cs_main);
 
 /** Mempool information to JSON */
-UniValue MempoolInfoToJSON(const CTxMemPool& pool);
+UniValue MempoolInfoToJSON(const CTxMemPool& pool) EXCLUSIVE_LOCKS_REQUIRED(!pool.cs);
 
 /** Mempool to JSON */
-UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose = false, bool include_mempool_sequence = false);
+UniValue MempoolToJSON(const CTxMemPool& pool, bool verbose = false, bool include_mempool_sequence = false) EXCLUSIVE_LOCKS_REQUIRED(!pool.cs);
 
 /** Block header to JSON */
 UniValue blockheaderToJSON(const CBlockIndex* tip, const CBlockIndex* blockindex) LOCKS_EXCLUDED(cs_main);

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,7 @@ static void AddTx(const CTransactionRef& tx, const CAmount& fee, CTxMemPool& poo`
`18`	`18`	`static void RpcMempool(benchmark::Bench& bench)`
`19`	`19`	`{`
`20`	`20`	`CTxMemPool pool;`
	`21`	`+ {`
`21`	`22`	`LOCK2(cs_main, pool.cs);`
`22`	`23`
`23`	`24`	`for (int i = 0; i < 1000; ++i) {`
`@@ -31,6 +32,7 @@ static void RpcMempool(benchmark::Bench& bench)`
`31`	`32`	`const CTransactionRef tx_r{MakeTransactionRef(tx)};`
`32`	`33`	`AddTx(tx_r, /* fee */ i, pool);`
`33`	`34`	`}`
	`35`	`+ }`
`34`	`36`
`35`	`37`	`bench.run([&] {`
`36`	`38`	`(void)MempoolToJSON(pool, /verbose/ true);`