diff options
author | Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> | 2021-04-12 14:54:58 +0300 |
---|---|---|
committer | Hennadii Stepanov <32963518+hebasto@users.noreply.github.com> | 2021-04-22 17:28:39 +0300 |
commit | a3d090d1103cd6c25daf07afdf4e65febca6d3f7 (patch) | |
tree | d56437fabbfdda126e24aa011dda36710d476250 /src | |
parent | f0b457212f9876a8c7e4d680178b49b555b488e2 (diff) | |
download | bitcoin-a3d090d1103cd6c25daf07afdf4e65febca6d3f7.tar.xz |
net: Restrict period when cs_vNodes mutex is locked
Diffstat (limited to 'src')
-rw-r--r-- | src/init.cpp | 15 | ||||
-rw-r--r-- | src/net.cpp | 8 | ||||
-rw-r--r-- | src/test/fuzz/process_message.cpp | 1 | ||||
-rw-r--r-- | src/test/fuzz/process_messages.cpp | 1 |
4 files changed, 5 insertions, 20 deletions
diff --git a/src/init.cpp b/src/init.cpp index 701a7529af..280a0cbda7 100644 --- a/src/init.cpp +++ b/src/init.cpp @@ -200,20 +200,7 @@ void Shutdown(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(); diff --git a/src/net.cpp b/src/net.cpp index ae38acdc3c..65f8b8baec 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -2636,8 +2636,9 @@ void CConnman::StopNodes() } // Close sockets - LOCK(cs_vNodes); - for (CNode* pnode : vNodes) + std::vector<CNode*> nodes; + WITH_LOCK(cs_vNodes, nodes.swap(vNodes)); + for (CNode* pnode : nodes) pnode->CloseSocketDisconnect(); for (ListenSocket& hListenSocket : vhListenSocket) if (hListenSocket.socket != INVALID_SOCKET) @@ -2645,13 +2646,12 @@ void CConnman::StopNodes() LogPrintf("CloseSocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAGetLastError())); // clean up some globals (to help leak detection) - for (CNode* pnode : vNodes) { + for (CNode* pnode : nodes) { DeleteNode(pnode); } for (CNode* pnode : vNodesDisconnected) { DeleteNode(pnode); } - vNodes.clear(); vNodesDisconnected.clear(); vhListenSocket.clear(); semOutbound.reset(); diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp index 96e1cfa08f..7b99193ad0 100644 --- a/src/test/fuzz/process_message.cpp +++ b/src/test/fuzz/process_message.cpp @@ -100,7 +100,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(); } diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp index 203c0ef8e1..11b236c9bd 100644 --- a/src/test/fuzz/process_messages.cpp +++ b/src/test/fuzz/process_messages.cpp @@ -80,6 +80,5 @@ FUZZ_TARGET_INIT(process_messages, initialize_process_messages) } } SyncWithValidationInterfaceQueue(); - LOCK2(::cs_main, g_cs_orphans); // See init.cpp for rationale for implicit locking order requirement g_setup->m_node.connman->StopNodes(); } |