Skip to content

Commit

Permalink
merge bitcoin#25109: Strengthen AssertLockNotHeld assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
@kwvg
kwvg committed May 8, 2024
1 parent e3e101b commit 6d5ff0a
Show file tree
Hide file tree
Showing 15 changed files with 148 additions and 112 deletions.
10 changes: 5 additions & 5 deletions src/checkqueue.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ class CCheckQueue
bool m_request_stop GUARDED_BY(m_mutex){false};

/** Internal function that does bulk of the verification work. */
bool Loop(bool fMaster)
bool Loop(bool fMaster) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
std::condition_variable& cond = fMaster ? m_master_cv : m_worker_cv;
std::vector<T> vChecks;
Expand Down Expand Up @@ -139,7 +139,7 @@ class CCheckQueue
}

//! Create a pool of new worker threads.
void StartWorkerThreads(const int threads_num)
void StartWorkerThreads(const int threads_num) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
{
LOCK(m_mutex);
Expand All @@ -157,13 +157,13 @@ class CCheckQueue
}

//! Wait until execution finishes, and return whether all evaluations were successful.
bool Wait()
bool Wait() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
return Loop(true /* master thread */);
}

//! Add a batch of checks to the queue
void Add(std::vector<T>& vChecks)
void Add(std::vector<T>& vChecks) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
if (vChecks.empty()) {
return;
Expand All @@ -186,7 +186,7 @@ class CCheckQueue
}

//! Stop all of the worker threads.
void StopWorkerThreads()
void StopWorkerThreads() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex)
{
WITH_LOCK(m_mutex, m_request_stop = true);
m_worker_cv.notify_all();
Expand Down
17 changes: 12 additions & 5 deletions src/coinjoin/coinjoin.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -368,15 +368,22 @@ class CDSTXManager
void AddDSTX(const CCoinJoinBroadcastTx& dstx) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
CCoinJoinBroadcastTx GetDSTX(const uint256& hash) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);

void UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync);
void NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler, const CMasternodeSync& mn_sync);
void UpdatedBlockTip(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler,
const CMasternodeSync& mn_sync)
EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void NotifyChainLock(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler,
const CMasternodeSync& mn_sync)
EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);

void TransactionAddedToMempool(const CTransactionRef& tx) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex*) EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void BlockConnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex* pindex)
EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void BlockDisconnected(const std::shared_ptr<const CBlock>& pblock, const CBlockIndex*)
EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);

private:
void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler);
void CheckDSTXes(const CBlockIndex* pindex, const llmq::CChainLocksHandler& clhandler)
EXCLUSIVE_LOCKS_REQUIRED(!cs_mapdstx);
void UpdateDSTXConfirmedHeight(const CTransactionRef& tx, std::optional<int> nHeight);

};
Expand Down
6 changes: 3 additions & 3 deletions src/httpserver.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ class WorkQueue
{
}
/** Enqueue a work item */
bool Enqueue(WorkItem* item)
bool Enqueue(WorkItem* item) EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
if (!running || queue.size() >= maxDepth) {
Expand All @@ -94,7 +94,7 @@ class WorkQueue
return true;
}
/** Thread function */
void Run()
void Run() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
while (true) {
std::unique_ptr<WorkItem> i;
Expand All @@ -111,7 +111,7 @@ class WorkQueue
}
}
/** Interrupt and exit loops */
void Interrupt()
void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!cs)
{
LOCK(cs);
running = false;
Expand Down
6 changes: 3 additions & 3 deletions src/i2p.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ class Session
* to the listening socket and address.
* @return true on success
*/
bool Listen(Connection& conn);
bool Listen(Connection& conn) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);

/**
* Wait for and accept a new incoming connection.
Expand All @@ -103,7 +103,7 @@ class Session
* it is set to `false`. Only set if `false` is returned.
* @return true on success
*/
bool Connect(const CService& to, Connection& conn, bool& proxy_error);
bool Connect(const CService& to, Connection& conn, bool& proxy_error) EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);

private:
/**
Expand Down Expand Up @@ -172,7 +172,7 @@ class Session
/**
* Check the control socket for errors and possibly disconnect.
*/
void CheckControlSock();
void CheckControlSock() EXCLUSIVE_LOCKS_REQUIRED(!m_mutex);

/**
* Generate a new destination with the SAM proxy and set `m_private_key` to it.
Expand Down
2 changes: 1 addition & 1 deletion src/index/blockfilterindex.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class BlockFilterIndex final : public BaseIndex
bool LookupFilter(const CBlockIndex* block_index, BlockFilter& filter_out) const;

/** Get a single filter header by block. */
bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out);
bool LookupFilterHeader(const CBlockIndex* block_index, uint256& header_out) EXCLUSIVE_LOCKS_REQUIRED(!m_cs_headers_cache);

