net: handle nodesignals in CConnman

src/main.cpp

@@ -408,16 +408,17 @@ void InitializeNode(NodeId nodeid, const CNode* pnode)
     state.address = pnode->addr;
 }
 
-void FinalizeNode(NodeId nodeid)
+void FinalizeNode(NodeId nodeid, bool& fUpdateConnectionTime)
 {
+    fUpdateConnectionTime = false;
     LOCK(cs_main);
     CNodeState* state = State(nodeid);
 
     if (state->fSyncStarted)
         nSyncStarted--;
 
     if (state->nMisbehavior == 0 && state->fCurrentlyConnected) {
-        AddressCurrentlyConnected(state->address);
+        fUpdateConnectionTime = true;
     }
 
     for (const QueuedBlock& entry : state->vBlocksInFlight)

src/net.cpp

@@ -350,12 +350,6 @@ bool IsReachable(const CNetAddr& addr)
     return IsReachable(net);
 }
 
-void AddressCurrentlyConnected(const CService& addr)
-{
-    addrman.Connected(addr);
-}
-
-
 uint64_t CNode::nTotalBytesRecv = 0;
 uint64_t CNode::nTotalBytesSent = 0;
 RecursiveMutex CNode::cs_totalBytesRecv;
@@ -441,6 +435,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char* pszDest, bool fCo
 
         // Add node
         CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false);
+        GetNodeSignals().InitializeNode(pnode->GetId(), pnode);
         pnode->AddRef();
 
         {
@@ -1067,6 +1062,7 @@ void CConnman::AcceptConnection(const ListenSocket& hListenSocket) {
     }
 
     CNode* pnode = new CNode(hSocket, addr, "", true);
+    GetNodeSignals().InitializeNode(pnode->GetId(), pnode);
     pnode->AddRef();
     pnode->fWhitelisted = whitelisted;
 
@@ -1128,7 +1124,7 @@ void CConnman::ThreadSocketHandler()
                     }
                     if (fDelete) {
                         vNodesDisconnected.remove(pnode);
-                        delete pnode;
+                        DeleteNode(pnode);
                     }
                 }
             }
@@ -2021,6 +2017,7 @@ bool CConnman::Start(boost::thread_group& threadGroup, std::string& strNodeError
         CNetAddr local;
         LookupHost("127.0.0.1", local, false);
         pnodeLocalHost = new CNode(INVALID_SOCKET, CAddress(CService(local, 0), nLocalServices));
+        GetNodeSignals().InitializeNode(pnodeLocalHost->GetId(), pnodeLocalHost);
     }
 
     //
@@ -2104,19 +2101,32 @@ void CConnman::Stop()
                 LogPrintf("CloseSocket(hListenSocket) failed with error %s\n", NetworkErrorString(WSAGetLastError()));
 
     // clean up some globals (to help leak detection)
-    for(CNode* pnode : vNodes)
-        delete pnode;
-    for(CNode* pnode : vNodesDisconnected)
-        delete pnode;
+    for(CNode* pnode : vNodes) {
+        DeleteNode(pnode);
+    }
+    for(CNode* pnode : vNodesDisconnected) {
+        DeleteNode(pnode);
+    }
     vNodes.clear();
     vNodesDisconnected.clear();
     vhListenSocket.clear();
     delete semOutbound;
     semOutbound = NULL;
-    delete pnodeLocalHost;
+    if(pnodeLocalHost)
+        DeleteNode(pnodeLocalHost);
     pnodeLocalHost = NULL;
 }
 
+void CConnman::DeleteNode(CNode* pnode)
+{
+    assert(pnode);
+    bool fUpdateConnectionTime = false;
+    GetNodeSignals().FinalizeNode(pnode->GetId(), fUpdateConnectionTime);
+    if(fUpdateConnectionTime)
+        addrman.Connected(pnode->addr);
+    delete pnode;
+}
+
 CConnman::~CConnman()
 {
 }
@@ -2301,8 +2311,6 @@ CNode::CNode(SOCKET hSocketIn, CAddress addrIn, std::string addrNameIn, bool fIn
     // Be shy and don't send version until we hear
     if (hSocket != INVALID_SOCKET && !fInbound)
         PushVersion();
-
-    GetNodeSignals().InitializeNode(GetId(), this);
 }
 
 CNode::~CNode()
@@ -2311,8 +2319,6 @@ CNode::~CNode()
 
     if (pfilter)
         delete pfilter;
-
-    GetNodeSignals().FinalizeNode(GetId());
 }
 
 void CNode::AskFor(const CInv& inv)

src/net.h

@@ -121,6 +121,7 @@ class CConnman
     void ThreadDNSAddressSeed();
 
     CNode* ConnectNode(CAddress addrConnect, const char* pszDest, bool fCountFailure);
+    void DeleteNode(CNode* pnode);
 
     std::vector<ListenSocket> vhListenSocket;
 };
@@ -156,7 +157,7 @@ struct CNodeSignals
     boost::signals2::signal<bool (CNode*, CConnman&), CombinerAll> ProcessMessages;
     boost::signals2::signal<bool (CNode*, CConnman&), CombinerAll> SendMessages;
     boost::signals2::signal<void (NodeId, const CNode*)> InitializeNode;
-    boost::signals2::signal<void (NodeId)> FinalizeNode;
+    boost::signals2::signal<void (NodeId, bool&)> FinalizeNode;
 };
 
 

src/test/DoS_tests.cpp

@@ -55,6 +55,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
     CNode::ClearBanned();
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
+    GetNodeSignals().InitializeNode(dummyNode1.GetId(), &dummyNode1);
     dummyNode1.nVersion = 1;
     misbehave(dummyNode1.GetId(), 100); // Should get banned
     SendMessages(&dummyNode1, *connman);
@@ -63,6 +64,7 @@ BOOST_AUTO_TEST_CASE(DoS_banning)
 
     CAddress addr2(ip(0xa0b0c002), NODE_NONE);
     CNode dummyNode2(INVALID_SOCKET, addr2, "", true);
+    GetNodeSignals().InitializeNode(dummyNode2.GetId(), &dummyNode2);
     dummyNode2.nVersion = 1;
     misbehave(dummyNode2.GetId(), 50);
     SendMessages(&dummyNode2, *connman);
@@ -79,6 +81,7 @@ BOOST_AUTO_TEST_CASE(DoS_banscore)
     mapArgs["-banscore"] = "111"; // because 11 is my favorite number
     CAddress addr1(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode1(INVALID_SOCKET, addr1, "", true);
+    GetNodeSignals().InitializeNode(dummyNode1.GetId(), &dummyNode1);
     dummyNode1.nVersion = 1;
     misbehave(dummyNode1.GetId(), 100);
     SendMessages(&dummyNode1, *connman);
@@ -100,6 +103,7 @@ BOOST_AUTO_TEST_CASE(DoS_bantime)
 
     CAddress addr(ip(0xa0b0c001), NODE_NONE);
     CNode dummyNode(INVALID_SOCKET, addr, "", true);
+    GetNodeSignals().InitializeNode(dummyNode.GetId(), &dummyNode);
     dummyNode.nVersion = 1;
 
     misbehave(dummyNode.GetId(), 100);