mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-20 02:25:40 +01:00
Code Issues Projects Releases Wiki Activity

Merge #12882: tests: Make test_bitcoin pass under ThreadSanitzer (clang). Fix lock-order-inversion (potential deadlock).

Browse Source 
9fdf05d70c tests: Fix lock-order-inversion (potential deadlock) in DoS_tests. Reported by TSAN. (practicalswift)

Pull request description:

  Fix lock-order-inversion (potential deadlock) in `DoS_tests`. Reported by Clang's TSAN.

  Makes `src/test/test_bitcoin` pass also when compiled with TreadSanitizer (`./configure --with-sanitizers=thread` with `clang`).

Tree-SHA512: 41403bb7b6e26bdf1b830b5699e27c637d522bae1799d2a19ed4b68b21b2555438b42170d8b1189613beb32a69b76a65175d29a83f5f4e493896c3d0d94ae26d
This commit is contained in:
MarcoFalke 2018-06-28 00:30:00 +14:00
commit d96bdd7830
No known key found for this signature in database
GPG Key ID: D2EA4850E7528B25
2 changed files with 55 additions and 23 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
src/net_processing.h
View File

@ -77,7 +77,7 @@ public:
* @param[in] interrupt Interrupt condition for processing threads
* @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 */
void ConsiderEviction(CNode *pto, int64_t time_in_seconds);

76
src/test/denialofservice_tests.cpp
View File

@ -66,25 +66,40 @@ 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);
{
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();
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
}
{
LOCK2(cs_main, dummyNode1.cs_vSend);
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
dummyNode1.vSendMsg.clear();
}
int64_t nStartTime = GetTime();
// Wait 21 minutes
SetMockTime(nStartTime+21*60);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in getheaders
}
{
LOCK2(cs_main, dummyNode1.cs_vSend);
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
}
// Wait 3 more minutes
SetMockTime(nStartTime+24*60);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy); // should result in disconnect
}
BOOST_CHECK(dummyNode1.fDisconnect == true);
SetMockTime(0);
@ -190,8 +205,10 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 100); // Should get banned
}
LOCK(dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr1));
BOOST_CHECK(!connman->IsBanned(ip(0xa0b0c001|0x0000ff00))); // Different IP, not banned
@ -205,15 +222,20 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 50);
}
LOCK(dummyNode2.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode2, interruptDummy);
{
LOCK2(cs_main, 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
{
LOCK(cs_main);
Misbehaving(dummyNode2.GetId(), 50);
}
peerLogic->SendMessages(&dummyNode2, interruptDummy);
{
LOCK2(cs_main, dummyNode2.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode2, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr2));
bool dummy;
@ -237,20 +259,28 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 100);
}
LOCK(dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(!connman->IsBanned(addr1));
{
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 10);
}
peerLogic->SendMessages(&dummyNode1, interruptDummy);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(!connman->IsBanned(addr1));
{
LOCK(cs_main);
Misbehaving(dummyNode1.GetId(), 1);
}
peerLogic->SendMessages(&dummyNode1, interruptDummy);
{
LOCK2(cs_main, dummyNode1.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode1, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr1));
gArgs.ForceSetArg("-banscore", std::to_string(DEFAULT_BANSCORE_THRESHOLD));
@ -277,8 +307,10 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
LOCK(cs_main);
Misbehaving(dummyNode.GetId(), 100);
}
LOCK(dummyNode.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode, interruptDummy);
{
LOCK2(cs_main, dummyNode.cs_sendProcessing);
peerLogic->SendMessages(&dummyNode, interruptDummy);
}
BOOST_CHECK(connman->IsBanned(addr));
SetMockTime(nStartTime+60*60);