Skip to content

Commit 109a858

Browse files
practicalswiftpracticalswift
committed
tests: Add missing locks to tests
Add missing locks to tests to satisfy lock requirements (such as EXCLUSIVE_LOCKS_REQUIRED(...) (Clang Thread Safety Analysis), AssertLockHeld(...) and implicit lock assumptions).
1 parent fe503e1 commit 109a858

File tree

7 files changed

+60
-10
lines changed

7 files changed

+60
-10
lines changed

‎src/qt/test/wallettests.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,10 @@ void TestGUI()
164164
wallet.SetAddressBook(test.coinbaseKey.GetPubKey().GetID(), "", "receive");
165165
wallet.AddKeyPubKey(test.coinbaseKey, test.coinbaseKey.GetPubKey());
166166
}
167-
wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true);
167+
{
168+
LOCK(cs_main);
169+
wallet.ScanForWalletTransactions(chainActive.Genesis(), nullptr, true);
170+
}
168171
wallet.SetBroadcastTransactions(true);
169172

170173
// Create widgets for sending coins and listing transactions.

‎src/test/DoS_tests.cpp‎

Lines changed: 37 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -66,11 +66,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
6666
dummyNode1.fSuccessfullyConnected = true;
6767

6868
// This test requires that we have a chain with non-zero work.
69+
LOCK(cs_main);
6970
BOOST_CHECK(chainActive.Tip() != nullptr);
7071
BOOST_CHECK(chainActive.Tip()->nChainWork > 0);
7172

7273
// Test starts here
74+
LOCK(dummyNode1.cs_sendProcessing);
7375
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
76+
LOCK(dummyNode1.cs_vSend);
7477
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
7578
dummyNode1.vSendMsg.clear();
7679

@@ -183,7 +186,11 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
183186
peerLogic->InitializeNode(&dummyNode1);
184187
dummyNode1.nVersion = 1;
185188
dummyNode1.fSuccessfullyConnected = true;
186-
Misbehaving(dummyNode1.GetId(), 100); // Should get banned
189+
{
190+
LOCK(cs_main);
191+
Misbehaving(dummyNode1.GetId(), 100); // Should get banned
192+
}
193+
LOCK(dummyNode1.cs_sendProcessing);
187194
peerLogic->SendMessages(&dummyNode1, interruptDummy);
188195
BOOST_CHECK(connman->IsBanned(addr1));
189196
BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
@@ -194,11 +201,18 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
194201
peerLogic->InitializeNode(&dummyNode2);
195202
dummyNode2.nVersion = 1;
196203
dummyNode2.fSuccessfullyConnected = true;
197-
Misbehaving(dummyNode2.GetId(), 50);
204+
{
205+
LOCK(cs_main);
206+
Misbehaving(dummyNode2.GetId(), 50);
207+
}
208+
LOCK(dummyNode2.cs_sendProcessing);
198209
peerLogic->SendMessages(&dummyNode2, interruptDummy);
199210
BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet...
200211
BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be
201-
Misbehaving(dummyNode2.GetId(), 50);
212+
{
213+
LOCK(cs_main);
214+
Misbehaving(dummyNode2.GetId(), 50);
215+
}
202216
peerLogic->SendMessages(&dummyNode2, interruptDummy);
203217
BOOST_CHECK(connman->IsBanned(addr2));
204218

@@ -219,13 +233,23 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
219233
peerLogic->InitializeNode(&dummyNode1);
220234
dummyNode1.nVersion = 1;
221235
dummyNode1.fSuccessfullyConnected = true;
222-
Misbehaving(dummyNode1.GetId(), 100);
236+
{
237+
LOCK(cs_main);
238+
Misbehaving(dummyNode1.GetId(), 100);
239+
}
240+
LOCK(dummyNode1.cs_sendProcessing);
223241
peerLogic->SendMessages(&dummyNode1, interruptDummy);
224242
BOOST_CHECK(!connman->IsBanned(addr1));
225-
Misbehaving(dummyNode1.GetId(), 10);
243+
{
244+
LOCK(cs_main);
245+
Misbehaving(dummyNode1.GetId(), 10);
246+
}
226247
peerLogic->SendMessages(&dummyNode1, interruptDummy);
227248
BOOST_CHECK(!connman->IsBanned(addr1));
228-
Misbehaving(dummyNode1.GetId(), 1);
249+
{
250+
LOCK(cs_main);
251+
Misbehaving(dummyNode1.GetId(), 1);
252+
}
229253
peerLogic->SendMessages(&dummyNode1, interruptDummy);
230254
BOOST_CHECK(connman->IsBanned(addr1));
231255
gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
@@ -249,7 +273,11 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
249273
dummyNode.nVersion = 1;
250274
dummyNode.fSuccessfullyConnected = true;
251275

