C# tcp async listener gets stuck on my on_receive callback after client closes socket

Question

I've got a listener socket that accepts, receives and sends as a TCP server typically does. I've given my accept and receive code below, it's not that different from the example on Microsoft's documentation. The main difference is that my server doesn't kill a connection after it stops receiving data (I don't know if this is a bad design or not?).

private void on_accept(IAsyncResult xResult)
{
    Socket listener = null;
    Socket handler = null;
    TStateObject state = null;
    Task<int> consumer = null;

    try
    {
        mxResetEvent.Set();
        listener = (Socket)xResult.AsyncState;
        handler = listener.EndAccept(xResult);
        state = new TStateObject()
        {
            Socket = handler
        };
        consumer = async_input_consumer(state);
        OnConnect?.Invoke(this, handler);
        handler.BeginReceive(state.Buffer, 0, TStateObject.BufferSize, 0, new AsyncCallback(on_receive), state);
    }
    catch (SocketException se)
    {
        if (se.ErrorCode == 10054)
        {
            on_disconnect(state);
        }
    }
    catch (ObjectDisposedException)
    {
        return;
    }
    catch (Exception ex)
    {
        System.Console.WriteLine("Exception in TCPServer::AcceptCallback, exception: " + ex.Message);
    }
}

private void on_receive(IAsyncResult xResult)
{
    Socket handler = null;
    TStateObject state = null;
    try
    {
        state = xResult.AsyncState as TStateObject;
        handler = state.Socket;
        int bytesRead = handler.EndReceive(xResult);
        UInt16 id = TClientRegistry.GetIdBySocket(handler);
        TContext context = TClientRegistry.GetContext(id);

        if (bytesRead > 0)
        {
            var buffer_data = new byte[bytesRead];
            Array.Copy(state.Buffer, buffer_data, bytesRead);
            state.BufferBlock.Post(buffer_data);
        }
        Array.Clear(state.Buffer, 0, state.Buffer.Length);
        handler.BeginReceive(state.Buffer, 0, TStateObject.BufferSize, 0, new AsyncCallback(on_receive), state);
    }
    catch (SocketException se)
    {
        if(se.ErrorCode == 10054)
        {
            on_disconnect(state);
        }
    }
    catch (ObjectDisposedException)
    {
        return;
    }
    catch (Exception ex)
    {
        System.Console.WriteLine("Exception in TCPServer::ReadCallback, exception: " + ex.Message);
    }
}

This code is used to connect to an embedded device and works (mostly) fine. I was investigating a memory leak and trying to speed up the process a bit by replicating exactly what the device does (our connection speeds are in the realm of about 70kbps to our device, and it took an entire weekend of stress testing to get the memory leak to double the memory footprint of the server).

So I wrote a C# program to replicate the data transactions, but I've run into an issue where when I disconnect the test program, the server gets caught in a loop where it endlessly has its on_receive callback called. I was under the impression that BeginReceive wouldn't be triggered until something was received, and it seems to call on_receive, ends the receiving like an async callback should do, process the data, and then I want the connection to await more data so I call BeginReceive again.

The part of my test program where the issue occurs is in here:

private static void read_write_test()
{
    mxConnection = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
    mxConnection.Connect("12.12.12.18", 10);
    if (mxConnection.Connected)
    {
        byte[] data = Encoding.ASCII.GetBytes("HANDSHAKESTRING"); //Connect string
        int len = data.Length;
        mxConnection.Send(data);
        data = new byte[4];
        len = mxConnection.Receive(data);

        if (len == 0 || data[0] != '1')
        {
            mxConnection.Disconnect(false);
            return;
        }
    }

    //Meat of the test goes here but isn't relevant

    mxConnection.Shutdown(SocketShutdown.Both);
    mxConnection.Close();
}

Up until the Shutdown(SocketShutdown.Both) call, everything works as expected. When I make that call however, it seems like the server never gets notification that the client has closed the socket and gets stuck in a loop of endlessly trying to receive. I've done my homework and I think I am closing my connection properly as per this discussion. I've messed around with the disconnect section to just do mxConnection.Disconnect(false) as well, but the same thing occurs.

