C# socket BeginAccept event stops firing

Question

The code below creates socket server and client

I start the server and if I start one after the other the clients it works fine

If I start three clients immediately, then for one or more clients the BeginAccept event is not fired

The results bellow are after executing the code bellow

Server Started

Server is waiting for a connection...

Client 0.0.0.0:6352 requests connection

Client 0.0.0.0:6353 requests connection

Client 127.0.0.1:6351 requests connection

Client 127.0.0.1:6351 connected

Client 127.0.0.1:6352 connected

Client 127.0.0.1:6353 connected

ServerOnClientConnection Client: 127.0.0.1:6351

Server is waiting for a connection...

ServerOnClientConnection Client: 127.0.0.1:6353

code follows

    using System;
    using System.Collections.Generic;
    using System.Linq;
    using System.Net;
    using System.Net.Sockets;
    using System.Text;
    using System.Threading;
    using System.Windows.Forms;

    namespace Test {
public class TestSockets {

    #region server
    Socket serverSocket;
    bool serverIsAlive;
    public ManualResetEvent waitForConnection = new ManualResetEvent(false);
    private Encoding encod = Encoding.Unicode;

    public void ServerStartInThread() {
        byte[] bytes = new Byte[1024];
        IPAddress ipAddress = IPAddress.Parse("127.0.0.1");
        IPEndPoint localEndPoint = new IPEndPoint(ipAddress, 5500);
        Socket socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
        serverSocket = socket;
        try {
            socket.Bind(localEndPoint);
            socket.Listen(100);
            Thread pollThread = new Thread(delegate () {
                serverIsAlive = true;    // needs if reopen
                SendMessage("Server Started");
                while (serverIsAlive) {
                    try {
                        SendMessage("Server is waiting for a connection...");
                        socket.BeginAccept(new AsyncCallback(ServerOnClientConnection), socket);
                        waitForConnection.Reset();
                        waitForConnection.WaitOne();
                    }
                    catch (Exception ex) {
                        SendMessage("Server: " + ex.ToString());
                    }
                }
                SendMessage("Server Stopped");
                socket.Close();
            }) {
                Name = "SocketServer"
            };
            pollThread.Start();
        }
        catch (Exception ex) {
            SendMessage("Server: " + ex.ToString());
        }
    }

    public void ServerOnClientConnection(IAsyncResult ar) {
        try {
            Socket listener = (Socket)ar.AsyncState;
            Socket clientSocket = listener.EndAccept(ar);
            SendMessage("ServerOnClientConnection Client: " + clientSocket.RemoteEndPoint.ToString());
            StateObject state = new StateObject() {
                socket = clientSocket
            };
            clientSocket.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ServerReceiveCallback), state);
            waitForConnection.Set();
        }
        catch (Exception ex) {
            SendMessage("ServerOnClientConnection: " + ex.ToString());
        }
    }

    public void ServerReceiveCallback(IAsyncResult ar) {
        StateObject state = (StateObject)ar.AsyncState;
        Socket socket = state.socket;
        try {
            if (socket == null) return;
            if (!socket.Connected) {
                return;
            }
            int bytesRead = socket.EndReceive(ar);
            if (bytesRead > 0) {
                state.sb.Append(encod.GetString(state.buffer, 0, bytesRead));
                socket.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ServerReceiveCallback), state);
            }
        }
        catch (Exception ex) {
            SendMessage("ServerReceiveCallback: " + ex.ToString());
        }
    }
    #endregion

    #region client
    private Socket client;
    private bool isAlive = false;
    private ManualResetEvent connectDone = new ManualResetEvent(false);

    public void StartInThread() {
        try {
            IPAddress ipAddress = IPAddress.Parse("127.0.0.1");
            IPEndPoint remoteEP = new IPEndPoint(ipAddress, 5500);
            Thread pollThread = new Thread(delegate () {
                isAlive = true;
                while (isAlive) {
                    try {
                        if (client != null && client.Connected) {
                            continue;
                        }
                        client = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
                        client.BeginConnect(remoteEP, new AsyncCallback(ClientConnectCallback), client);
                        SendMessage(string.Format("Client {0} requests connection", client.LocalEndPoint.ToString()));
                        connectDone.Reset();
                        connectDone.WaitOne(3000, false);
                        if (client.Connected) {
                            StateObject state = new StateObject() {
                                socket = client
                            };
                            SendMessage(string.Format("Client {0} connected", client.LocalEndPoint.ToString()));
                            client.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ClientReceiveCallback), state);
                        }
                    }
                    catch (Exception ex) {
                        SendMessage("ClientStartInThread1: " + ex.ToString());
                    }
                }
                SendMessage("Client Disconnected");
            }) {
                Name = "ClientThread"
            };
            pollThread.Start();
        }
        catch (Exception ex) {
            SendMessage("ClientStartInThread2: " + ex.ToString());
        }
    }

    private void ClientConnectCallback(IAsyncResult ar) {
        try {
            Socket socket = (Socket)ar.AsyncState;
            socket.EndConnect(ar);
            connectDone.Set();
        }
        catch (Exception ex) {
            SendMessage("ClientConnectCallback: " + ex.ToString());
        }
    }

    private void ClientReceiveCallback(IAsyncResult ar) {
        StateObject state = (StateObject)ar.AsyncState;
        Socket socket = state.socket;
        if (socket == null || !socket.Connected) return;
        try {
            int bytesRead = socket.EndReceive(ar);
            if (bytesRead > 0) {
                state.sb.Append(encod.GetString(state.buffer, 0, bytesRead));
                socket.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ClientReceiveCallback), state);
            }
            else {
                socket.Close();
            }
        }
        catch (Exception ex) {
            SendMessage("ClientReceiveCallback: " + ex.ToString());
            socket.Close();
        }
    }

    #endregion

    private void SendMessage(string v) {
        System.Diagnostics.Debug.WriteLine(v);
    }

    public static void Start() {
        TestSockets server = new TestSockets();
        server.ServerStartInThread();
        TestSockets c1 = new TestSockets();
        c1.StartInThread();
        TestSockets c2 = new TestSockets();
        c2.StartInThread();
        TestSockets c3 = new TestSockets();
        c3.StartInThread();

    }
}
public class StateObject {
    public Socket socket = null;
    public const int BufferSize = 1024;
    public byte[] buffer = new byte[BufferSize];
    public StringBuilder sb = new StringBuilder();
}

}

