Socket Server Code - async connect failure handling change

| 0 Comments
I've just made a small change to The Server Framework. The change is in how the AsyncConnect() function reports connection errors and the potential problem can occur if there's a race condition between a call to Close() on a socket that has not yet successfully connected. If the call to Close() completes before connection is attempted by the IO thread pool (possible but usually unlikely in normal operation) then the connection failure doesn't currently make its way back to OnOutgoingConnectionFailed() which means that any code you might have in there for dealing with connection failures wont get called...

The fix is in CStreamSocketConnectionManager::ExecuteConnect() and changes this code:

   if (!ConnectEx(
      pSocket->GetSocket(), 
      pAddress,
      addressLength,
      0,
      0,
      0,
      pBuffer))
   {
      const DWORD lastError = ::WSAGetLastError();

      if (ERROR_IO_PENDING != lastError)
      {
         pSocket->OnConnectionError(ConnectError, pBuffer, lastError);
         
         pSocket->Release();           
         pBuffer->Release();
      }
   }

to this:

   if (!ConnectEx(
      pSocket->GetSocket(), 
      pAddress,
      addressLength,
      0,
      0,
      0,
      pBuffer))
   {
      DWORD lastError = ::WSAGetLastError();

      if (ERROR_IO_PENDING != lastError)
      {
         const CAddress address(*pAddress, addressLength);

         const void *pUserData = reinterpret_cast<void*>(*reinterpret_cast<const DWORD *>(pData + sizeof(void*)));

#pragma TODO("could call via the socket and make this const again?")

         if (lastError == WSAENOTSOCK || 
             lastError == WSAEINVAL)
         {
            // if a connect is cancelled by a call to Close() before the connect attempt takes place then 
            // we'll get either WSAENOTSOCK if the socket has been closed but the member variable hasn't been
            // updated to INVALID_SOCKET and WSAEINVAL if the socket has been closed and the member variable
            // HAS been set to INVALID_SOCKET... IMHO ERROR_OPERATION_ABORTED is a better representation of 
            // what actually happened...

            lastError = ERROR_OPERATION_ABORTED;
         }

         OnOutgoingConnectionFailed(pSocket, address, pUserData, lastError);

         pSocket->Release();           
         pBuffer->Release();
      }
   }

This requires that CStreamSocketConnectionManager::ExecuteConnect() is made non const in the class and the interface...

Leave a comment