tests: Fix lock-order-inversion (potential deadlock) in DoS_tests. Reported by TSAN.

Makes `src/test/test_bitcoin --run_test=DoS_tests` pass also when
compiled with TreadSanitizer (`./configure --with-sanitizers=thread`).
This commit is contained in:
practicalswift 2018-04-04 14:04:10 +02:00
parent a607d23ae8
commit 9fdf05d70c
2 changed files with 55 additions and 23 deletions

View file

@ -77,7 +77,7 @@ public:
* @param[in] interrupt Interrupt condition for processing threads * @param[in] interrupt Interrupt condition for processing threads
* @return True if there is more work to be done * @return True if there is more work to be done
*/ */
bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override; bool SendMessages(CNode* pto, std::atomic<bool>& interrupt) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing);
/** Consider evicting an outbound peer based on the amount of time they've been behind our tip */ /** Consider evicting an outbound peer based on the amount of time they've been behind our tip */
void ConsiderEviction(CNode *pto, int64_t time_in_seconds); void ConsiderEviction(CNode *pto, int64_t time_in_seconds);

View file

@ -66,25 +66,40 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
dummyNode1.fSuccessfullyConnected = true; dummyNode1.fSuccessfullyConnected = true;
// This test requires that we have a chain with non-zero work. // This test requires that we have a chain with non-zero work.
{
LOCK(cs_main); LOCK(cs_main);
BOOST_CHECK(chainActive.Tip() != nullptr); BOOST_CHECK(chainActive.Tip() != nullptr);
BOOST_CHECK(chainActive.Tip()->nChainWork > 0); BOOST_CHECK(chainActive.Tip()->nChainWork > 0);
}
// Test starts here // Test starts here
LOCK(dummyNode1.cs_sendProcessing); {
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
LOCK(dummyNode1.cs_vSend); }
{
LOCK2(cs_main, dummyNode1.cs_vSend);
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
dummyNode1.vSendMsg.clear(); dummyNode1.vSendMsg.clear();
}
int64_t nStartTime = GetTime(); int64_t nStartTime = GetTime();
// Wait 21 minutes // Wait 21 minutes
SetMockTime(nStartTime+21*60); SetMockTime(nStartTime+21*60);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
}
{
LOCK2(cs_main, dummyNode1.cs_vSend);
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
}
// Wait 3 more minutes // Wait 3 more minutes
SetMockTime(nStartTime+24*60); SetMockTime(nStartTime+24*60);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect
}
BOOST_CHECK(dummyNode1.fDisconnect == true); BOOST_CHECK(dummyNode1.fDisconnect == true);
SetMockTime(0); SetMockTime(0);
@ -190,8 +205,10 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 100); // Should get banned Misbehaving(dummyNode1.GetId(), 100); // Should get banned
} }
LOCK(dummyNode1.cs_sendProcessing); {
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr1)); BOOST_CHECK(connman->IsBanned(addr1));
BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
@ -205,15 +222,20 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 50); Misbehaving(dummyNode2.GetId(), 50);
} }
LOCK(dummyNode2.cs_sendProcessing); {
LOCK2(cs_main, dummyNode2.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode2, interruptDummy); peerLogic->SendMessages(&dummyNode2, interruptDummy);
}
BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet... BOOST_CHECK(!connman->IsBanned(addr2)); // 2 not banned yet...
BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be BOOST_CHECK(connman->IsBanned(addr1)); // ... but 1 still should be
{ {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 50); Misbehaving(dummyNode2.GetId(), 50);
} }
{
LOCK2(cs_main, dummyNode2.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode2, interruptDummy); peerLogic->SendMessages(&dummyNode2, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr2)); BOOST_CHECK(connman->IsBanned(addr2));
bool dummy; bool dummy;
@ -237,20 +259,28 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 100); Misbehaving(dummyNode1.GetId(), 100);
} }
LOCK(dummyNode1.cs_sendProcessing); {
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(!connman->IsBanned(addr1)); BOOST_CHECK(!connman->IsBanned(addr1));
{ {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 10); Misbehaving(dummyNode1.GetId(), 10);
} }
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(!connman->IsBanned(addr1)); BOOST_CHECK(!connman->IsBanned(addr1));
{ {
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 1); Misbehaving(dummyNode1.GetId(), 1);
} }
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr1)); BOOST_CHECK(connman->IsBanned(addr1));
gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD)); gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
@ -277,8 +307,10 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
LOCK(cs_main); LOCK(cs_main);
Misbehaving(dummyNode.GetId(), 100); Misbehaving(dummyNode.GetId(), 100);
} }
LOCK(dummyNode.cs_sendProcessing); {
LOCK2(cs_main, dummyNode.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode, interruptDummy); peerLogic->SendMessages(&dummyNode, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr)); BOOST_CHECK(connman->IsBanned(addr));
SetMockTime(nStartTime+60*60); SetMockTime(nStartTime+60*60);