Thank you, by the way, for being one of the few people to ever post a [tag:sockets] question to Stack Overflow that actually includes a good [mcve] that reliably reproduces the problem. (Well, I did have to add the actual call to `TestSockets.Start()`...but that's a minor nit, compared to the awful network/socket questions we usually get here.) — Peter Duniho, Aug 12 '17 at 17:04

score 0 · Accepted Answer · answered Aug 12 '17 at 17:03

You have a race condition in your code that results in the waitForConnection event handle being set twice, with the second call to Set() having no effect, before the main server thread has a chance to proceed past the BeginAccept() and reset the handle. This causes it to miss the second set.

One way to fix it would be to switch to a semaphore object. For example:

#region server
Socket serverSocket;
bool serverIsAlive;
SemaphoreSlim waitForConnection = new SemaphoreSlim(0);
private Encoding encod = Encoding.Unicode;

public void ServerStartInThread()
{
    byte[] bytes = new Byte[1024];
    IPAddress ipAddress = IPAddress.Parse("127.0.0.1");
    IPEndPoint localEndPoint = new IPEndPoint(ipAddress, 5500);
    Socket socket = new Socket(AddressFamily.InterNetwork, SocketType.Stream, ProtocolType.Tcp);
    serverSocket = socket;
    try
    {
        socket.Bind(localEndPoint);
        socket.Listen(100);
        Thread pollThread = new Thread(delegate () {
            serverIsAlive = true;    // needs if reopen
            SendMessage("Server Started");
            while (serverIsAlive)
            {
                try
                {
                    SendMessage("Server is waiting for a connection...");
                    socket.BeginAccept(new AsyncCallback(ServerOnClientConnection), socket);
                    waitForConnection.Wait();
                }
                catch (Exception ex)
                {
                    SendMessage("Server: " + ex.ToString());
                }
            }
            SendMessage("Server Stopped");
            socket.Close();
        })
        {
            Name = "SocketServer"
        };
        pollThread.Start();
    }
    catch (Exception ex)
    {
        SendMessage("Server: " + ex.ToString());
    }
}

public void ServerOnClientConnection(IAsyncResult ar)
{
    try
    {
        Socket listener = (Socket)ar.AsyncState;
        Socket clientSocket = listener.EndAccept(ar);
        SendMessage("ServerOnClientConnection Client: " + clientSocket.RemoteEndPoint.ToString());
        StateObject state = new StateObject()
        {
            socket = clientSocket
        };
        clientSocket.BeginReceive(state.buffer, 0, StateObject.BufferSize, 0, new AsyncCallback(ServerReceiveCallback), state);
        waitForConnection.Release();
    }
    catch (Exception ex)
    {
        SendMessage("ServerOnClientConnection: " + ex.ToString());
    }
}

This will allow the ServerOnClientConnection() method to increment the semaphore count, so that the main server thread can continue to loop until all of the accepted connections have been observed.

However, frankly, the MSDN examples that use these event handles (which is what I'm assuming your code is based on, either directly or indirectly) are IMHO simply broken. They introduce thread synchronization complexities where none is actually needed, unnecessarily complicating the code and making it harder to get the code to work correctly.

A more idiomatic approach would be to not have the extra thread at all, call BeginAccept() once in the ServerStartInThread() method (which you'd probably rename to just ServerStart()) and call BeginAccept() in the ServerOnClientConnection() method after handling the currently accepted client. I.e. just like you handle the BeginReceive() now.

That said, IMHO even the idiomatic approach is outdated. It worked very well when the API was first designed, but .NET has come a long way since then and has much better mechanisms for dealing with asynchronous operations like this. I posted in this answer an example of a simple chat server that shows how you can use async/await to implement this sort of thing in a much simpler, easier-to-read way. You may want to look at that for ideas on how to apply the same techniques to your own code.

Thank you the SemaphoreSlim solution was very straightforward but I preferred the more idiomatic approcach with BeginAccept() to the ServerOnClientConnection. — user3624787, Aug 14 '17 at 04:59

C# socket BeginAccept event stops firing

1 Answers1