252-
Misbehaving(dummyNode.GetId(), 100);
276+
{
277+
LOCK(cs_main);
278+
Misbehaving(dummyNode.GetId(), 100);
279+
}
280+
LOCK(dummyNode.cs_sendProcessing);
253281
peerLogic->SendMessages(&dummyNode, interruptDummy);
254282
BOOST_CHECK(connman->IsBanned(addr));
255283

@@ -266,6 +294,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
266294
CTransactionRef RandomOrphan()
267295
{
268296
std::map<uint256, COrphanTx>::iterator it;
297+
LOCK(cs_main);
269298
it = mapOrphanTransactions.lower_bound(InsecureRand256());
270299
if (it == mapOrphanTransactions.end())
271300
it = mapOrphanTransactions.begin();
@@ -335,6 +364,7 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
335364
BOOST_CHECK(!AddOrphanTx(MakeTransactionRef(tx), i));
336365
}
337366

367+
LOCK(cs_main);
338368
// Test EraseOrphansFor:
339369
for (NodeId i = 0; i < 3; i++)
340370
{

‎src/test/blockencodings_tests.cpp‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ BOOST_AUTO_TEST_CASE(SimpleRoundTripTest)
6262
CBlock block(BuildBlockTestCase());
6363

6464
pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2]));
65+
LOCK(pool.cs);
6566
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
6667

6768
// Do a simple ShortTxIDs RT
@@ -161,6 +162,7 @@ BOOST_AUTO_TEST_CASE(NonCoinbasePreforwardRTTest)
161162
CBlock block(BuildBlockTestCase());
162163

163164
pool.addUnchecked(block.vtx[2]->GetHash(), entry.FromTx(*block.vtx[2]));
165+
LOCK(pool.cs);
164166
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[2]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
165167

166168
uint256 txhash;
@@ -227,6 +229,7 @@ BOOST_AUTO_TEST_CASE(SufficientPreforwardRTTest)
227229
CBlock block(BuildBlockTestCase());
228230

229231
pool.addUnchecked(block.vtx[1]->GetHash(), entry.FromTx(*block.vtx[1]));
232+
LOCK(pool.cs);
230233
BOOST_CHECK_EQUAL(pool.mapTx.find(block.vtx[1]->GetHash())->GetSharedTx().use_count(), SHARED_TX_OFFSET + 0);
231234

232235
uint256 txhash;

‎src/test/mempool_tests.cpp‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,7 @@ BOOST_AUTO_TEST_CASE(MempoolIndexingTest)
165165
sortedOrder[2] = tx1.GetHash().ToString(); // 10000
166166
sortedOrder[3] = tx4.GetHash().ToString(); // 15000
167167
sortedOrder[4] = tx2.GetHash().ToString(); // 20000
168+
LOCK(pool.cs);
168169
CheckSort<descendant_score>(pool, sortedOrder);
169170

170171
/* low fee but with high fee child */
@@ -375,6 +376,7 @@ BOOST_AUTO_TEST_CASE(MempoolAncestorIndexingTest)
375376
}
376377
sortedOrder[4] = tx3.GetHash().ToString(); // 0
377378

379+
LOCK(pool.cs);
378380
CheckSort<ancestor_score>(pool, sortedOrder);
379381

380382
/* low fee parent with high fee child */