When the device disconnects from the server, my server catches a SocketException with error code 10054, which documentation says:

Connection reset by peer.

An existing connection was forcibly closed by the remote host. This normally results if the peer application on the remote host is suddenly stopped, the host is rebooted, the host or remote network interface is disabled, or the remote host uses a hard close (see setsockopt for more information on the SO_LINGER option on the remote socket). This error may also result if a connection was broken due to keep-alive activity detecting a failure while one or more operations are in progress. Operations that were in progress fail with WSAENETRESET. Subsequent operations fail with WSAECONNRESET.

I've used this to handle the socket being closed and has worked well for the most part. However, with my C# test program, it doesn't seem like it works the same way.

Am I missing something here? I'd appreciate any input. Thanks.

Answer 1

The main difference is that my server doesn't kill a connection after it stops receiving data (I don't know if this is a bad design or not?).

Of course it is.

it seems like the server never gets notification that the client has closed the socket and gets stuck in a loop of endlessly trying to receive

The server does get notification. It's just that you ignore it. The notification is that your receive operation returns 0. When that happens, you just call BeginReceive() again. Which starts a new read operation. Which…returns 0! You just keep doing that over and over again.

When a receive operation returns 0, you're supposed to complete the graceful closure (with a call to Shutdown() and Close()) that the remote endpoint started. Do not try to receive again. You'll just keep getting the same result.

I strongly recommend you do more homework. A good place to start would be the Winsock Programmer's FAQ. It is a fairly old resource and doesn't address .NET at all. But for the most part, the things that novice network programmers are getting wrong in .NET are the same things that novice Winsock programmers were getting wrong twenty years ago. The document is still just as relevant today as it was then.

By the way, your client-side code has some issues as well. First, when the Connect() method returns successfully, the socket is connected. You don't have to check the Connected property (and in fact, should never have to check that property). Second, the Disconnect() method doesn't do anything useful. It's used when you want to re-use the underlying socket handle, but you should be disposing the Socket object here. Just use Shutdown() and Close(), per the usual socket API idioms. Third, any code that receives from a TCP socket must do that in a loop, and make use of the received byte-count value to determine what data has been read and whether enough has been read to do anything useful. TCP can return any positive number of bytes on a successful read, and it's your program's job to identify the start and end of any particular blocks of data that were sent.

Answer 2

You missed this in the documentation for EndReceive() and Receive():

If the remote host shuts down the Socket connection with the Shutdown method, and all available data has been received, the Receive method will complete immediately and return zero bytes.

When you read zero bytes, you still start another BeginReceive(), instead of shutting down:

    if (bytesRead > 0)
    {
        var buffer_data = new byte[bytesRead];
        Array.Copy(state.Buffer, buffer_data, bytesRead);
        state.BufferBlock.Post(buffer_data);
    }
    Array.Clear(state.Buffer, 0, state.Buffer.Length);
    handler.BeginReceive(state.Buffer, 0, TStateObject.BufferSize, 0, new AsyncCallback(on_receive), state);

Since you keep calling BeginReceive on a socket that's 'shutdown', you're going to keep getting callbacks to receive zero bytes.

Compare with the example from Microsoft in the documentation for EndReceive():

public static void Read_Callback(IAsyncResult ar){
    StateObject so = (StateObject) ar.AsyncState;
    Socket s = so.workSocket;

    int read = s.EndReceive(ar);

    if (read > 0) {
            so.sb.Append(Encoding.ASCII.GetString(so.buffer, 0, read));
            s.BeginReceive(so.buffer, 0, StateObject.BUFFER_SIZE, 0, 
                                     new AsyncCallback(Async_Send_Receive.Read_Callback), so);
    }
    else{
         if (so.sb.Length > 1) {
              //All of the data has been read, so displays it to the console
              string strContent;
              strContent = so.sb.ToString();
              Console.WriteLine(String.Format("Read {0} byte from socket" + 
                               "data = {1} ", strContent.Length, strContent));
         }
         s.Close();
    }
}