Reputation: 37
I have a listen function that checks if a new connection is waiting on the server socket, if I start the client first and then start the server I am seeing the 'new connection' multiple times, where if I start the server and then the client I see it once.
Am I missing some knowledge of the Posix standard or just lacking knowledge (most likely)?
The function to poll for a new connection is:
bool IPV4Socket::HasNewConnection( TimeoutValue *timeout )
{
SocketSet read_fd_set;
bool hasConnection = false;
SocketSet_ZERO( &read_fd_set );
SocketSet_SET( m_socket, &read_fd_set );
if ( m_socketAdaptor->select( m_socket + 1
, &read_fd_set
, NULL
, NULL
, timeout ) < 0 )
{
throw( std::string( "::select() failed, unable to continue." ) );
}
for ( unsigned int i = 0; i <= m_socket; i++ )
{
if ( m_socketAdaptor->SocketSet_ISSET( i, &read_fd_set ) &&
( i == m_socket ) )
{
hasConnection = true;
}
}
return hasConnection;
}
It is called using:
while( 1 )
{
if ( socket->HasNewConnection( &newConnTimeout ) )
{
std::cout << "[INFO] A new connection is waiting...." << std::endl;
acceptedSocket = socket->Accept( &newConnTimeout );
if ( acceptedSocket )
{
std::cout << "[INFO] New connection received...." << std::endl;
}
}
}
The connect functionality is:
while( connStatus == false )
{
socket = socketlayer->CreateSocket( socketlayer::SockType_stream );
socket->SetNonBlocking( true );
socket->SetSocketOption( socketlayer::SocketOption_KeepAlive, true );
socket->SetSocketOption( socketlayer::SocketOption_ReuseAddr, true );
connStatus = socket->Connect( "127.0.0.1", 18000, &tv );
if ( connStatus == false ) socket->Close();
}
As this is an open source piece of code, which can be found in Sourceforge
Connect is done via:
bool IPV4Socket::Connect( std::string hostname
, unsigned short remotePort
, TimeoutValue *timeout )
{
AddrInfo getResults;
AddrInfo getaddrinfoHints;
int connReturn = 0;
SockAddr_In *addrData;
//bool connectSuccess = false;
std::string service = std::to_string( remotePort );
getaddrinfoHints.ai_family = AddressFamily_inet;
getaddrinfoHints.ai_socktype = SockType_stream;
if ( m_socketAdaptor->getaddrinfo( hostname
, service
, &getaddrinfoHints
, &getResults ) != 0 )
{
return false;
}
addrData = (SockAddr_In *)&( *getResults.ai_addr.begin() );
connReturn = m_socketAdaptor->connect( m_socket
, (const Sockaddr *)addrData
, (int)getResults.ai_addrlen );
if ( connReturn == SocketError)
{
int m_lastErrorCode = m_socketAdaptor->GetLastError();
// Connection error : FATAL
if ( ( m_lastErrorCode != SockErr_EWOULDBLOCK) &&
( m_lastErrorCode != SockErr_EALREADY ) &&
( m_lastErrorCode != SockErr_EINPROGRESS ) )
{
return false;
}
}
SocketSet writeFDS;
//SocketSet exceptFDS;
int selectReturn = 0;
// Clear all the socket FDS structures
SocketSet_ZERO( &writeFDS );
//SocketSet_ZERO( &exceptFDS );
// Put the socket into the FDS structures
SocketSet_SET( m_socket, &writeFDS );
//SocketSet_SET( m_socket, &exceptFDS );
selectReturn = m_socketAdaptor->select( m_socket + 1
, NULL
, &writeFDS
, NULL
, timeout );
// Check for Socket Error or timeout
if ( ( selectReturn == SocketError ) || ( selectReturn == 0 ) )
{
return false;
}
return true;
}
Upvotes: 0
Views: 2051
Reputation: 595402
HasNewConnection()
is ignoring when select()
times out, and its loop is completely redundant since it is waiting on only one socket at a time. The code can be simplified to the following:
bool IPV4Socket::HasNewConnection( TimeoutValue *timeout )
{
SocketSet read_fd_set;
SocketSet_ZERO( &read_fd_set );
SocketSet_SET( m_socket, &read_fd_set );
int ret = m_socketAdaptor->select( m_socket + 1
, &read_fd_set
, NULL
, NULL
, timeout );
if (ret < 0)
{
throw( std::string( "::select() failed, unable to continue." ) );
}
return (ret > 0);
}
With that said, I just looked at the code for Accept()
(please don't ask people to look at code an external site, please always put it in your actual question, where it belongs) and I see one big mistake in it. You are using the same SocketSet
variable for two different parameters of select()
, so the content of that variable will be undefined when select()
exits. If you want to check the read/write
and except
parameters together, you need to use separate variables (like you do in Connect()
):
iSocket *IPV4Socket::Accept( TimeoutValue *timeout )
{
SocketSet readFDS;
SocketSet exceptFDS;
Socket newSocketHandle = 0;
// Clear all the socket FDS structures
SocketSet_ZERO( &readFDS );
SocketSet_ZERO( &exceptFDS );
// Add listening socket to the FDS structures
SocketSet_SET( m_socket, &readFDS );
SocketSet_SET( m_socket, &exceptFDS );
if ( m_socketAdaptor->select( m_socket + 1, &readFDS, NULL, &exceptFDS, timeout ) > 0 )
{
if ( !m_socketAdaptor->SocketSet_ISSET( m_socket, &exceptFDS ) )
{
newSocketHandle = m_socketAdaptor->accept( m_socket, NULL, NULL );
if ( newSocketHandle != Invalid_Socket )
{
return new IPV4Socket( newSocketHandle, m_socketType, m_socketAdaptor );
}
}
}
return NULL;
}
However, this is fairly redundant for accept()
, you don't need to use the except
parameter at all (it makes sense for connect()
but not accept()
):
iSocket *IPV4Socket::Accept( TimeoutValue *timeout )
{
SocketSet readFDS;
Socket newSocketHandle = 0;
// Clear the socket FDS structure
SocketSet_ZERO( &readFDS );
// Add listening socket to the FDS structure
SocketSet_SET( m_socket, &readFDS );
if ( m_socketAdaptor->select( m_socket + 1, &readFDS, NULL, NULL, timeout ) > 0 )
{
newSocketHandle = m_socketAdaptor->accept( m_socket, NULL, NULL );
if ( newSocketHandle != Invalid_Socket )
{
return new IPV4Socket( newSocketHandle, m_socketType, m_socketAdaptor );
}
}
return NULL;
}
Upvotes: 1
Reputation: 9093
The socket is set up to be non-blocking, and a call to connect will typically return an error because the connection status can't be determined yet:
With a nonblocking socket, the connection attempt cannot be completed immediately. In this case, connect will return SOCKET_ERROR, and WSAGetLastError will return WSAEWOULDBLOCK. (...)
- Use the select function to determine the completion of the connection request by checking to see if the socket is writeable.
Given your client code and the above documentation, it should never be able to successfully connect. However, since you connect using the loopback device, in the case where your server is already running it can (and does) happen that the connection is immediately accepted due to timing.
So either use select
on the client after the connect
call as outlined above or simply use a blocking socket.
Upvotes: 1