‎src/test/test_bitcoin.cpp‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -149,7 +149,10 @@ TestChain100Setup::CreateAndProcessBlock(const std::vector<CMutableTransaction>&
149149
block.vtx.push_back(MakeTransactionRef(tx));
150150
// IncrementExtraNonce creates a valid coinbase and merkleRoot
151151
unsigned int extraNonce = 0;
152-
IncrementExtraNonce(&block, chainActive.Tip(), extraNonce);
152+
{
153+
LOCK(cs_main);
154+
IncrementExtraNonce(&block, chainActive.Tip(), extraNonce);
155+
}
153156

154157
while (!CheckProofOfWork(block.GetHash(), block.nBits, chainparams.GetConsensus())) ++block.nNonce;
155158

‎src/test/txvalidationcache_tests.cpp‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ BOOST_FIXTURE_TEST_CASE(tx_mempool_block_doublespend, TestChain100Setup)
6666

6767
// Test 1: block with both of those transactions should be rejected.
6868
block = CreateAndProcessBlock(spends, scriptPubKey);
69+
LOCK(cs_main);
6970
BOOST_CHECK(chainActive.Tip()->GetBlockHash() != block.GetHash());
7071

7172
// Test 2: ... and should be rejected if spend1 is in the memory pool
@@ -151,7 +152,10 @@ BOOST_FIXTURE_TEST_CASE(checkinputs_test, TestChain100Setup)
151152
{
152153
// Test that passing CheckInputs with one set of script flags doesn't imply
153154
// that we would pass again with a different set of flags.
154-
InitScriptExecutionCache();
155+
{
156+
LOCK(cs_main);
157+
InitScriptExecutionCache();
158+
}
155159

156160
CScript p2pk_scriptPubKey = CScript() << ToByteVector(coinbaseKey.GetPubKey()) << OP_CHECKSIG;
157161
CScript p2sh_scriptPubKey = GetScriptForDestination(CScriptID(p2pk_scriptPubKey));

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -489,6 +489,7 @@ BOOST_FIXTURE_TEST_CASE(importwallet_rescan, TestChain100Setup)
489489
vpwallets[0] = &wallet;
490490
::importwallet(request);
491491

492+
LOCK(wallet.cs_wallet);
492493
BOOST_CHECK_EQUAL(wallet.mapWallet.size(), 3);
493494
BOOST_CHECK_EQUAL(coinbaseTxns.size(), 103);
494495
for (size_t i = 0; i < coinbaseTxns.size(); ++i) {
@@ -534,6 +535,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
534535
SetMockTime(mockTime);
535536
CBlockIndex* block = nullptr;
536537
if (blockTime > 0) {
538+
LOCK(cs_main);
537539
auto inserted = mapBlockIndex.emplace(GetRandHash(), new CBlockIndex);
538540
assert(inserted.second);
539541
const uint256& hash = inserted.first->first;
@@ -547,6 +549,7 @@ static int64_t AddTx(CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64
547549
wtx.SetMerkleBranch(block, 0);
548550
}
549551
wallet.AddToWallet(wtx);
552+
LOCK(wallet.cs_wallet);
550553
return wallet.mapWallet.at(wtx.GetHash()).nTimeSmart;
551554
}
552555

@@ -583,6 +586,7 @@ BOOST_AUTO_TEST_CASE(ComputeTimeSmart)
583586
BOOST_AUTO_TEST_CASE(LoadReceiveRequests)
584587
{
585588
CTxDestination dest = CKeyID();
589+
LOCK(pwalletMain->cs_wallet);
586590
pwalletMain->AddDestData(dest, "misc", "val_misc");
587591
pwalletMain->AddDestData(dest, "rr0", "val_rr0");
588592
pwalletMain->AddDestData(dest, "rr1", "val_rr1");
@@ -625,6 +629,7 @@ class ListCoinsTestingSetup : public TestChain100Setup
625629
BOOST_CHECK(wallet->CreateTransaction({recipient}, wtx, reservekey, fee, changePos, error, dummy));
626630
CValidationState state;
627631
BOOST_CHECK(wallet->CommitTransaction(wtx, reservekey, nullptr, state));
632+
LOCK(wallet->cs_wallet);
628633
auto it = wallet->mapWallet.find(wtx.GetHash());
629634
BOOST_CHECK(it != wallet->mapWallet.end());
630635
CreateAndProcessBlock({CMutableTransaction(*it->second.tx)}, GetScriptForRawPubKey(coinbaseKey.GetPubKey()));

0 commit comments

Comments
 (0)