Merge #11623: tests: Add missing locks to tests

109a85899 tests: Add missing locks to tests (practicalswift)

Pull request description:

  Add missing locks to tests to satisfy lock requirements (such as `EXCLUSIVE_LOCKS_REQUIRED(...)` (Clang Thread Safety Analysis, see #11226), `AssertLockHeld(...)` and implicit lock assumptions).

Tree-SHA512: 1aaeb1da89df1779f02fcceff9d2f8ea24a3926d421f9ea305a19be04dd0b3e63d91f6c1ed22fb7e6988343f6a5288829a387ef872cfa7b6add57bd01046b5d9

1 files changed, 37 insertions, 7 deletions

diff --git a/src/test/DoS_tests.cpp b/src/test/DoS_tests.cpp
index d1f9e63ecf..bf6f2533df 100644
--- a/src/test/DoS_tests.cpp
+++ b/src/test/DoS_tests.cpp
@@ -66,11 +66,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
     dummyNode1.fSuccessfullyConnected = true;
 
     // This test requires that we have a chain with non-zero work.
+    LOCK(cs_main);
     BOOST_CHECK(chainActive.Tip() != nullptr);
     BOOST_CHECK(chainActive.Tip()->nChainWork > 0);
 
     // Test starts here
+    LOCK(dummyNode1.cs_sendProcessing);
     peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
+    LOCK(dummyNode1.cs_vSend);
     BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
     dummyNode1.vSendMsg.clear();
 
@@ -183,7 +186,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     peerLogic->InitializeNode(&dummyNode1);
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode1.GetId(), 100); // Should get banned
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 100); // Should get banned
+    }
+    LOCK(dummyNode1.cs_sendProcessing);
     peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr1));
     BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
@@ -194,11 +201,18 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     peerLogic->InitializeNode(&dummyNode2);
     dummyNode2.nVersion = 1;
     dummyNode2.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode2.GetId(), 50);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode2.GetId(), 50);
+    }
+    LOCK(dummyNode2.cs_sendProcessing);
     peerLogic->SendMessages(&dummyNode2, interruptDummy);
     BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet...
     BOOST_CHECK(connman->IsBanned(addr1));  // ... but 1 still should be
-    Misbehaving(dummyNode2.GetId(), 50);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode2.GetId(), 50);
+    }
     peerLogic->SendMessages(&dummyNode2, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr2));
 
@@ -219,13 +233,23 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
     peerLogic->InitializeNode(&dummyNode1);
     dummyNode1.nVersion = 1;
     dummyNode1.fSuccessfullyConnected = true;
-    Misbehaving(dummyNode1.GetId(), 100);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 100);
+    }
+    LOCK(dummyNode1.cs_sendProcessing);
     peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(!connman->IsBanned(addr1));
-    Misbehaving(dummyNode1.GetId(), 10);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 10);
+    }
     peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(!connman->IsBanned(addr1));
-    Misbehaving(dummyNode1.GetId(), 1);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode1.GetId(), 1);
+    }
     peerLogic->SendMessages(&dummyNode1, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr1));
     gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
@@ -249,7 +273,11 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
     dummyNode.nVersion = 1;
     dummyNode.fSuccessfullyConnected = true;
 
-    Misbehaving(dummyNode.GetId(), 100);
+    {
+        LOCK(cs_main);
+        Misbehaving(dummyNode.GetId(), 100);
+    }
+    LOCK(dummyNode.cs_sendProcessing);
     peerLogic->SendMessages(&dummyNode, interruptDummy);
     BOOST_CHECK(connman->IsBanned(addr));
 
@@ -266,6 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
 CTransactionRef RandomOrphan()
 {
     std::map<uint256, COrphanTx>::iterator it;
+    LOCK(cs_main);
     it = mapOrphanTransactions.lower_bound(InsecureRand256());
     if (it == mapOrphanTransactions.end())
         it = mapOrphanTransactions.begin();
@@ -335,6 +364,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
         BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i));
     }
 
+    LOCK(cs_main);
     // Test EraseOrphansFor:
     for (NodeId i = 0; i < 3; i++)
     {