CConnman::SocketEvents() may poll the wrong socket #21744
Description
Adverse scenario:
- thread "net"
CConnman::ThreadSocketHandler()
CConnman::SocketHandler()
CConnman::SocketEvents()
CConnman::GenerateSelectSet()
Cycles through CConnman::vNodes and remembers some sockets to
be polled for readiness later by CConnman::SocketEvents().
Lets say socket (file descriptor) 10 is remembered.
- thread "msghand"
CConnman::ThreadMessageHandler()
PeerManagerImpl::ProcessMessages()
PeerManagerImpl::ProcessMessage()
PeerManagerImpl::PushNodeVersion()
CConnman::PushMessage()
CConnman::SocketSendData()
CNode::CloseSocketDisconnect()
Closes socket 10.
- thread "opencon"
CConnman::ThreadOpenConnections()
CConnman::OpenNetworkConnection()
CConnman::ConnectNode()
CreateSock()
Creates a new socket, reusing file descriptor 10.
- thread "net"
CConnman::ThreadSocketHandler()
CConnman::SocketHandler()
CConnman::SocketEvents()
polls the remembered socket 10, which corresponds to a different
connection now (bug)
As a result the following logic may not hold:
Lines 1306 to 1315 in e16f872
I think the severity of this is low and that it has a low chance of happening. However the pattern of "remember a socket by its file descriptor number and use it later, concurrently with other threads that might close it" better be avoided. That pattern could lead to closing or writing to the wrong socket which would be more severe.