views:

46

answers:

1

I have a TCP server that continually monitors for new incoming clients asynchronously and adds them to a client list:

public class TcpServer
{
    public List<TcpClient> ClientsList = new List<TcpClient>();
    protected TcpListener Server = new TcpListener(IPAddress.Any, 3000);
    private _isMonitoring = false;

    public TcpServer()
    {
        Server.Start();
        Server.StartMonitoring();
    }

    public void StartMonitoring()
    {
        _isMonitoring = true;
        Server.BeginAcceptTcpClient(HandleNewClient, null);
    }

    public void StopMonitoring()
    {
        _isMonitoring = false;
    }

    protected void HandleNewClient(IAsyncResult result)
    {
        if (_isMonitoring)
        {
            var client = Server.EndAcceptTcpClient(result);
            ClientsList.Add(client);

            StartMonitoring(); // repeats the monitoring
        }
    }
}

However, I'm having two issues with this code.

The first is the StartMonitoring() call in HandleNewClient(). Without it, the server will accept only one incoming connection and ignore any additional connections. What I'd like to do is have it continually monitor for new clients, but something rubs me wrong about the way I'm doing it now. Is there a better way of doing this?

The second is the _isMonitoring flag. I don't know how else to stop the async callback from activating and stop it from looping. Any advice on how this can be improved? I'd like to stick to using asynchronous callbacks and avoid having to manually create new threads running methods that have while (true) loops in them.

+3  A: 

Basically, your StartMonitoring function, needs to loop - you'll only accept a single client at a time, and then you'd typically pass the request off to a worker thread, and then resume accepting new connections. The way its written, as you've stated, it will only accept a single client.

You'll want to expand on this to suit your startup/shutdown/terminate needs, but basically, what you're looking for is StartMonitoring to be more like this:

public void StartMonitoring()
{
    _isMonitoring = true;
    while (_isMonitoring)
        Server.BeginAcceptTcpClient(HandleNewClient, null);
}

Note that if _isMonitoring is going to be set by another thread, you'd better mark it as volatile, or you'll likely never terminate the loops.

Nathan Ernst
There's a problem with this code. The `while()` loop will tie up the thread so that I can never set `IsMonitoring = false`. Does this mean I have to spawn it in a new thread?
Daniel T.
Yes, you would likely need another thread, or you'd need to poll for incoming connections, instead of blocking on an accept.
Nathan Ernst