Merge bitcoin/bitcoin#23604: Use Sock in CNode

ef5014d256 style: wrap long lines in CNode creation and add some comments (Vasil Dimov)
b683491648 scripted-diff: rename CNode::cs_hSocket to CNode::m_sock_mutex (Vasil Dimov)
c41a1162ac net: use Sock in CNode (Vasil Dimov)
c5dd72e146 fuzz: move FuzzedSock earlier in src/test/fuzz/util.h (Vasil Dimov)

Pull request description:

  _This is a piece of #21878, chopped off to ease review._

  Change `CNode` to use a pointer to `Sock` instead of a bare `SOCKET`.
  This will help mocking / testing / fuzzing more code.

ACKs for top commit:
  jonatack:
    re-ACK ef5014d256 changes since last review are the removal of an unneeded dtor and the addition of a style commit
  w0xlt:
    reACK ef5014d
  PastaPastaPasta:
    utACK ef5014d256, I have reviewed the code, and believe it makes sense to merge
  theStack:
    Cod-review ACK ef5014d256

Tree-SHA512: 7f5414dd339cd2f16f7cbdc5fcec238d68b6d50072934aea10b901f409da28ff1ece6db6e899196616aa8127b8b25ab5b86d000bdcee58b4cadd7a3c1cf560c5
This commit is contained in:
laanwj
2022-02-04 09:19:20 +01:00
5 changed files with 230 additions and 115 deletions

View File

