sikkienl/dash
1
0
Fork 0
mirror of https://github.com/dashpay/dash.git synced 2024-12-24 11:32:46 +01:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#21563: net: Restrict period when cs_vNodes mutex is locked

Browse Source 
8c8237a4a10feb2ac9ce46f67b5d14bf879b670f net, refactor: Fix style in CConnman::StopNodes (Hennadii Stepanov)
229ac1892d807a1eea5a7c24ae0fe27dc913b1bd net: Combine two loops into one, and update comments (Hennadii Stepanov)
a3d090d1103cd6c25daf07afdf4e65febca6d3f7 net: Restrict period when cs_vNodes mutex is locked (Hennadii Stepanov)

Pull request description:

  This PR restricts the period when the `cs_vNodes` mutex is locked, prevents the only case when `cs_vNodes` could be locked before the `::cs_main`.

  This change makes the explicit locking of recursive mutexes in the explicit order redundant.

ACKs for top commit:
  jnewbery:
    utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
  vasild:
    ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f
  ajtowns:
    utACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f - logic seems sound
  MarcoFalke:
    review ACK 8c8237a4a10feb2ac9ce46f67b5d14bf879b670f 👢

Tree-SHA512: a8277924339622b188b12d260a100adf5d82781634cf974320cf6007341f946a7ff40351137c2f5369aed0d318f38aac2d32965c9b619432440d722a4e78bb73
This commit is contained in:
MarcoFalke 2021-04-25 10:08:52 +02:00 committed by Konstantin Akimov
parent 16052f10ae
commit 6ed62b323c
No known key found for this signature in database
GPG Key ID: 2176C4A5D01EA524
4 changed files with 9 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
src/init.cpp
View File

@ -254,20 +254,7 @@ void PrepareShutdown(NodeContext& node)
// Because these depend on each-other, we make sure that neither can be
// using the other before destroying them.
if (node.peerman) UnregisterValidationInterface(node.peerman.get());
// Follow the lock order requirements:
// * CheckForStaleTipAndEvictPeers locks cs_main before indirectly calling GetExtraFullOutboundCount
// which locks cs_vNodes.
// * ProcessMessage locks cs_main and g_cs_orphans before indirectly calling ForEachNode which
// locks cs_vNodes.
// * CConnman::Stop calls DeleteNode, which calls FinalizeNode, which locks cs_main and calls
// EraseOrphansFor, which locks g_cs_orphans.
//
// Thus the implicit locking order requirement is: (1) cs_main, (2) g_cs_orphans, (3) cs_vNodes.
if (node.connman) {
node.connman->StopThreads();
LOCK2(::cs_main, ::g_cs_orphans);
node.connman->StopNodes();
}
if (node.connman) node.connman->Stop();
StopTorControl();

20
src/net.cpp
View File

@ -4301,13 +4301,15 @@ void CConnman::StopNodes()
}
}
{
LOCK(m_nodes_mutex);
// Close sockets
for (CNode *pnode : m_nodes)
pnode->CloseSocketDisconnect(this);
// Delete peer connections.
std::vector<CNode*> nodes;
WITH_LOCK(m_nodes_mutex, nodes.swap(m_nodes));
for (CNode *pnode : nodes) {
pnode->CloseSocketDisconnect(this);
DeleteNode(pnode);
}
// Close listening sockets.
for (ListenSocket& hListenSocket : vhListenSocket) {
if (hListenSocket.sock) {
if (m_edge_trig_events && !m_edge_trig_events->RemoveSocket(hListenSocket.sock->Get())) {
@ -4316,12 +4318,6 @@ void CConnman::StopNodes()
}
}
// clean up some globals (to help leak detection)
std::vector<CNode*> nodes;
WITH_LOCK(m_nodes_mutex, nodes.swap(m_nodes));
for (CNode* pnode : nodes) {
DeleteNode(pnode);
}
for (CNode* pnode : m_nodes_disconnected) {
DeleteNode(pnode);
}

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

@ -103,7 +103,6 @@ void fuzz_target(FuzzBufferType buffer, const std::string& LIMIT_TO_MESSAGE_TYPE
}
g_setup->m_node.peerman->SendMessages(&p2p_node);
SyncWithValidationInterfaceQueue();
LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
g_setup->m_node.connman->StopNodes();
}

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

@ -76,6 +76,5 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages)
g_setup->m_node.peerman->SendMessages(&random_node);
}
SyncWithValidationInterfaceQueue();
LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement
g_setup->m_node.connman->StopNodes();
}