Merge #9225: Fix some benign races

dfed983 Fix unlocked access to vNodes.size() (Matt Corallo)
3033522 Remove double brackets in addrman (Matt Corallo)
dbfaade Fix AddrMan locking (Matt Corallo)
047ea10 Make fImporting an std::atomic (Matt Corallo)
42071ca Make fDisconnect an std::atomic (Matt Corallo)
This commit is contained in:
Wladimir J. van der Laan 2016-11-29 12:38:12 +01:00
commit 5488514b90
No known key found for this signature in database
GPG key ID: 74810B012346C9A6
5 changed files with 32 additions and 36 deletions

View file

@ -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<CAddress> &vAddr, const CNetAddr& source, int64_t nTimePenalty = 0)
{
LOCK(cs);
int nAdd = 0;
{
LOCK(cs);
Check();
for (std::vector<CAddress>::const_iterator it = vAddr.begin(); it != vAddr.end(); it++)
nAdd += Add_(*it, source, nTimePenalty) ? 1 : 0;
Check();
}
Check();
for (std::vector<CAddress>::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;
@ -532,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();
}
/**
@ -582,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)

View file

@ -69,7 +69,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;

View file

@ -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;

View file

@ -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);
}

View file

@ -615,7 +615,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