From 1e3bcd251768baeb95e555d51d2dc787a6b2acee Mon Sep 17 00:00:00 2001 From: Jesse Cohen Date: Fri, 8 Jun 2018 14:09:08 -0400 Subject: [net_processing] Add thread safety annotations --- src/net_processing.cpp | 91 +++++++++++++++++++++++++------------------------- 1 file changed, 46 insertions(+), 45 deletions(-) (limited to 'src/net_processing.cpp') diff --git a/src/net_processing.cpp b/src/net_processing.cpp index 5267f96f70..d5d664b27e 100644 --- a/src/net_processing.cpp +++ b/src/net_processing.cpp @@ -76,7 +76,7 @@ std::map mapOrphanTransactions GUARDED_BY(g_cs_orphans); void EraseOrphansFor(NodeId peer); /** Increase a node's misbehavior score. */ -void Misbehaving(NodeId nodeid, int howmuch, const std::string& message=""); +void Misbehaving(NodeId nodeid, int howmuch, const std::string& message="") EXCLUSIVE_LOCKS_REQUIRED(cs_main); /** Average delay between local address broadcasts in seconds. */ static constexpr unsigned int AVG_LOCAL_ADDRESS_BROADCAST_INTERVAL = 24 * 60 * 60; @@ -96,22 +96,20 @@ static constexpr unsigned int MAX_FEEFILTER_CHANGE_DELAY = 5 * 60; // Internal stuff namespace { /** Number of nodes with fSyncStarted. */ - int nSyncStarted = 0; + int nSyncStarted GUARDED_BY(cs_main) = 0; /** * Sources of received blocks, saved to be able to send them reject - * messages or ban them when processing happens afterwards. Protected by - * cs_main. + * messages or ban them when processing happens afterwards. * Set mapBlockSource[hash].second to false if the node should not be * punished if the block is invalid. */ - std::map> mapBlockSource; + std::map> mapBlockSource GUARDED_BY(cs_main); /** * Filter for transactions that were recently rejected by * AcceptToMemoryPool. These are not rerequested until the chain tip - * changes, at which point the entire filter is reset. Protected by - * cs_main. + * changes, at which point the entire filter is reset. * * Without this filter we'd be re-requesting txs from each of our peers, * increasing bandwidth consumption considerably. For instance, with 100 @@ -127,38 +125,38 @@ namespace { * * Memory used: 1.3 MB */ - std::unique_ptr recentRejects; - uint256 hashRecentRejectsChainTip; + std::unique_ptr recentRejects GUARDED_BY(cs_main); + uint256 hashRecentRejectsChainTip GUARDED_BY(cs_main); - /** Blocks that are in flight, and that are in the queue to be downloaded. Protected by cs_main. */ + /** Blocks that are in flight, and that are in the queue to be downloaded. */ struct QueuedBlock { uint256 hash; const CBlockIndex* pindex; //!< Optional. bool fValidatedHeaders; //!< Whether this block has validated headers at the time of request. std::unique_ptr partialBlock; //!< Optional, used for CMPCTBLOCK downloads }; - std::map::iterator> > mapBlocksInFlight; + std::map::iterator> > mapBlocksInFlight GUARDED_BY(cs_main); /** Stack of nodes which we have set to announce using compact blocks */ - std::list lNodesAnnouncingHeaderAndIDs; + std::list lNodesAnnouncingHeaderAndIDs GUARDED_BY(cs_main); /** Number of preferable block download peers. */ - int nPreferredDownload = 0; + int nPreferredDownload GUARDED_BY(cs_main) = 0; /** Number of peers from which we're downloading blocks. */ - int nPeersWithValidatedDownloads = 0; + int nPeersWithValidatedDownloads GUARDED_BY(cs_main) = 0; /** Number of outbound peers with m_chain_sync.m_protect. */ - int g_outbound_peers_with_protect_from_disconnect = 0; + int g_outbound_peers_with_protect_from_disconnect GUARDED_BY(cs_main) = 0; /** When our tip was last updated. */ std::atomic g_last_tip_update(0); - /** Relay map, protected by cs_main. */ + /** Relay map */ typedef std::map MapRelay; - MapRelay mapRelay; - /** Expiration-time ordered list of (expire time, relay map entry) pairs, protected by cs_main). */ - std::deque> vRelayExpiration; + MapRelay mapRelay GUARDED_BY(cs_main); + /** Expiration-time ordered list of (expire time, relay map entry) pairs. */ + std::deque> vRelayExpiration GUARDED_BY(cs_main); std::atomic nTimeBestReceived(0); // Used only to inform the wallet of when we last received a block @@ -302,18 +300,17 @@ struct CNodeState { } }; -/** Map maintaining per-node state. Requires cs_main. */ -static std::map mapNodeState; +/** Map maintaining per-node state. */ +static std::map mapNodeState GUARDED_BY(cs_main); -// Requires cs_main. -static CNodeState *State(NodeId pnode) { +static CNodeState *State(NodeId pnode) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::map::iterator it = mapNodeState.find(pnode); if (it == mapNodeState.end()) return nullptr; return &it->second; } -static void UpdatePreferredDownload(CNode* node, CNodeState* state) +static void UpdatePreferredDownload(CNode* node, CNodeState* state) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { nPreferredDownload -= state->fPreferredDownload; @@ -344,10 +341,9 @@ static void PushNodeVersion(CNode *pnode, CConnman* connman, int64_t nTime) } } -// Requires cs_main. // Returns a bool indicating whether we requested this block. // Also used if a block was /not/ received and timed out or started with another peer -static bool MarkBlockAsReceived(const uint256& hash) { +static bool MarkBlockAsReceived(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { std::map::iterator> >::iterator itInFlight = mapBlocksInFlight.find(hash); if (itInFlight != mapBlocksInFlight.end()) { CNodeState *state = State(itInFlight->second.first); @@ -370,10 +366,9 @@ static bool MarkBlockAsReceived(const uint256& hash) { return false; } -// Requires cs_main. // returns false, still setting pit, if the block was already in flight from the same peer // pit will only be valid as long as the same cs_main lock is being held -static bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex* pindex = nullptr, std::list::iterator** pit = nullptr) { +static bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlockIndex* pindex = nullptr, std::list::iterator** pit = nullptr) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { CNodeState *state = State(nodeid); assert(state != nullptr); @@ -407,7 +402,7 @@ static bool MarkBlockAsInFlight(NodeId nodeid, const uint256& hash, const CBlock } /** Check whether the last unknown block a peer advertised is not yet known. */ -static void ProcessBlockAvailability(NodeId nodeid) { +static void ProcessBlockAvailability(NodeId nodeid) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { CNodeState *state = State(nodeid); assert(state != nullptr); @@ -423,7 +418,7 @@ static void ProcessBlockAvailability(NodeId nodeid) { } /** Update tracking information about which blocks a peer is assumed to have. */ -static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) { +static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { CNodeState *state = State(nodeid); assert(state != nullptr); @@ -447,7 +442,8 @@ static void UpdateBlockAvailability(NodeId nodeid, const uint256 &hash) { * lNodesAnnouncingHeaderAndIDs, and keeping that list under a certain size by * removing the first element if necessary. */ -static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) { +static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connman) +{ AssertLockHeld(cs_main); CNodeState* nodestate = State(nodeid); if (!nodestate || !nodestate->fSupportsDesiredCmpctVersion) { @@ -463,11 +459,13 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connma } } connman->ForNode(nodeid, [connman](CNode* pfrom){ + AssertLockHeld(cs_main); uint64_t nCMPCTBLOCKVersion = (pfrom->GetLocalServices() & NODE_WITNESS) ? 2 : 1; if (lNodesAnnouncingHeaderAndIDs.size() >= 3) { // As per BIP152, we only get 3 of our peers to announce // blocks using compact encodings. connman->ForNode(lNodesAnnouncingHeaderAndIDs.front(), [connman, nCMPCTBLOCKVersion](CNode* pnodeStop){ + AssertLockHeld(cs_main); connman->PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetSendVersion()).Make(NetMsgType::SENDCMPCT, /*fAnnounceUsingCMPCTBLOCK=*/false, nCMPCTBLOCKVersion)); return true; }); @@ -480,7 +478,7 @@ static void MaybeSetPeerAsAnnouncingHeaderAndIDs(NodeId nodeid, CConnman* connma } } -static bool TipMayBeStale(const Consensus::Params &consensusParams) +static bool TipMayBeStale(const Consensus::Params &consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { AssertLockHeld(cs_main); if (g_last_tip_update == 0) { @@ -489,14 +487,12 @@ static bool TipMayBeStale(const Consensus::Params &consensusParams) return g_last_tip_update < GetTime() - consensusParams.nPowTargetSpacing * 3 && mapBlocksInFlight.empty(); } -// Requires cs_main -static bool CanDirectFetch(const Consensus::Params &consensusParams) +static bool CanDirectFetch(const Consensus::Params &consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { return chainActive.Tip()->GetBlockTime() > GetAdjustedTime() - consensusParams.nPowTargetSpacing * 20; } -// Requires cs_main -static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) +static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { if (state->pindexBestKnownBlock && pindex == state->pindexBestKnownBlock->GetAncestor(pindex->nHeight)) return true; @@ -507,7 +503,8 @@ static bool PeerHasHeader(CNodeState *state, const CBlockIndex *pindex) /** Update pindexLastCommonBlock and add not-in-flight missing successors to vBlocks, until it has * at most count entries. */ -static void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) { +static void FindNextBlocksToDownload(NodeId nodeid, unsigned int count, std::vector& vBlocks, NodeId& nodeStaller, const Consensus::Params& consensusParams) EXCLUSIVE_LOCKS_REQUIRED(cs_main) +{ if (count == 0) return; @@ -797,10 +794,8 @@ unsigned int LimitOrphanTxSize(unsigned int nMaxOrphans) /** * Mark a misbehaving peer to be banned depending upon the value of `-banscore`. - * - * Requires cs_main. */ -void Misbehaving(NodeId pnode, int howmuch, const std::string& message) +void Misbehaving(NodeId pnode, int howmuch, const std::string& message) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { if (howmuch == 0) return; @@ -898,10 +893,10 @@ void PeerLogicValidation::BlockConnected(const std::shared_ptr& pb // All of the following cache a recent block, and are protected by cs_most_recent_block static CCriticalSection cs_most_recent_block; -static std::shared_ptr most_recent_block; -static std::shared_ptr most_recent_compact_block; -static uint256 most_recent_block_hash; -static bool fWitnessesPresentInMostRecentCompactBlock; +static std::shared_ptr most_recent_block GUARDED_BY(cs_most_recent_block); +static std::shared_ptr most_recent_compact_block GUARDED_BY(cs_most_recent_block); +static uint256 most_recent_block_hash GUARDED_BY(cs_most_recent_block); +static bool fWitnessesPresentInMostRecentCompactBlock GUARDED_BY(cs_most_recent_block); /** * Maintain state about the best-seen block and fast-announce a compact block @@ -930,6 +925,8 @@ void PeerLogicValidation::NewPoWValidBlock(const CBlockIndex *pindex, const std: } connman->ForEachNode([this, &pcmpctblock, pindex, &msgMaker, fWitnessEnabled, &hashBlock](CNode* pnode) { + AssertLockHeld(cs_main); + // TODO: Avoid the repeated-serialization here if (pnode->nVersion < INVALID_CB_NO_BAN_VERSION || pnode->fDisconnect) return; @@ -1327,7 +1324,7 @@ void static ProcessGetData(CNode* pfrom, const CChainParams& chainparams, CConnm } } -static uint32_t GetFetchFlags(CNode* pfrom) { +static uint32_t GetFetchFlags(CNode* pfrom) EXCLUSIVE_LOCKS_REQUIRED(cs_main) { uint32_t nFetchFlags = 0; if ((pfrom->GetLocalServices() & NODE_WITNESS) && State(pfrom->GetId())->fHaveWitness) { nFetchFlags |= MSG_WITNESS_FLAG; @@ -3160,6 +3157,8 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) LOCK(cs_main); connman->ForEachNode([&](CNode* pnode) { + AssertLockHeld(cs_main); + // Ignore non-outbound peers, or nodes marked for disconnect already if (!IsOutboundDisconnectionCandidate(pnode) || pnode->fDisconnect) return; CNodeState *state = State(pnode->GetId()); @@ -3173,6 +3172,8 @@ void PeerLogicValidation::EvictExtraOutboundPeers(int64_t time_in_seconds) }); if (worst_peer != -1) { bool disconnected = connman->ForNode(worst_peer, [&](CNode *pnode) { + AssertLockHeld(cs_main); + // Only disconnect a peer that has been connected to us for // some reasonable fraction of our check-frequency, to give // it time for new information to have arrived. -- cgit v1.2.3