highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-06-21 22:31:21 +02:00
Code Issues Packages Projects Releases Wiki Activity

net: drop cs_sendProcessing

Browse Source 
SendMessages() is now protected g_msgproc_mutex; so this additional
per-node mutex is redundant.
This commit is contained in:
Anthony Towns 2022-09-13 12:22:18 +10:00
parent 1e78f566d5
commit bf12abe454
8 changed files with 18 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
src/net.cpp
View File

@ -2005,10 +2005,7 @@ void CConnman::ThreadMessageHandler()
if (flagInterruptMsgProc) if (flagInterruptMsgProc)
return; return;
// Send messages // Send messages
{ m_msgproc->SendMessages(pnode);
LOCK(pnode->cs_sendProcessing);
m_msgproc->SendMessages(pnode);
}
if (flagInterruptMsgProc) if (flagInterruptMsgProc)
return; return;

4
src/net.h
View File

@ -377,8 +377,6 @@ public:
std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg); std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg);
size_t nProcessQueueSize GUARDED_BY(cs_vProcessMsg){0}; size_t nProcessQueueSize GUARDED_BY(cs_vProcessMsg){0};
RecursiveMutex cs_sendProcessing;
uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0}; uint64_t nRecvBytes GUARDED_BY(cs_vRecv){0};
std::atomic<std::chrono::seconds> m_last_send{0s}; std::atomic<std::chrono::seconds> m_last_send{0s};
@ -653,7 +651,7 @@ public:
* @param[in] pnode The node which we are sending messages to. * @param[in] pnode The node which we are sending messages to.
* @return True if there is more work to be done * @return True if there is more work to be done
*/ */
virtual bool SendMessages(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_sendProcessing) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0; virtual bool SendMessages(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(g_msgproc_mutex) = 0;
protected: protected:

2
src/net_processing.cpp
View File

@ -516,7 +516,7 @@ public:
void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex); void FinalizeNode(const CNode& node) override EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_headers_presync_mutex);
bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override bool ProcessMessages(CNode* pfrom, std::atomic<bool>& interrupt) override
EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex); EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, !m_headers_presync_mutex, g_msgproc_mutex);
bool SendMessages(CNode* pto) override EXCLUSIVE_LOCKS_REQUIRED(pto->cs_sendProcessing) bool SendMessages(CNode* pto) override
EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex); EXCLUSIVE_LOCKS_REQUIRED(!m_peer_mutex, !m_recent_confirmed_transactions_mutex, !m_most_recent_block_mutex, g_msgproc_mutex);
/** Implement PeerManager */ /** Implement PeerManager */

42
src/test/denialofservice_tests.cpp
View File

@ -82,10 +82,8 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
} }
// Test starts here // Test starts here
{ BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders
}
{ {
LOCK(dummyNode1.cs_vSend); LOCK(dummyNode1.cs_vSend);
BOOST_CHECK(dummyNode1.vSendMsg.size() > 0); BOOST_CHECK(dummyNode1.vSendMsg.size() > 0);
@ -95,20 +93,14 @@ BOOST_AUTO_TEST_CASE(outbound_slow_chain_eviction)
int64_t nStartTime = GetTime(); int64_t nStartTime = GetTime();
// Wait 21 minutes // Wait 21 minutes
SetMockTime(nStartTime+21*60); SetMockTime(nStartTime+21*60);
{ BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in getheaders
}
{ {
LOCK(dummyNode1.cs_vSend); LOCK(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);
{ BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect
LOCK(dummyNode1.cs_sendProcessing);
BOOST_CHECK(peerman.SendMessages(&dummyNode1)); // should result in disconnect
}
BOOST_CHECK(dummyNode1.fDisconnect == true); BOOST_CHECK(dummyNode1.fDisconnect == true);
peerman.FinalizeNode(dummyNode1); peerman.FinalizeNode(dummyNode1);
@ -312,10 +304,8 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
nodes[0]->fSuccessfullyConnected = true; nodes[0]->fSuccessfullyConnected = true;
connman->AddTestNode(*nodes[0]); connman->AddTestNode(*nodes[0]);
peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged peerLogic->UnitTestMisbehaving(nodes[0]->GetId(), DISCOURAGEMENT_THRESHOLD); // Should be discouraged
{ BOOST_CHECK(peerLogic->SendMessages(nodes[0]));
LOCK(nodes[0]->cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(nodes[0]));
}
BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0]));
BOOST_CHECK(nodes[0]->fDisconnect); BOOST_CHECK(nodes[0]->fDisconnect);
BOOST_CHECK(!banman->IsDiscouraged(other_addr)); // Different address, not discouraged BOOST_CHECK(!banman->IsDiscouraged(other_addr)); // Different address, not discouraged
@ -334,10 +324,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
nodes[1]->fSuccessfullyConnected = true; nodes[1]->fSuccessfullyConnected = true;
connman->AddTestNode(*nodes[1]); connman->AddTestNode(*nodes[1]);
peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1); peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), DISCOURAGEMENT_THRESHOLD - 1);
{ BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
LOCK(nodes[1]->cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
}
// [0] is still discouraged/disconnected. // [0] is still discouraged/disconnected.
BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0]));
BOOST_CHECK(nodes[0]->fDisconnect); BOOST_CHECK(nodes[0]->fDisconnect);
@ -345,10 +332,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
BOOST_CHECK(!banman->IsDiscouraged(addr[1])); BOOST_CHECK(!banman->IsDiscouraged(addr[1]));
BOOST_CHECK(!nodes[1]->fDisconnect); BOOST_CHECK(!nodes[1]->fDisconnect);
peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), 1); // [1] reaches discouragement threshold peerLogic->UnitTestMisbehaving(nodes[1]->GetId(), 1); // [1] reaches discouragement threshold
{ BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
LOCK(nodes[1]->cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(nodes[1]));
}
// Expect both [0] and [1] to be discouraged/disconnected now. // Expect both [0] and [1] to be discouraged/disconnected now.
BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0]));
BOOST_CHECK(nodes[0]->fDisconnect); BOOST_CHECK(nodes[0]->fDisconnect);
@ -371,10 +355,7 @@ BOOST_AUTO_TEST_CASE(peer_discouragement)
nodes[2]->fSuccessfullyConnected = true; nodes[2]->fSuccessfullyConnected = true;
connman->AddTestNode(*nodes[2]); connman->AddTestNode(*nodes[2]);
peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD); peerLogic->UnitTestMisbehaving(nodes[2]->GetId(), DISCOURAGEMENT_THRESHOLD);
{ BOOST_CHECK(peerLogic->SendMessages(nodes[2]));
LOCK(nodes[2]->cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(nodes[2]));
}
BOOST_CHECK(banman->IsDiscouraged(addr[0])); BOOST_CHECK(banman->IsDiscouraged(addr[0]));
BOOST_CHECK(banman->IsDiscouraged(addr[1])); BOOST_CHECK(banman->IsDiscouraged(addr[1]));
BOOST_CHECK(banman->IsDiscouraged(addr[2])); BOOST_CHECK(banman->IsDiscouraged(addr[2]));
@ -417,10 +398,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
dummyNode.fSuccessfullyConnected = true; dummyNode.fSuccessfullyConnected = true;
peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD); peerLogic->UnitTestMisbehaving(dummyNode.GetId(), DISCOURAGEMENT_THRESHOLD);
{ BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
LOCK(dummyNode.cs_sendProcessing);
BOOST_CHECK(peerLogic->SendMessages(&dummyNode));
}
BOOST_CHECK(banman->IsDiscouraged(addr)); BOOST_CHECK(banman->IsDiscouraged(addr));
peerLogic->FinalizeNode(dummyNode); peerLogic->FinalizeNode(dummyNode);