/** Get a range of filters between two heights on a chain. */
bool LookupFilterRange(int start_height, const CBlockIndex* stop_index,
Expand Down
1 change: 0 additions & 1 deletion src/net.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2986,7 +2986,6 @@ void CConnman::OpenNetworkConnection(const CAddress& addrConnect, bool fCountFai
}

void CConnman::OpenMasternodeConnection(const CAddress &addrConnect, MasternodeProbeConn probe) {

OpenNetworkConnection(addrConnect, false, nullptr, nullptr, ConnectionType::OUTBOUND_FULL_RELAY, MasternodeConn::IsConnection, probe);
}

Expand Down
75 changes: 43 additions & 32 deletions src/net.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -652,7 +652,7 @@ class CNode
* @return True if the peer should stay connected,
* False if the peer should be disconnected from.
*/
bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete);
bool ReceiveMsgBytes(Span<const uint8_t> msg_bytes, bool& complete) EXCLUSIVE_LOCKS_REQUIRED(!cs_vRecv);

void SetCommonVersion(int greatest_common_version)
{
Expand All @@ -664,9 +664,9 @@ class CNode
return m_greatest_common_version;
}

CService GetAddrLocal() const LOCKS_EXCLUDED(m_addr_local_mutex);
CService GetAddrLocal() const EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex);
//! May not be called more than once
void SetAddrLocal(const CService& addrLocalIn) LOCKS_EXCLUDED(m_addr_local_mutex);
void SetAddrLocal(const CService& addrLocalIn) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_local_mutex);

CNode* AddRef()
{
Expand All @@ -679,9 +679,9 @@ class CNode
nRefCount--;
}

void CloseSocketDisconnect(CConnman* connman);
void CloseSocketDisconnect(CConnman* connman) EXCLUSIVE_LOCKS_REQUIRED(!cs_hSocket);

void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap);
void copyStats(CNodeStats &stats, const std::vector<bool> &m_asmap) EXCLUSIVE_LOCKS_REQUIRED(!m_subver_mutex, !m_addr_local_mutex, !cs_vSend, !cs_vRecv);

ServiceFlags GetLocalServices() const
{
Expand Down Expand Up @@ -857,7 +857,7 @@ friend class CNode;
bool m_i2p_accept_incoming;
};

void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex)
void Init(const Options& connOptions) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !m_total_bytes_sent_mutex)
{
AssertLockNotHeld(m_total_bytes_sent_mutex);

Expand Down Expand Up @@ -891,7 +891,8 @@ friend class CNode;
CConnman(uint64_t seed0, uint64_t seed1, CAddrMan& addrman, bool network_active = true);
~CConnman();
bool Start(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman, CMasternodeSync& mn_sync,
CScheduler& scheduler, const Options& options) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
CScheduler& scheduler, const Options& options)
EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !m_added_nodes_mutex, !m_addr_fetches_mutex, !mutexMsgProc);

void StopThreads();
void StopNodes();
Expand All @@ -901,7 +902,7 @@ friend class CNode;
StopNodes();
};

void Interrupt();
void Interrupt() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
bool GetNetworkActive() const { return fNetworkActive; };
bool GetUseAddrmanOutgoing() const { return m_use_addrman_outgoing; };
void SetNetworkActive(bool active, CMasternodeSync* const mn_sync);
Expand All @@ -917,8 +918,13 @@ friend class CNode;
IsConnection,
};

void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound, const char* strDest, ConnectionType conn_type, MasternodeConn masternode_connection = MasternodeConn::IsNotConnection, MasternodeProbeConn masternode_probe_connection = MasternodeProbeConn::IsNotConnection);
void OpenMasternodeConnection(const CAddress& addrConnect, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection);
void OpenNetworkConnection(const CAddress& addrConnect, bool fCountFailure, CSemaphoreGrant* grantOutbound,
const char* strDest, ConnectionType conn_type,
MasternodeConn masternode_connection = MasternodeConn::IsNotConnection,
MasternodeProbeConn masternode_probe_connection = MasternodeProbeConn::IsNotConnection)
EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
void OpenMasternodeConnection(const CAddress& addrConnect, MasternodeProbeConn probe = MasternodeProbeConn::IsConnection)
EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
bool CheckIncomingNonce(uint64_t nonce);

struct CFullyConnectedOnly {
Expand Down Expand Up @@ -959,7 +965,8 @@ friend class CNode;

bool IsMasternodeOrDisconnectRequested(const CService& addr);

void PushMessage(CNode* pnode, CSerializedNetMsg&& msg) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
void PushMessage(CNode* pnode, CSerializedNetMsg&& msg)
EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc, !m_total_bytes_sent_mutex);

template<typename Condition, typename Callable>
bool ForEachNodeContinueIf(const Condition& cond, Callable&& func)
Expand Down Expand Up @@ -1099,9 +1106,9 @@ friend class CNode;
// Count the number of block-relay-only peers we have over our limit.
int GetExtraBlockRelayCount() const;

