From db2dc7a58cb0a3df58188b748df8e0d04ba76f00 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 6 Feb 2017 12:18:51 -0500 Subject: Move CNode::addrLocal access behind locked accessors --- src/net.cpp | 24 ++++++++++++++++++++---- src/net.h | 8 +++++++- src/net_processing.cpp | 4 ++-- 3 files changed, 29 insertions(+), 7 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 8aa1261984..505eb971c0 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()) { @@ -606,6 +607,20 @@ void CNode::MaybeSetAddrName(const std::string& 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) @@ -659,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 diff --git a/src/net.h b/src/net.h index 2cfc74e3d5..29b6a44c88 100644 --- a/src/net.h +++ b/src/net.h @@ -590,7 +590,6 @@ public: const int64_t nTimeConnected; std::atomic nTimeOffset; const CAddress addr; - 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 @@ -698,6 +697,9 @@ private: mutable CCriticalSection cs_addrName; std::string addrName; + + CService addrLocal; + mutable CCriticalSection cs_addrLocal; public: NodeId GetId() const { @@ -731,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++; diff --git a/src/net_processing.cpp b/src/net_processing.cpp index b0c9b3c71b..7d76aa0b48 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -1274,7 +1274,7 @@ 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->SetAddrLocal(addrMe); { LOCK(pfrom->cs_SubVer); pfrom->strSubVer = strSubVer; @@ -1315,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); } -- cgit v1.2.3