5
src/test/fuzz/process_message.cpp
View File

@ -94,10 +94,7 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE
GetTime<std::chrono::microseconds>(), std::atomic<bool>{false}); GetTime<std::chrono::microseconds>(), std::atomic<bool>{false});
} catch (const std::ios_base::failure&) { } catch (const std::ios_base::failure&) {
} }
{ g_setup->m_node.peerman->SendMessages(&p2p_node);
LOCK(p2p_node.cs_sendProcessing);
g_setup->m_node.peerman->SendMessages(&p2p_node);
}
SyncWithValidationInterfaceQueue(); SyncWithValidationInterfaceQueue();
g_setup->m_node.connman->StopNodes(); g_setup->m_node.connman->StopNodes();
} }

5
src/test/fuzz/process_messages.cpp
View File

@ -72,10 +72,7 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
connman.ProcessMessagesOnce(random_node); connman.ProcessMessagesOnce(random_node);
} catch (const std::ios_base::failure&) { } catch (const std::ios_base::failure&) {
} }
{ g_setup->m_node.peerman->SendMessages(&random_node);
LOCK(random_node.cs_sendProcessing);
g_setup->m_node.peerman->SendMessages(&random_node);
}
} }
SyncWithValidationInterfaceQueue(); SyncWithValidationInterfaceQueue();
g_setup->m_node.connman->StopNodes(); g_setup->m_node.connman->StopNodes();

5
src/test/net_tests.cpp
View File

@ -891,10 +891,7 @@ BOOST_AUTO_TEST_CASE(initial_advertise_from_version_message)
} }
}; };
{ m_node.peerman->SendMessages(&peer);
LOCK(peer.cs_sendProcessing);
m_node.peerman->SendMessages(&peer);
}
BOOST_CHECK(sent); BOOST_CHECK(sent);

10
src/test/util/net.cpp
View File

@ -44,10 +44,7 @@ void ConnmanTestMsg::Handshake(CNode& node,
(void)connman.ReceiveMsgFrom(node, msg_version); (void)connman.ReceiveMsgFrom(node, msg_version);
node.fPauseSend = false; node.fPauseSend = false;
connman.ProcessMessagesOnce(node); connman.ProcessMessagesOnce(node);
{ peerman.SendMessages(&node);
LOCK(node.cs_sendProcessing);
peerman.SendMessages(&node);
}
if (node.fDisconnect) return; if (node.fDisconnect) return;
assert(node.nVersion == version); assert(node.nVersion == version);
assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION)); assert(node.GetCommonVersion() == std::min(version, PROTOCOL_VERSION));
@ -60,10 +57,7 @@ void ConnmanTestMsg::Handshake(CNode& node,
(void)connman.ReceiveMsgFrom(node, msg_verack); (void)connman.ReceiveMsgFrom(node, msg_verack);
node.fPauseSend = false; node.fPauseSend = false;
connman.ProcessMessagesOnce(node); connman.ProcessMessagesOnce(node);
{ peerman.SendMessages(&node);
LOCK(node.cs_sendProcessing);
peerman.SendMessages(&node);
}
assert(node.fSuccessfullyConnected == true); assert(node.fSuccessfullyConnected == true);
} }
} }