@ -506,7 +506,16 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
if (!addr_bind.IsValid()) {
addr_bind = GetBindAddress(sock->Get());
}
CNode* pnode = new CNode(id, nLocalServices, sock->Release(), addrConnect, CalculateKeyedNetGroup(addrConnect), nonce, addr_bind, pszDest ? pszDest : "", conn_type, /* inbound_onion */ false);
CNode* pnode = new CNode(id,
nLocalServices,
std::move(sock),
addrConnect,
CalculateKeyedNetGroup(addrConnect),
nonce,
addr_bind,
pszDest ? pszDest : "",
conn_type,
/*inbound_onion=*/false);
pnode->AddRef();
// We're making a new connection, harvest entropy from the time (and our peer count)
@ -518,11 +527,10 @@ CNode* CConnman::ConnectNode(CAddress addrConnect, const char *pszDest, bool fCo
void CNode::CloseSocketDisconnect()
{
fDisconnect = true;
LOCK(cs_hSocket);
if (hSocket != INVALID_SOCKET)
{
LOCK(m_sock_mutex);
if (m_sock) {
LogPrint(BCLog::NET, "disconnecting peer=%d\n", id);
CloseSocket(hSocket);
m_sock.reset();
}
}
@ -802,10 +810,11 @@ size_t CConnman::SocketSendData(CNode& node) const
assert(data.size() > node.nSendOffset);
int nBytes = 0;
{
LOCK(node.cs_hSocket);
if (node.hSocket == INVALID_SOCKET)
LOCK(node.m_sock_mutex);
if (!node.m_sock) {
break;
nBytes = send(node.hSocket, reinterpret_cast<const char*>(data.data()) + node.nSendOffset, data.size() - node.nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT);
}
nBytes = node.m_sock->Send(reinterpret_cast<const char*>(data.data()) + node.nSendOffset, data.size() - node.nSendOffset, MSG_NOSIGNAL | MSG_DONTWAIT);
}
if (nBytes > 0) {
node.m_last_send = GetTime<std::chrono::seconds>();
@ -1200,7 +1209,16 @@ void CConnman::CreateNodeFromAcceptedSocket(std::unique_ptr<Sock>&& sock,
}
const bool inbound_onion = std::find(m_onion_binds.begin(), m_onion_binds.end(), addr_bind) != m_onion_binds.end();
CNode* pnode = new CNode(id, nodeServices, sock->Release(), addr, CalculateKeyedNetGroup(addr), nonce, addr_bind, "", ConnectionType::INBOUND, inbound_onion);
CNode* pnode = new CNode(id,
nodeServices,
std::move(sock),
addr,
CalculateKeyedNetGroup(addr),
nonce,
addr_bind,
/*addrNameIn=*/"",
ConnectionType::INBOUND,
inbound_onion);
pnode->AddRef();
pnode->m_permissionFlags = permissionFlags;
pnode->m_prefer_evict = discouraged;
@ -1384,17 +1402,18 @@ bool CConnman::GenerateSelectSet(const std::vector<CNode*>& nodes,
select_send = !pnode->vSendMsg.empty();
}
LOCK(pnode->cs_hSocket);
if (pnode->hSocket == INVALID_SOCKET)
LOCK(pnode->m_sock_mutex);
if (!pnode->m_sock) {
continue;
}
error_set.insert(pnode->hSocket);
error_set.insert(pnode->m_sock->Get());
if (select_send) {
send_set.insert(pnode->hSocket);
send_set.insert(pnode->m_sock->Get());
continue;
}
if (select_recv) {
recv_set.insert(pnode->hSocket);
recv_set.insert(pnode->m_sock->Get());
}
}
@ -1564,12 +1583,13 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
bool sendSet = false;
bool errorSet = false;
{
LOCK(pnode->cs_hSocket);
if (pnode->hSocket == INVALID_SOCKET)
LOCK(pnode->m_sock_mutex);
if (!pnode->m_sock) {
continue;
recvSet = recv_set.count(pnode->hSocket) > 0;
sendSet = send_set.count(pnode->hSocket) > 0;
errorSet = error_set.count(pnode->hSocket) > 0;
}
recvSet = recv_set.count(pnode->m_sock->Get()) > 0;
sendSet = send_set.count(pnode->m_sock->Get()) > 0;
errorSet = error_set.count(pnode->m_sock->Get()) > 0;
}
if (recvSet || errorSet)
{
@ -1577,10 +1597,11 @@ void CConnman::SocketHandlerConnected(const std::vector<CNode*>& nodes,
uint8_t pchBuf[0x10000];
int nBytes = 0;
{
LOCK(pnode->cs_hSocket);
if (pnode->hSocket == INVALID_SOCKET)
LOCK(pnode->m_sock_mutex);
if (!pnode->m_sock) {
continue;
nBytes = recv(pnode->hSocket, (char*)pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
}
nBytes = pnode->m_sock->Recv(pchBuf, sizeof(pchBuf), MSG_DONTWAIT);
}
if (nBytes > 0)
{
@ -2965,8 +2986,9 @@ ServiceFlags CConnman::GetLocalServices() const
unsigned int CConnman::GetReceiveFloodSize() const { return nReceiveFloodSize; }
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion)
: m_connected{GetTime<std::chrono::seconds>()},
CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, std::shared_ptr<Sock> sock, const CAddress& addrIn, uint64_t nKeyedNetGroupIn, uint64_t nLocalHostNonceIn, const CAddress& addrBindIn, const std::string& addrNameIn, ConnectionType conn_type_in, bool inbound_onion)
: m_sock{sock},
m_connected{GetTime<std::chrono::seconds>()},
addr(addrIn),
addrBind(addrBindIn),
m_addr_name{addrNameIn.empty() ? addr.ToStringIPPort() : addrNameIn},
@ -2978,7 +3000,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
nLocalServices(nLocalServicesIn)
{
if (inbound_onion) assert(conn_type_in == ConnectionType::INBOUND);
hSocket = hSocketIn;
if (conn_type_in != ConnectionType::BLOCK_RELAY) {
m_tx_relay = std::make_unique<TxRelay>();
}
@ -2997,11 +3018,6 @@ CNode::CNode(NodeId idIn, ServiceFlags nLocalServicesIn, SOCKET hSocketIn, const
m_serializer = std::make_unique<V1TransportSerializer>(V1TransportSerializer());
}
CNode::~CNode()
{
CloseSocket(hSocket);
}
bool CConnman::NodeFullyConnected(const CNode* pnode)
{
return pnode && pnode->fSuccessfullyConnected && !pnode->fDisconnect;