From 42071ca2640c6de156d72a32b52bff1020c4a38d Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Nov 2016 18:03:12 -0800 Subject: [PATCH 1/5] Make fDisconnect an std::atomic --- src/net.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/net.h b/src/net.h index 7a32e21f4..31385c231 100644 --- a/src/net.h +++ b/src/net.h @@ -630,7 +630,7 @@ public: const bool fInbound; bool fNetworkNode; bool fSuccessfullyConnected; - bool fDisconnect; + std::atomic_bool fDisconnect; // We use fRelayTxes for two purposes - // a) it allows us to not relay tx invs before receiving the peer's version message // b) the peer may tell us in its version message that we should not relay tx invs From 047ea1052d4f20c762b1b75099c8421932021b8a Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Nov 2016 21:52:44 -0800 Subject: [PATCH 2/5] Make fImporting an std::atomic --- src/main.cpp | 2 +- src/main.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main.cpp b/src/main.cpp index 05442057e..d1ff8699e 100644 --- a/src/main.cpp +++ b/src/main.cpp @@ -68,7 +68,7 @@ int64_t nTimeBestReceived = 0; // Used only to inform the wallet of when we last CWaitableCriticalSection csBestBlock; CConditionVariable cvBlockChange; int nScriptCheckThreads = 0; -bool fImporting = false; +std::atomic_bool fImporting(false); bool fReindex = false; bool fTxIndex = false; bool fHavePruned = false; diff --git a/src/main.h b/src/main.h index 43c62f6de..c98ed0572 100644 --- a/src/main.h +++ b/src/main.h @@ -165,7 +165,7 @@ extern uint64_t nLastBlockWeight; extern const std::string strMessageMagic; extern CWaitableCriticalSection csBestBlock; extern CConditionVariable cvBlockChange; -extern bool fImporting; +extern std::atomic_bool fImporting; extern bool fReindex; extern int nScriptCheckThreads; extern bool fTxIndex; From dbfaade72a9d9c1e503c47f8e12d26b1540a921b Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Nov 2016 18:11:25 -0800 Subject: [PATCH 3/5] Fix AddrMan locking --- src/addrman.h | 23 ++++++++++------------- 1 file changed, 10 insertions(+), 13 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index cabacbbea..d466eefb8 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -482,6 +482,7 @@ public: //! Return the number of (unique) addresses in all tables. size_t size() const { + LOCK(cs); // TODO: Cache this in an atomic to avoid this overhead return vRandom.size(); } @@ -501,13 +502,11 @@ public: //! Add a single address. bool Add(const CAddress &addr, const CNetAddr& source, int64_t nTimePenalty = 0) { + LOCK(cs); bool fRet = false; - { - LOCK(cs); - Check(); - fRet |= Add_(addr, source, nTimePenalty); - Check(); - } + Check(); + fRet |= Add_(addr, source, nTimePenalty); + Check(); if (fRet) LogPrint("addrman", "Added %s from %s: %i tried, %i new\n", addr.ToStringIPPort(), source.ToString(), nTried, nNew); return fRet; @@ -516,14 +515,12 @@ public: //! Add multiple addresses. bool Add(const std::vector &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0) { + LOCK(cs); int nAdd = 0; - { - LOCK(cs); - Check(); - for (std::vector::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) - nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; - Check(); - } + Check(); + for (std::vector::const_iterator it = vAddr.begin(); it != vAddr.end(); it++) + nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0; + Check(); if (nAdd) LogPrint("addrman", "Added %i addresses from %s: %i tried, %i new\n", nAdd, source.ToString(), nTried, nNew); return nAdd > 0; From 303352286f2af0f12fd5ae7f65ad48330b5a00ec Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Nov 2016 18:13:48 -0800 Subject: [PATCH 4/5] Remove double brackets in addrman --- src/addrman.h | 30 ++++++++++++------------------ 1 file changed, 12 insertions(+), 18 deletions(-) diff --git a/src/addrman.h b/src/addrman.h index d466eefb8..76e7b210f 100644 --- a/src/addrman.h +++ b/src/addrman.h @@ -529,23 +529,19 @@ public: //! Mark an entry as accessible. void Good(const CService &addr, int64_t nTime = GetAdjustedTime()) { - { - LOCK(cs); - Check(); - Good_(addr, nTime); - Check(); - } + LOCK(cs); + Check(); + Good_(addr, nTime); + Check(); } //! Mark an entry as connection attempted to. void Attempt(const CService &addr, bool fCountFailure, int64_t nTime = GetAdjustedTime()) { - { - LOCK(cs); - Check(); - Attempt_(addr, fCountFailure, nTime); - Check(); - } + LOCK(cs); + Check(); + Attempt_(addr, fCountFailure, nTime); + Check(); } /** @@ -579,12 +575,10 @@ public: //! Mark an entry as currently-connected-to. void Connected(const CService &addr, int64_t nTime = GetAdjustedTime()) { - { - LOCK(cs); - Check(); - Connected_(addr, nTime); - Check(); - } + LOCK(cs); + Check(); + Connected_(addr, nTime); + Check(); } void SetServices(const CService &addr, ServiceFlags nServices) From dfed983f19e7b61da243362f00fa3a14c92dae45 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Fri, 25 Nov 2016 19:29:55 -0800 Subject: [PATCH 5/5] Fix unlocked access to vNodes.size() --- src/net.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/net.cpp b/src/net.cpp index 15c4514f1..233c0fbc8 100644 --- a/src/net.cpp +++ b/src/net.cpp @@ -1103,8 +1103,13 @@ void CConnman::ThreadSocketHandler() } } } - if(vNodes.size() != nPrevNodeCount) { - nPrevNodeCount = vNodes.size(); + size_t vNodesSize; + { + LOCK(cs_vNodes); + vNodesSize = vNodes.size(); + } + if(vNodesSize != nPrevNodeCount) { + nPrevNodeCount = vNodesSize; if(clientInterface) clientInterface->NotifyNumConnectionsChanged(nPrevNodeCount); }