Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net: Add Clang thread safety annotations for guarded variables in the networking code #13123

Merged
merged 2 commits into from
Nov 28, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 7 additions & 13 deletions src/net.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ bool fDiscover = true;
bool fListen = true;
bool fRelayTxes = true;
CCriticalSection cs_mapLocalHost;
std::map<CNetAddr, LocalServiceInfo> mapLocalHost;
static bool vfLimited[NET_MAX] = {};
std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
static bool vfLimited[NET_MAX] GUARDED_BY(cs_mapLocalHost) = {};
std::string strSubVersion;

limitedmap<uint256, int64_t> mapAlreadyAskedFor(MAX_INV_SZ);
Expand Down Expand Up @@ -715,7 +715,10 @@ void CNode::copyStats(CNodeStats &stats)
X(nRecvBytes);
}
X(fWhitelisted);
X(minFeeFilter);
{
LOCK(cs_feeFilter);
X(minFeeFilter);
}

// It is common for nodes with good ping times to suddenly become lagged,
// due to a new block arriving or other large transfer.
Expand Down Expand Up @@ -874,16 +877,7 @@ const uint256& CNetMessage::GetMessageHash() const
return data_hash;
}









// requires LOCK(cs_vSend)
size_t CConnman::SocketSendData(CNode *pnode) const
size_t CConnman::SocketSendData(CNode *pnode) const EXCLUSIVE_LOCKS_REQUIRED(pnode->cs_vSend)
{
auto it = pnode->vSendMsg.begin();
size_t nSentSize = 0;
Expand Down
38 changes: 19 additions & 19 deletions src/net.h
Original file line number Diff line number Diff line change
Expand Up @@ -400,12 +400,12 @@ class CConnman

std::vector<ListenSocket> vhListenSocket;
std::atomic<bool> fNetworkActive;
banmap_t setBanned;
banmap_t setBanned GUARDED_BY(cs_setBanned);
CCriticalSection cs_setBanned;
bool setBannedIsDirty;
bool setBannedIsDirty GUARDED_BY(cs_setBanned);
bool fAddressesInitialized;
CAddrMan addrman;
std::deque<std::string> vOneShots;
std::deque<std::string> vOneShots GUARDED_BY(cs_vOneShots);
CCriticalSection cs_vOneShots;
std::vector<std::string> vAddedNodes GUARDED_BY(cs_vAddedNodes);
CCriticalSection cs_vAddedNodes;
Expand Down Expand Up @@ -540,7 +540,7 @@ struct LocalServiceInfo {
};

extern CCriticalSection cs_mapLocalHost;
extern std::map<CNetAddr, LocalServiceInfo> mapLocalHost;
extern std::map<CNetAddr, LocalServiceInfo> mapLocalHost GUARDED_BY(cs_mapLocalHost);
typedef std::map<std::string, uint64_t> mapMsgCmdSize; //command, total bytes

class CNodeStats
Expand Down Expand Up @@ -630,23 +630,23 @@ class CNode
public:
// socket
std::atomic<ServiceFlags> nServices;
SOCKET hSocket;
SOCKET hSocket GUARDED_BY(cs_hSocket);
size_t nSendSize; // total size of all vSendMsg entries
size_t nSendOffset; // offset inside the first vSendMsg already sent
uint64_t nSendBytes;
std::deque<std::vector<unsigned char>> vSendMsg;
uint64_t nSendBytes GUARDED_BY(cs_vSend);
std::deque<std::vector<unsigned char>> vSendMsg GUARDED_BY(cs_vSend);
CCriticalSection cs_vSend;
CCriticalSection cs_hSocket;
CCriticalSection cs_vRecv;
practicalswift marked this conversation as resolved.
Show resolved Hide resolved

CCriticalSection cs_vProcessMsg;
std::list<CNetMessage> vProcessMsg;
std::list<CNetMessage> vProcessMsg GUARDED_BY(cs_vProcessMsg);
size_t nProcessQueueSize;

CCriticalSection cs_sendProcessing;

std::deque<CInv> vRecvGetData;
uint64_t nRecvBytes;
uint64_t nRecvBytes GUARDED_BY(cs_vRecv);
std::atomic<int> nRecvVersion;

std::atomic<int64_t> nLastSend;
Expand All @@ -662,7 +662,7 @@ class CNode
// 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;
std::string strSubVer GUARDED_BY(cs_SubVer), cleanSubVer GUARDED_BY(cs_SubVer);
practicalswift marked this conversation as resolved.
Show resolved Hide resolved
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.
Expand All @@ -681,7 +681,7 @@ class CNode
bool fSentAddr;
CSemaphoreGrant grantOutbound;
mutable CCriticalSection cs_filter;
std::unique_ptr<CBloomFilter> pfilter;
std::unique_ptr<CBloomFilter> pfilter PT_GUARDED_BY(cs_filter);
std::atomic<int> nRefCount;

const uint64_t nKeyedNetGroup;
Expand All @@ -690,7 +690,7 @@ class CNode
protected:

mapMsgCmdSize mapSendBytesPerMsgCmd;
mapMsgCmdSize mapRecvBytesPerMsgCmd;
mapMsgCmdSize mapRecvBytesPerMsgCmd GUARDED_BY(cs_vRecv);

public:
uint256 hashContinue;
Expand All @@ -701,18 +701,18 @@ class CNode
CRollingBloomFilter addrKnown;
bool fGetAddr;
std::set<uint256> setKnown;
int64_t nNextAddrSend;
int64_t nNextLocalAddrSend;
int64_t nNextAddrSend GUARDED_BY(cs_sendProcessing);
int64_t nNextLocalAddrSend GUARDED_BY(cs_sendProcessing);

// inventory based relay
CRollingBloomFilter filterInventoryKnown;
CRollingBloomFilter filterInventoryKnown GUARDED_BY(cs_inventory);
// Set of transaction ids we still have to announce.
// They are sorted by the mempool before relay, so the order is not important.
std::set<uint256> setInventoryTxToSend;
// List of block ids we still have announce.
// There is no final sorting before sending, as they are always sent immediately
// and in the order requested.
std::vector<uint256> vInventoryBlockToSend;
std::vector<uint256> vInventoryBlockToSend GUARDED_BY(cs_inventory);
CCriticalSection cs_inventory;
std::set<uint256> setAskFor;
std::multimap<int64_t, CInv> mapAskFor;
Expand Down Expand Up @@ -741,7 +741,7 @@ class CNode
// Whether a ping is requested.
std::atomic<bool> fPingQueued;
// Minimum fee rate with which to filter inv's to this node
CAmount minFeeFilter;
CAmount minFeeFilter GUARDED_BY(cs_feeFilter);
CCriticalSection cs_feeFilter;
CAmount lastSentFeeFilter;
int64_t nextSendTimeFeeFilter;
Expand All @@ -761,10 +761,10 @@ class CNode
std::list<CNetMessage> vRecvMsg; // Used only by SocketHandler thread

mutable CCriticalSection cs_addrName;
std::string addrName;
std::string addrName GUARDED_BY(cs_addrName);

// Our address, as reported by the peer
CService addrLocal;
CService addrLocal GUARDED_BY(cs_addrLocal);
mutable CCriticalSection cs_addrLocal;
public:

Expand Down
4 changes: 2 additions & 2 deletions src/netbase.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@
#endif

// Settings
static proxyType proxyInfo[NET_MAX];
static proxyType nameProxy;
static CCriticalSection cs_proxyInfos;
static proxyType proxyInfo[NET_MAX] GUARDED_BY(cs_proxyInfos);
static proxyType nameProxy GUARDED_BY(cs_proxyInfos);
int nConnectTimeout = DEFAULT_CONNECT_TIMEOUT;
bool fNameLookup = DEFAULT_NAME_LOOKUP;

Expand Down