bool AddNode(const std::string& node);
bool RemoveAddedNode(const std::string& node);
std::vector<AddedNodeInfo> GetAddedNodeInfo() const;
bool AddNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
bool RemoveAddedNode(const std::string& node) EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);
std::vector<AddedNodeInfo> GetAddedNodeInfo() const EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex);

/**
* Attempts to open a connection. Currently only used from tests.
Expand All @@ -1114,7 +1121,7 @@ friend class CNode;
* - Max total outbound connection capacity filled
* - Max connection capacity for type is filled
*/
bool AddConnection(const std::string& address, ConnectionType conn_type);
bool AddConnection(const std::string& address, ConnectionType conn_type) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);

bool AddPendingMasternode(const uint256& proTxHash);
void SetMasternodeQuorumNodes(Consensus::LLMQType llmqType, const uint256& quorumHash, const std::set<uint256>& proTxHashes);
Expand Down Expand Up @@ -1166,8 +1173,8 @@ friend class CNode;

unsigned int GetReceiveFloodSize() const;

void WakeMessageHandler();
void WakeSelect();
void WakeMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
void WakeSelect() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);

/** Attempts to obfuscate tx time through exponentially distributed emitting.
Works assuming that a single interval is used.
Expand Down Expand Up @@ -1221,13 +1228,15 @@ friend class CNode;
const std::vector<NetWhitebindPermissions>& whiteBinds,
const std::vector<CService>& onion_binds);

void ThreadOpenAddedConnections();
void AddAddrFetch(const std::string& strDest);
void ProcessAddrFetch();
void ThreadOpenConnections(const std::vector<std::string> connect, CDeterministicMNManager& dmnman);
void ThreadMessageHandler();
void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync);
void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync);
void ThreadOpenAddedConnections() EXCLUSIVE_LOCKS_REQUIRED(!m_added_nodes_mutex, !mutexMsgProc);
void AddAddrFetch(const std::string& strDest) EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex);
void ProcessAddrFetch() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !mutexMsgProc);
void ThreadOpenConnections(const std::vector<std::string> connect, CDeterministicMNManager& dmnman)
EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_added_nodes_mutex, !m_nodes_mutex, !mutexMsgProc);
void ThreadMessageHandler() EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
void ThreadI2PAcceptIncoming(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
void AcceptConnection(const ListenSocket& hListenSocket, CMasternodeSync& mn_sync)
EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);

/**
* Create a `CNode` object from a socket that has just been accepted and add the node to
Expand All @@ -1241,7 +1250,7 @@ friend class CNode;
NetPermissionFlags permissionFlags,
const CAddress& addr_bind,
const CAddress& addr,
CMasternodeSync& mn_sync);
CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);

void DisconnectNodes();
void NotifyNumConnectionsChanged(CMasternodeSync& mn_sync);
Expand Down Expand Up @@ -1305,7 +1314,7 @@ friend class CNode;
/**
* Check connected and listening sockets for IO readiness and process them accordingly.
*/
void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
void SocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc);

/**
* Do the read/write for connected sockets that are ready for IO.
Expand All @@ -1316,18 +1325,20 @@ friend class CNode;
void SocketHandlerConnected(const std::set<SOCKET>& recv_set,
const std::set<SOCKET>& send_set,
const std::set<SOCKET>& error_set)
EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc);

/**
* Accept incoming connections, one from each read-ready listening socket.
* @param[in] recv_set Sockets that are ready for read.
*/
void SocketHandlerListening(const std::set<SOCKET>& recv_set, CMasternodeSync& mn_sync);
void SocketHandlerListening(const std::set<SOCKET>& recv_set, CMasternodeSync& mn_sync)
EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);

void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex);
void ThreadDNSAddressSeed();
void ThreadSocketHandler(CMasternodeSync& mn_sync) EXCLUSIVE_LOCKS_REQUIRED(!m_total_bytes_sent_mutex, !mutexMsgProc);
void ThreadDNSAddressSeed() EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex);
void ThreadOpenMasternodeConnections(CDeterministicMNManager& dmnman, CMasternodeMetaMan& mn_metaman,
CMasternodeSync& mn_sync);
CMasternodeSync& mn_sync)
EXCLUSIVE_LOCKS_REQUIRED(!m_addr_fetches_mutex, !m_nodes_mutex, !mutexMsgProc);

uint64_t CalculateKeyedNetGroup(const CAddress& ad) const;

Expand All @@ -1351,7 +1362,7 @@ friend class CNode;
NodeId GetNewNodeId();

size_t SocketSendData(CNode& node) EXCLUSIVE_LOCKS_REQUIRED(node.cs_vSend);
size_t SocketRecvData(CNode* pnode);
size_t SocketRecvData(CNode* pnode) EXCLUSIVE_LOCKS_REQUIRED(!mutexMsgProc);
void DumpAddresses();

// Network stats
Expand Down
Loading

0 comments on commit 6d5ff0a

Please sign in to comment.