Is this tcp listener code implemented correctly regarding ResetEvents and Threading?

Question

I have this code that I don't feel is working as well as it should. I've pieced it together and it works, but I don't fully understand Async threads and the ResetEvents.

When I test this listener with a multi-threaded client sending 100 connections in the response time goes from 300 milliseconds for the first 10 or so up to 2-3 seconds for the last 50.

1.) Is the threading and the resetevent handlers implemented as they were designed to do?

2.) Is there some way to speed up the response time of high traffic bursts?

3.) Would it make sense to run the tcpConnectionLogic on another thread?

I have a manual and auto reset event globally scoped:

   private AutoResetEvent connectionWaitHandle = new AutoResetEvent(false);
   private ManualResetEvent _shutdownEvent = new ManualResetEvent(false);

I have a windows service which starts a tcp listener on a new thread

protected override void OnStart(string[] args)
{
    _mainThread = new Thread(ThreadListener_Begin);

    _mainThread.Name = "EMServiceThread";
    _mainThread.IsBackground = false;
    _mainThread.Start();
}

My thread runs a loop waiting for the manual resent event to signal a shutdown.

private void ThreadListener_Begin()
{
    TcpListener listener = null;
    listener = new TcpListener(IPAddress.Parse("172.16.30.248"), 10010);

    listener.ExclusiveAddressUse = false;
    listener.Start();

        while (!_shutdownEvent.WaitOne(0))
        {
            IAsyncResult result = listener.BeginAcceptTcpClient(HandleAsyncConnection, listener);
            connectionWaitHandle.WaitOne();
        }
}

Finally when a connection comes in I accept it and pass it off to a type of logic layer which processes the requests.

private void HandleAsyncConnection(IAsyncResult result)
{ 
  TcpListener listener = (TcpListener)result.AsyncState;

  connectionWaitHandle.Set();

  TcpClient c = listener.EndAcceptTcpClient(result);

  var _tcpConnectedLogic = new TcpConnectionLogic(c);

  _tcpConnectedLogic.BadRequestAlert += _serviceLogic_BadRequestAlert;
  _tcpConnectedLogic.RequestDecodedAlert += _tcpConnectedLogic_RequestDecodedAlert;
  _tcpConnectedLogic.Start();

    }

EDIT When I stop my service my thread does not close as I expect it to. Rather it end's up aborting. Why? How can I make it close gracefully?

protected override void OnStop()
{
    WriteToEventLog("Stop Command Received" + CurrentTimeStamp, entryType: EventLogEntryType.Information);
    WriteToLogFile("Stop Command Received @ " + CurrentTimeStamp);
    _shutdownEvent.Set();

    if (!_mainThread.Join(4000))
    {
        WriteToEventLog("OnStop: Aborting thread, Join did not work.  This is not preferred.", entryType: EventLogEntryType.Warning);
        _mainThread.Abort();
    }
}

Answer 1

In this block

while (!_shutdownEvent.WaitOne(0))
{
    IAsyncResult result = listener.BeginAcceptTcpClient(HandleAsyncConnection, listener);
    connectionWaitHandle.WaitOne();
}

You are basically accepting clients in a loop, because you are blocking thread by waiting on connectionWaitHandle. You could as well forget about async stuff and just do:

while (!_shutdownEvent.WaitOne(0))
{                
     var client = listener.AcceptTcpClient();
     // start your logic in separate thread                
}

I don't say you should do it like this, just note that your current "async" implementation does not add any benefits over synchronous one.

Then, because of the same blocking on connectionWaitHandle, you have your issue with aborting the thread. Suppose for example you just start your listener and no client is ever connected. Then client is blocked on connectionWaitHandle and the fact you set _shutdownEvent does not help - main thread will never have a chance to check it.

Your best option to fix it is to use Task based approach:

class Listener {
    // we don't use cancellation tokens because AcceptTcpClientAsync does not support them anyway
    private volatile bool _shutdown;
    private TcpListener _listener;

    public async Task Start() {
        _listener = new TcpListener(IPAddress.Parse("127.0.0.1"), 10010);
        _listener.ExclusiveAddressUse = false;
        _listener.Start();
        while (!_shutdown) {
            TcpClient client = null;
            try {
                client = await _listener.AcceptTcpClientAsync();
            }
            catch (ObjectDisposedException) {
                // will be thrown when you stop listener   
            }
            if (client != null) {
                var tcpConnectedLogic = new TcpConnectionLogic(client);
                // start processing on thread pool thread if necessary
                // don't wait for it to finish - you need to accept more connections
                tcpConnectedLogic.StartAsync();
            }
        }
    }

    public void Stop() {
        _shutdown = true;
        _listener.Stop();
    }
}