Merge #924: Fix AddRef() usage

513506f Fixing AddRef() usage
Using AddRef() in ConnectNode() for existing connections doesn't feel right considering how refs are released in ThreadSocketHandler(). I guess this could be the reason that sometimes refs stay >0 no matter what and nodes stuck in vNodesDisconnected forever which means that node never get deleted and FinalizeNode signal is never fired which in its turn means that for example mapBlocksInFlight can't be cleaned properly and then blocks stuck.

This commit should solve the issue by:
- removing AddRef() for existing connections
- adding AddRef() in CNode's constructor using the same conditions as in ThreadSocketHandler()
- addding AddRef() in ConnectNode() and Release() in ThreadSocketHandler() for mixing nodes
- removing explicit calls to Release() (back to `pnode->fDisconnect = true` in `CMasternodeMan::ProcessMasternodeConnections`)

9da4a83 fix names/comments
This commit is contained in:
UdjinM6 2016-07-30 15:05:41 +04:00 committed by Holger Schinzel
parent d514ee7f9a
commit 17dfbdea1b
5 changed files with 33 additions and 30 deletions

View File

@ -105,11 +105,10 @@ void CDarkSendRelay::RelayThroughNode(int nRank)
if(pmn != NULL){
//printf("RelayThroughNode %s\n", pmn->addr.ToString().c_str());
CNode* pnode = ConnectNode((CAddress)pmn->addr, NULL, false);
if(pnode){
CNode* pnode = ConnectNode((CAddress)pmn->addr, NULL);
if(pnode) {
//printf("Connected\n");
pnode->PushMessage("dsr", (*this));
pnode->Release();
return;
}
} else {

View File

@ -526,11 +526,10 @@ void CMasternodeMan::ProcessMasternodeConnections()
LOCK(cs_vNodes);
BOOST_FOREACH(CNode* pnode, vNodes) {
if(pnode->fDarkSendMaster){
if(pnode->fMasternode) {
if(darkSendPool.pSubmittedToMasternode != NULL && pnode->addr == darkSendPool.pSubmittedToMasternode->addr) continue;
LogPrintf("Closing Masternode connection %s \n", pnode->addr.ToString());
pnode->fDarkSendMaster = false;
pnode->Release();
pnode->fDisconnect = true;
}
}
}

View File

@ -390,21 +390,24 @@ CNode* FindNode(const CService& addr)
return NULL;
}
CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool darkSendMaster)
CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool fConnectToMasternode)
{
if (pszDest == NULL) {
// we clean masternode connections in CMasternodeMan::ProcessMasternodeConnections()
// so should be safe to skip this and connect to local Hot MN on CActiveMasternode::ManageStatus()
if (IsLocal(addrConnect) && !darkSendMaster)
if (IsLocal(addrConnect) && !fConnectToMasternode)
return NULL;
// Look for an existing connection
CNode* pnode = FindNode((CService)addrConnect);
if (pnode)
{
pnode->fDarkSendMaster = darkSendMaster;
pnode->AddRef();
// we have existing connection to this node but it was not a connection to masternode,
// change flag and add reference so that we can correctly clear it later
if(fConnectToMasternode && !pnode->fMasternode) {
pnode->fMasternode = true;
pnode->AddRef();
}
return pnode;
}
}
@ -429,8 +432,7 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool darkSendMaste
addrman.Attempt(addrConnect);
// Add node
CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false);
pnode->AddRef();
CNode* pnode = new CNode(hSocket, addrConnect, pszDest ? pszDest : "", false, true);
{
LOCK(cs_vNodes);
@ -438,7 +440,10 @@ CNode* ConnectNode(CAddress addrConnect, const char *pszDest, bool darkSendMaste
}
pnode->nTimeConnected = GetTime();
if(darkSendMaster) pnode->fDarkSendMaster = true;
if(fConnectToMasternode) {
pnode->fMasternode = true;
pnode->AddRef();
}
return pnode;
} else if (!proxyConnectionFailed) {
@ -1025,7 +1030,6 @@ static void AcceptConnection(const ListenSocket& hListenSocket) {
}
CNode* pnode = new CNode(hSocket, addr, "", true);
pnode->AddRef();
pnode->fWhitelisted = whitelisted;
LogPrint("net", "connection from %s accepted\n", addr.ToString());
@ -1065,6 +1069,8 @@ void ThreadSocketHandler()
// hold in disconnected pool until all refs are released
if (pnode->fNetworkNode || pnode->fInbound)
pnode->Release();
if (pnode->fMasternode)
pnode->Release();
vNodesDisconnected.push_back(pnode);
}
}
@ -1715,7 +1721,6 @@ bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant *grantOu
return false;
if (grantOutbound)
grantOutbound->MoveTo(pnode->grantOutbound);
pnode->fNetworkNode = true;
if (fOneShot)
pnode->fOneShot = true;
@ -2377,7 +2382,7 @@ bool CAddrDB::Read(CAddrMan& addr)
unsigned int ReceiveFloodSize() { return 1000*GetArg("-maxreceivebuffer", DEFAULT_MAXRECEIVEBUFFER); }
unsigned int SendBufferSize() { return 1000*GetArg("-maxsendbuffer", DEFAULT_MAXSENDBUFFER); }
CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNameIn, bool fInboundIn) :
CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNameIn, bool fInboundIn, bool fNetworkNodeIn) :
ssSend(SER_NETWORK, INIT_PROTO_VERSION),
addrKnown(5000, 0.001),
filterInventoryKnown(50000, 0.000001)
@ -2399,7 +2404,7 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
fOneShot = false;
fClient = false; // set by version message
fInbound = fInboundIn;
fNetworkNode = false;
fNetworkNode = fNetworkNodeIn;
fSuccessfullyConnected = false;
fDisconnect = false;
nRefCount = 0;
@ -2418,7 +2423,7 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
nPingUsecStart = 0;
nPingUsecTime = 0;
fPingQueued = false;
fDarkSendMaster = false;
fMasternode = false;
nMinPingUsecTime = std::numeric_limits<int64_t>::max();
{
@ -2426,6 +2431,9 @@ CNode::CNode(SOCKET hSocketIn, const CAddress& addrIn, const std::string& addrNa
id = nLastNodeId++;
}
if(fNetworkNode || fInbound)
AddRef();
if (fLogIPs)
LogPrint("net", "Added connection to %s peer=%d\n", addrName, id);
else

View File

@ -82,7 +82,9 @@ CNode* FindNode(const CNetAddr& ip);
CNode* FindNode(const CSubNet& subNet);
CNode* FindNode(const std::string& addrName);
CNode* FindNode(const CService& ip);
CNode* ConnectNode(CAddress addrConnect, const char *pszDest = NULL, bool darkSendMaster=false);
// fConnectToMasternode should be 'true' only if you want this node to allow to connect to itself
// and/or you want it to be disconnected on CMasternodeMan::ProcessMasternodeConnections()
CNode* ConnectNode(CAddress addrConnect, const char *pszDest = NULL, bool fConnectToMasternode = false);
bool OpenNetworkConnection(const CAddress& addrConnect, CSemaphoreGrant *grantOutbound = NULL, const char *strDest = NULL, bool fOneShot = false);
void MapPort(bool fUseUPnP);
unsigned short GetListenPort();
@ -355,12 +357,8 @@ public:
// b) the peer may tell us in its version message that we should not relay tx invs
// unless it loads a bloom filter.
bool fRelayTxes;
// Should be 'true' only if we connected to this node to actually mix funds.
// In this case node will be released automatically via CMasternodeMan::ProcessMasternodeConnections().
// Connecting to verify connectability/status or connecting for sending/relaying single message
// (even if it's relative to mixing e.g. for blinding) should NOT set this to 'true'.
// For such cases node should be released manually (preferably right after corresponding code).
bool fDarkSendMaster;
// If 'true' this node will be disconnected on CMasternodeMan::ProcessMasternodeConnections()
bool fMasternode;
CSemaphoreGrant grantOutbound;
CCriticalSection cs_filter;
CBloomFilter* pfilter;
@ -419,7 +417,7 @@ public:
// Whether a ping is requested.
bool fPingQueued;
CNode(SOCKET hSocketIn, const CAddress &addrIn, const std::string &addrNameIn = "", bool fInboundIn = false);
CNode(SOCKET hSocketIn, const CAddress &addrIn, const std::string &addrNameIn = "", bool fInboundIn = false, bool fNetworkNodeIn = false);
~CNode();
private:

View File

@ -138,9 +138,8 @@ UniValue masternode(const UniValue& params, bool fHelp)
CService addr = CService(strAddress);
CNode *pnode = ConnectNode((CAddress)addr, NULL, false);
if(pnode){
pnode->Release();
CNode *pnode = ConnectNode((CAddress)addr, NULL);
if(pnode) {
return "successfully connected";
} else {
throw JSONRPCError(RPC_INTERNAL_ERROR, "Error connecting");