rpc: read vNodes under cs_vNodes in getinfo/getnetworkinfo
getinfo and getnetworkinfo read vNodes.size() and counted the TLS peers under cs_main (getinfo also cs_wallet), not cs_vNodes, racing the socket handler that pushes to and erases from vNodes. That is a torn read and iteration over a vector being reallocated: undefined behaviour, in practice a rare crash or a stale, inconsistent count. Add GetNodeCounts(), which takes cs_vNodes once and returns a consistent snapshot (tls_connections <= connections). Both RPCs now read the counts through it before taking cs_main, so the two locks are never held at once (no lock-order change, no extended hold of cs_vNodes) and the "connections"/"tls_connections" output fields are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
+10
@@ -2951,6 +2951,16 @@ void CNode::PushAddrMessage(CSerializedNetMsg&& msg)
|
||||
}
|
||||
}
|
||||
|
||||
void GetNodeCounts(int& nAll, int& nTls)
|
||||
{
|
||||
LOCK(cs_vNodes);
|
||||
nAll = (int)vNodes.size();
|
||||
nTls = 0;
|
||||
for (const CNode* n : vNodes)
|
||||
if (n->ssl != NULL)
|
||||
nTls++;
|
||||
}
|
||||
|
||||
size_t GetNodeCount(NumConnections flags)
|
||||
{
|
||||
LOCK(cs_vNodes);
|
||||
|
||||
@@ -165,6 +165,7 @@ enum NumConnections {
|
||||
};
|
||||
|
||||
size_t GetNodeCount(NumConnections num);
|
||||
void GetNodeCounts(int& nAll, int& nTls);
|
||||
|
||||
bool GetNetworkActive();
|
||||
void SetNetworkActive(bool active);
|
||||
|
||||
+6
-2
@@ -132,6 +132,10 @@ UniValue getinfo(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
+ HelpExampleRpc("getinfo", "")
|
||||
);
|
||||
|
||||
// C268: snapshot peer counts under cs_vNodes (sequentially, before the cs_main/cs_wallet lock)
|
||||
int nAllConns = 0, nTlsConns = 0;
|
||||
GetNodeCounts(nAllConns, nTlsConns);
|
||||
|
||||
#ifdef ENABLE_WALLET
|
||||
LOCK2(cs_main, pwalletMain ? &pwalletMain->cs_wallet : NULL);
|
||||
#else
|
||||
@@ -175,8 +179,8 @@ UniValue getinfo(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
}
|
||||
//obj.push_back(Pair("build_date", BUILD_DATE));
|
||||
obj.push_back(Pair("timeoffset", 0));
|
||||
obj.push_back(Pair("connections", (int)vNodes.size()));
|
||||
obj.push_back(Pair("tls_connections", (int)std::count_if(vNodes.begin(), vNodes.end(), [](CNode* n) {return n->ssl != NULL;})));
|
||||
obj.push_back(Pair("connections", nAllConns));
|
||||
obj.push_back(Pair("tls_connections", nTlsConns));
|
||||
obj.push_back(Pair("proxy", (proxy.IsValid() ? proxy.proxy.ToStringIPPort() : string())));
|
||||
obj.push_back(Pair("testnet", Params().TestnetToBeDeprecatedFieldRPC()));
|
||||
obj.push_back(Pair("relayfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())));
|
||||
|
||||
+5
-2
@@ -533,6 +533,9 @@ UniValue getnetworkinfo(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
+ HelpExampleRpc("getnetworkinfo", "")
|
||||
);
|
||||
|
||||
int nAllConns = 0, nTlsConns = 0;
|
||||
GetNodeCounts(nAllConns, nTlsConns);
|
||||
|
||||
LOCK(cs_main);
|
||||
|
||||
UniValue obj(UniValue::VOBJ);
|
||||
@@ -541,8 +544,8 @@ UniValue getnetworkinfo(const UniValue& params, bool fHelp, const CPubKey& mypk)
|
||||
obj.push_back(Pair("protocolversion",PROTOCOL_VERSION));
|
||||
obj.push_back(Pair("localservices", strprintf("%016x", nLocalServices)));
|
||||
obj.push_back(Pair("timeoffset", 0));
|
||||
obj.push_back(Pair("connections", (int)vNodes.size()));
|
||||
obj.push_back(Pair("tls_connections", (int)std::count_if(vNodes.begin(), vNodes.end(), [](CNode* n) {return n->ssl != NULL;})));
|
||||
obj.push_back(Pair("connections", nAllConns));
|
||||
obj.push_back(Pair("tls_connections", nTlsConns));
|
||||
obj.push_back(Pair("networks", GetNetworksInfo()));
|
||||
obj.push_back(Pair("relayfee", ValueFromAmount(::minRelayTxFee.GetFeePerK())));
|
||||
UniValue localAddresses(UniValue::VARR);
|
||||
|
||||
Reference in New Issue
Block a user