diff --git a/src/net.cpp b/src/net.cpp index 7c45cff1d..505eb971c 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -164,8 +164,9 @@ int GetnScore(const CService& addr) // Is our peer's addrLocal potentially useful as an external IP source? bool IsPeerAddrLocalGood(CNode *pnode) { - return fDiscover && pnode->addr.IsRoutable() && pnode->addrLocal.IsRoutable() && - !IsLimited(pnode->addrLocal.GetNetwork()); + CService addrLocal = pnode->GetAddrLocal(); + return fDiscover && pnode->addr.IsRoutable() && addrLocal.IsRoutable() && + !IsLimited(addrLocal.GetNetwork()); } // pushes our own address to a peer @@ -180,7 +181,7 @@ void AdvertiseLocal(CNode *pnode) if (IsPeerAddrLocalGood(pnode) && (!addrLocal.IsRoutable() || GetRand((GetnScore(addrLocal) > LOCAL_MANUAL) ? 8:2) == 0)) { - addrLocal.SetIP(pnode->addrLocal); + addrLocal.SetIP(pnode->GetAddrLocal()); } if (addrLocal.IsRoutable()) { @@ -307,9 +308,11 @@ CNode* CConnman::FindNode(const CSubNet& subNet) CNode* CConnman::FindNode(const std::string& addrName) { LOCK(cs_vNodes); - BOOST_FOREACH(CNode* pnode, vNodes) - if (pnode->addrName == addrName) + BOOST_FOREACH(CNode* pnode, vNodes) { + if (pnode->GetAddrName() == addrName) { return (pnode); + } + } return NULL; } @@ -373,9 +376,7 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo CNode* pnode = FindNode((CService)addrConnect); if (pnode) { - if (pnode->addrName.empty()) { - pnode->addrName = std::string(pszDest); - } + pnode->MaybeSetAddrName(std::string(pszDest)); CloseSocket(hSocket); LogPrintf("Failed to open new connection, already connected\n"); return NULL; @@ -389,7 +390,6 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo uint64_t nonce = GetDeterministicRandomizer(RANDOMIZER_ID_LOCALHOSTNONCE).Write(id).Finalize(); CNode* pnode = new CNode(id, nLocalServices, GetBestHeight(), hSocket, addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, pszDest ? pszDest : "", false); pnode->nServicesExpected = ServiceFlags(addrConnect.nServices & nRelevantServices); - pnode->nTimeConnected = GetSystemTimeInSeconds(); pnode->AddRef(); return pnode; @@ -594,6 +594,33 @@ void CConnman::AddWhitelistedRange(const CSubNet &subnet) { vWhitelistedRange.push_back(subnet); } + +std::string CNode::GetAddrName() const { + LOCK(cs_addrName); + return addrName; +} + +void CNode::MaybeSetAddrName(const std::string& addrNameIn) { + LOCK(cs_addrName); + if (addrName.empty()) { + addrName = addrNameIn; + } +} + +CService CNode::GetAddrLocal() const { + LOCK(cs_addrLocal); + return addrLocal; +} + +void CNode::SetAddrLocal(const CService& addrLocalIn) { + LOCK(cs_addrLocal); + if (addrLocal.IsValid()) { + error("Addr local already set for node: %i. Refusing to change from %s to %s", id, addrLocal.ToString(), addrLocalIn.ToString()); + } else { + addrLocal = addrLocalIn; + } +} + #undef X #define X(name) stats.name = name void CNode::copyStats(CNodeStats &stats) @@ -601,21 +628,33 @@ void CNode::copyStats(CNodeStats &stats) stats.nodeid = this->GetId(); X(nServices); X(addr); - X(fRelayTxes); + { + LOCK(cs_filter); + X(fRelayTxes); + } X(nLastSend); X(nLastRecv); X(nTimeConnected); X(nTimeOffset); - X(addrName); + stats.addrName = GetAddrName(); X(nVersion); - X(cleanSubVer); + { + LOCK(cs_SubVer); + X(cleanSubVer); + } X(fInbound); X(fAddnode); X(nStartingHeight); - X(nSendBytes); - X(mapSendBytesPerMsgCmd); - X(nRecvBytes); - X(mapRecvBytesPerMsgCmd); + { + LOCK(cs_vSend); + X(mapSendBytesPerMsgCmd); + X(nSendBytes); + } + { + LOCK(cs_vRecv); + X(mapRecvBytesPerMsgCmd); + X(nRecvBytes); + } X(fWhitelisted); // It is common for nodes with good ping times to suddenly become lagged, @@ -635,7 +674,8 @@ void CNode::copyStats(CNodeStats &stats) stats.dPingWait = (((double)nPingUsecWait) / 1e6); // Leave string empty if addrLocal invalid (not filled in yet) - stats.addrLocal = addrLocal.IsValid() ? addrLocal.ToString() : ""; + CService addrLocalUnlocked = GetAddrLocal(); + stats.addrLocal = addrLocalUnlocked.IsValid() ? addrLocalUnlocked.ToString() : ""; } #undef X @@ -643,6 +683,7 @@ bool CNode::ReceiveMsgBytes(const char *pch, unsigned int nBytes, bool& complete { complete = false; int64_t nTimeMicros = GetTimeMicros(); + LOCK(cs_vRecv); nLastRecv = nTimeMicros / 1000000; nRecvBytes += nBytes; while (nBytes > 0) { @@ -1786,8 +1827,9 @@ std::vector CConnman::GetAddedNodeInfo() if (pnode->addr.IsValid()) { mapConnected[pnode->addr] = pnode->fInbound; } - if (!pnode->addrName.empty()) { - mapConnectedByName[pnode->addrName] = std::make_pair(pnode->fInbound, static_cast(pnode->addr)); + std::string addrName = pnode->GetAddrName(); + if (!addrName.empty()) { + mapConnectedByName[std::move(addrName)] = std::make_pair(pnode->fInbound, static_cast(pnode->addr)); } } } @@ -2414,9 +2456,8 @@ void CConnman::GetNodeStats(std::vector& vstats) vstats.reserve(vNodes.size()); for(std::vector::iterator it = vNodes.begin(); it != vNodes.end(); ++it) { CNode* pnode = *it; - CNodeStats stats; - pnode->copyStats(stats); - vstats.push_back(stats); + vstats.emplace_back(); + pnode->copyStats(vstats.back()); } } @@ -2568,6 +2609,7 @@ unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; } unsigned int CConnman::GetSendBufferSize() const{ return nSendBufferMaxSize; } CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const std::string& addrNameIn, bool fInboundIn) : + nTimeConnected(GetSystemTimeInSeconds()), addr(addrIn), fInbound(fInboundIn), id(idIn), @@ -2587,7 +2629,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, int nMyStartingHeightIn nLastRecv = 0; nSendBytes = 0; nRecvBytes = 0; - nTimeConnected = GetSystemTimeInSeconds(); nTimeOffset = 0; addrName = addrNameIn == "" ? addr.ToStringIPPort() : addrNameIn; nVersion = 0; diff --git a/src/net.h b/src/net.h index e5a19e0f4..29b6a44c8 100644 --- a/src/net.h +++ b/src/net.h @@ -564,7 +564,7 @@ class CNode friend class CConnman; public: // socket - ServiceFlags nServices; + std::atomic nServices; ServiceFlags nServicesExpected; SOCKET hSocket; size_t nSendSize; // total size of all vSendMsg entries @@ -573,6 +573,7 @@ public: std::deque> vSendMsg; CCriticalSection cs_vSend; CCriticalSection cs_hSocket; + CCriticalSection cs_vRecv; CCriticalSection cs_vProcessMsg; std::list vProcessMsg; @@ -584,19 +585,18 @@ public: uint64_t nRecvBytes; std::atomic nRecvVersion; - int64_t nLastSend; - int64_t nLastRecv; - int64_t nTimeConnected; - int64_t nTimeOffset; + std::atomic nLastSend; + std::atomic nLastRecv; + const int64_t nTimeConnected; + std::atomic nTimeOffset; const CAddress addr; - std::string addrName; - CService addrLocal; std::atomic nVersion; // strSubVer is whatever byte array we read from the wire. However, this field is intended // to be printed out, displayed to humans in various forms and so on. So we sanitize it and // store the sanitized version in cleanSubVer. The original should be used when dealing with // the network or wire types and the cleaned string used when displayed or logged. std::string strSubVer, cleanSubVer; + CCriticalSection cs_SubVer; // used for both cleanSubVer and strSubVer bool fWhitelisted; // This peer can bypass DoS banning. bool fFeeler; // If true this node is being used as a short lived feeler. bool fOneShot; @@ -614,7 +614,7 @@ public: CSemaphoreGrant grantOutbound; CCriticalSection cs_filter; CBloomFilter* pfilter; - int nRefCount; + std::atomic nRefCount; const NodeId id; const uint64_t nKeyedNetGroup; @@ -627,7 +627,7 @@ protected: public: uint256 hashContinue; - int nStartingHeight; + std::atomic nStartingHeight; // flood relay std::vector vAddrToSend; @@ -665,15 +665,15 @@ public: // Ping time measurement: // The pong reply we're expecting, or 0 if no pong expected. - uint64_t nPingNonceSent; + std::atomic nPingNonceSent; // Time (in usec) the last ping was sent, or 0 if no ping was ever sent. - int64_t nPingUsecStart; + std::atomic nPingUsecStart; // Last measured round-trip time. - int64_t nPingUsecTime; + std::atomic nPingUsecTime; // Best measured round-trip time. - int64_t nMinPingUsecTime; + std::atomic nMinPingUsecTime; // Whether a ping is requested. - bool fPingQueued; + std::atomic fPingQueued; // Minimum fee rate with which to filter inv's to this node CAmount minFeeFilter; CCriticalSection cs_feeFilter; @@ -694,6 +694,12 @@ private: const int nMyStartingHeight; int nSendVersion; std::list vRecvMsg; // Used only by SocketHandler thread + + mutable CCriticalSection cs_addrName; + std::string addrName; + + CService addrLocal; + mutable CCriticalSection cs_addrLocal; public: NodeId GetId() const { @@ -727,6 +733,10 @@ public: void SetSendVersion(int nVersionIn); int GetSendVersion() const; + CService GetAddrLocal() const; + //! May not be called more than once + void SetAddrLocal(const CService& addrLocalIn); + CNode* AddRef() { nRefCount++; @@ -796,6 +806,10 @@ public: { return nLocalServices; } + + std::string GetAddrName() const; + //! Sets the addrName only if it was not previously set + void MaybeSetAddrName(const std::string& addrNameIn); }; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index bb14e69d8..7d76aa0b4 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -36,7 +36,7 @@ # error "Bitcoin cannot be compiled without assertions." #endif -int64_t nTimeBestReceived = 0; // Used only to inform the wallet of when we last received a block +std::atomic nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block struct IteratorComparator { @@ -264,7 +264,7 @@ void PushNodeVersion(CNode *pnode, CConnman& connman, int64_t nTime) void InitializeNode(CNode *pnode, CConnman& connman) { CAddress addr = pnode->addr; - std::string addrName = pnode->addrName; + std::string addrName = pnode->GetAddrName(); NodeId nodeid = pnode->GetId(); { LOCK(cs_main); @@ -1211,6 +1211,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr int nVersion; int nSendVersion; std::string strSubVer; + std::string cleanSubVer; int nStartingHeight = -1; bool fRelay = true; @@ -1246,6 +1247,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr vRecv >> addrFrom >> nNonce; if (!vRecv.empty()) { vRecv >> LIMITED_STRING(strSubVer, MAX_SUBVERSION_LENGTH); + cleanSubVer = SanitizeString(strSubVer); } if (!vRecv.empty()) { vRecv >> nStartingHeight; @@ -1272,9 +1274,12 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr connman.PushMessage(pfrom, CNetMsgMaker(INIT_PROTO_VERSION).Make(NetMsgType::VERACK)); pfrom->nServices = nServices; - pfrom->addrLocal = addrMe; - pfrom->strSubVer = strSubVer; - pfrom->cleanSubVer = SanitizeString(strSubVer); + pfrom->SetAddrLocal(addrMe); + { + LOCK(pfrom->cs_SubVer); + pfrom->strSubVer = strSubVer; + pfrom->cleanSubVer = cleanSubVer; + } pfrom->nStartingHeight = nStartingHeight; pfrom->fClient = !(nServices & NODE_NETWORK); { @@ -1310,7 +1315,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr LogPrint("net", "ProcessMessages: advertising address %s\n", addr.ToString()); pfrom->PushAddress(addr, insecure_rand); } else if (IsPeerAddrLocalGood(pfrom)) { - addr.SetIP(pfrom->addrLocal); + addr.SetIP(addrMe); LogPrint("net", "ProcessMessages: advertising address %s\n", addr.ToString()); pfrom->PushAddress(addr, insecure_rand); } @@ -1330,7 +1335,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr remoteAddr = ", peeraddr=" + pfrom->addr.ToString(); LogPrintf("receive version message: %s: version %d, blocks=%d, us=%s, peer=%d%s\n", - pfrom->cleanSubVer, pfrom->nVersion, + cleanSubVer, pfrom->nVersion, pfrom->nStartingHeight, addrMe.ToString(), pfrom->id, remoteAddr); @@ -2450,7 +2455,7 @@ bool static ProcessMessage(CNode* pfrom, const std::string& strCommand, CDataStr if (pingUsecTime > 0) { // Successful ping time measurement, replace previous pfrom->nPingUsecTime = pingUsecTime; - pfrom->nMinPingUsecTime = std::min(pfrom->nMinPingUsecTime, pingUsecTime); + pfrom->nMinPingUsecTime = std::min(pfrom->nMinPingUsecTime.load(), pingUsecTime); } else { // This should never happen sProblem = "Timing mishap";