From 43c67feae43e62ae126807270518890b89b4495b Mon Sep 17 00:00:00 2001 From: Alexander Block Date: Fri, 17 Apr 2020 23:07:17 +0200 Subject: [PATCH] AddRef/Release when adding/erasing CNode* entries to/from mapNodesWithDataToSend --- src/net.cpp | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/net.cpp b/src/net.cpp index cb2fddd6b7d803..29fc18732ebd02 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -523,7 +523,10 @@ void CNode::CloseSocketDisconnect(CConnman* connman) connman->mapSendableNodes.erase(GetId()); { LOCK(connman->cs_mapNodesWithDataToSend); - connman->mapNodesWithDataToSend.erase(GetId()); + if (connman->mapNodesWithDataToSend.erase(GetId()) != 0) { + // See comment in PushMessage + Release(); + } } LogPrint(BCLog::NET, "disconnecting peer=%d\n", id); @@ -1679,6 +1682,8 @@ void CConnman::SocketHandler() vSendableNodes.reserve(mapNodesWithDataToSend.size()); for (auto it = mapNodesWithDataToSend.begin(); it != mapNodesWithDataToSend.end(); ) { if (it->second->nSendMsgSize == 0) { + // See comment in PushMessage + it->second->Release(); it = mapNodesWithDataToSend.erase(it); } else { if (it->second->fCanSendData) { @@ -3610,6 +3615,11 @@ void CConnman::PushMessage(CNode* pnode, CSerializedNetMsg&& msg) { LOCK(cs_mapNodesWithDataToSend); + // we're not holding cs_vNodes here, so there is a chance of this node being disconnected shortly before + // we get here. Whoever called PushMessage still has a ref to CNode*, but will later Release() it, so we + // might end up having an entry in mapNodesWithDataToSend that is not in vNodes anymore. We need to + // Add/Release refs when adding/erasing mapNodesWithDataToSend. + pnode->AddRef(); mapNodesWithDataToSend.emplace(pnode->GetId(), pnode); }