Cancel blocking AcceptTcpClient call

Question

As everyone may already know, the simplest way to accept incoming TCP connections in C# is by looping over TcpListener.AcceptTcpClient(). Additionally this way will block code execution until a connection is obtained. This is extremely limiting to a GUI, so I want to listen for connections in either a seperate thread or task.

I have been told, that threads have several disadvantages, however nobody explained me what these are. So instead of using threads, I used tasks. This works great, however since the AcceptTcpClient method is blocking execution, I can't find any way of handling a task cancellation.

Currently the code looks like this, but I have no idea how I would want to cancel the task when I want the program to stop listening for connections.

First off the function executed in the task:

static void Listen () {
// Create listener object
TcpListener serverSocket = new TcpListener ( serverAddr, serverPort );

// Begin listening for connections
while ( true ) {
    try {
        serverSocket.Start ();
    } catch ( SocketException ) {
        MessageBox.Show ( "Another server is currently listening at port " + serverPort );
    }

    // Block and wait for incoming connection
    if ( serverSocket.Pending() ) {
        TcpClient serverClient = serverSocket.AcceptTcpClient ();
        // Retrieve data from network stream
        NetworkStream serverStream = serverClient.GetStream ();
        serverStream.Read ( data, 0, data.Length );
        string serverMsg = ascii.GetString ( data );
        MessageBox.Show ( "Message recieved: " + serverMsg );

        // Close stream and TcpClient connection
        serverClient.Close ();
        serverStream.Close ();

        // Empty buffer
        data = new Byte[256];
        serverMsg = null;
    }
}

Second, the functions starting and stopping the listening service:

private void btnListen_Click (object sender, EventArgs e) {
    btnListen.Enabled = false;
    btnStop.Enabled = true;
    Task listenTask = new Task ( Listen );
    listenTask.Start();
}

private void btnStop_Click ( object sender, EventArgs e ) {
    btnListen.Enabled = true;
    btnStop.Enabled = false;
    //listenTask.Abort();
}

I just need something to replace the listenTask.Abort() call (Which I commented out because the method doesn't exist)

Answer 1

Here's how I overcame this. Hope this help. Might not be the cleanest, but works for me

    public class consoleService {
    private CancellationTokenSource cts;
    private TcpListener listener;
    private frmMain main;
    public bool started = false;
    public bool stopped = false;

   public void start() {
        try {
            if (started) {
                stop();
            }
            cts = new CancellationTokenSource();
            listener = new TcpListener(IPAddress.Any, CFDPInstanceData.Settings.RemoteConsolePort);
            listener.Start();
            Task.Run(() => {
                AcceptClientsTask(listener, cts.Token);
            });

            started = true;
            stopped = false;
            functions.Logger.log("Started Remote Console on port " + CFDPInstanceData.Settings.RemoteConsolePort, "RemoteConsole", "General", LOGLEVEL.INFO);

        } catch (Exception E) {
            functions.Logger.log("Error starting remote console socket: " + E.Message, "RemoteConsole", "General", LOGLEVEL.ERROR);
        }
    }

    public void stop() {
        try {
            if (!started) { return; }
            stopped = false;
            cts.Cancel();
            listener.Stop();
            int attempt = 0;
            while (!stopped && attempt < GlobalSettings.ConsoleStopAttempts) {
                attempt++;
                Thread.Sleep(GlobalSettings.ConsoleStopAttemptsDelayMS);
            }

        } catch (Exception E) {
            functions.Logger.log("Error stopping remote console socket: " + E.Message, "RemoteConsole", "General", LOGLEVEL.ERROR);
        } finally {
            started = false;
        }
    }

     void AcceptClientsTask(TcpListener listener, CancellationToken ct) {

        try {
            while (!ct.IsCancellationRequested) {
                try {
                    TcpClient client = listener.AcceptTcpClient();
                    if (!ct.IsCancellationRequested) {
                        functions.Logger.log("Client connected from " + client.Client.RemoteEndPoint.ToString(), "RemoteConsole", "General", LOGLEVEL.DEBUG);
                        ParseAndReply(client, ct);
                    }

                } catch (SocketException e) {
                    if (e.SocketErrorCode == SocketError.Interrupted) {
                        break;
                    } else {
                        throw e;
                    }
                 } catch (Exception E) {
                    functions.Logger.log("Error in Remote Console Loop: " + E.Message, "RemoteConsole", "General", LOGLEVEL.ERROR);
                }

            }
            functions.Logger.log("Stopping Remote Console Loop", "RemoteConsole", "General", LOGLEVEL.DEBUG); 

        } catch (Exception E) {
            functions.Logger.log("Error in Remote Console: " + E.Message, "RemoteConsole", "General", LOGLEVEL.ERROR);
        } finally {
            stopped = true;

        }
        functions.Logger.log("Stopping Remote Console", "RemoteConsole", "General", LOGLEVEL.INFO);

    }
    }

Answer 2

For completeness, async counterpart of the answer above:

async Task<TcpClient> AcceptAsync(TcpListener listener, CancellationToken ct)
{
    using (ct.Register(listener.Stop))
    {
        try
        {
            return await listener.AcceptTcpClientAsync();
        }
        catch (SocketException e)
        {
            if (e.SocketErrorCode == SocketError.Interrupted)
                throw new OperationCanceledException();
            throw;
        }
    }
}

---

Update: As @Mitch suggests in comments (and as this discussion confirms), awaiting AcceptTcpClientAsync may throw ObjectDisposedException after Stop (which we are calling anyway), so it makes sense to catch ObjectDisposedException too:

async Task<TcpClient> AcceptAsync(TcpListener listener, CancellationToken ct)
{
    using (ct.Register(listener.Stop))
    {
        try
        {
            return await listener.AcceptTcpClientAsync();
        }
        catch (SocketException e) when (e.SocketErrorCode == SocketError.Interrupted)
        {
            throw new OperationCanceledException();
        }
        catch (ObjectDisposedException) when (ct.IsCancellationRequested)
        {
            throw new OperationCanceledException();
        }
    }
}

Answer 3

Well, in the olden days before properly working asynchronous sockets (the best way today IMO, BitMask talks about this), we've used a simple trick: set the isRunning to false (again, ideally, you want to use CancellationToken instead, public static bool isRunning; is not a thread-safe way to terminate a background worker :)) and start a new TcpClient.Connect to yourself - this will return you from the Accept call and you can terminate gracefully.

As BitMask already said, Thread.Abort most definitely isn't a safe approach at termination. In fact, it wouldn't work at all, given that Accept is handled by native code, where Thread.Abort has no power. The only reason it works is because you're not actually blocking in the I/O, but rather running an infinite loop while checking for Pending (non-blocking call). This looks like a great way to have 100% CPU usage on one core :)

Your code has a lot of other issues too, which don't blow up in your face only because you're doing very simple stuff, and because of .NET being rather nice. For example, you're always doing GetString on the whole buffer you're reading into - but that's wrong. In fact, that's a textbook example of a buffer overflow in e.g. C++ - the only reason it seems to work in C# is because it pre-zeroes the buffer, so GetString ignores the data after the "real" string you read. Instead, you need to take the return value of the Read call - that tells you how many bytes you've read, and as such, how many you need to decode.

Another very important benefit of this is it means you no longer have to recreate the byte[] after each read - you can simply reuse the buffer over and over again.

Don't work with the GUI from other thread than the GUI thread (yes, your Task is running in a separate thread pool thread). MessageBox.Show is a dirty hack that in fact does work from other threads, but that really isn't what you want. You need to invoke the GUI actions on the GUI thread (for example using Form.Invoke, or by using a task that has a synchronization context on the GUI thread). That will mean the message box will be the proper dialog you'd expect.

There's many more issues with the snippet you posted, but given that this isn't Code Review, and that it's an old thread, I'm not going to make this any longer :)

Answer 4

Cancelling AcceptTcpClient

Your best bet for cancelling the blocking AcceptTcpClient operation is to call TcpListener.Stop which will throw a SocketException that you can catch if you want to explicitly check that the operation was cancelled.

       TcpListener serverSocket = new TcpListener ( serverAddr, serverPort );

       ...

       try
       {
           TcpClient serverClient = serverSocket.AcceptTcpClient ();
           // do something
       }
       catch (SocketException e)
       {
           if ((e.SocketErrorCode == SocketError.Interrupted))
           // a blocking listen has been cancelled
       }

       ...

       // somewhere else your code will stop the blocking listen:
       serverSocket.Stop();

Whatever wants to call Stop on your TcpListener will need some level of access to it, so you would either scope it outside of your Listen method or wrap your listener logic inside of an object that manages the TcpListener and exposes Start and Stop methods (with Stop calling TcpListener.Stop()).

Async Termination

Because the accepted answer uses Thread.Abort() to terminate the thread it might be helpful to note here that the best way to terminate an asynchronous operation is by cooperative cancellation rather than a hard abort.

In a cooperative model, the target operation can monitor a cancellation indicator which is signaled by the terminator. This allows the target to detect a cancellation request, clean up as needed, and then at an appropriate time communicate status of the termination back to the terminator. Without an approach like this, abrupt termination of the operation can leave the thread's resources and possibly even the hosting process or app domain in a corrupt state.

From .NET 4.0 onward, the best way to implement this pattern is with a CancellationToken. When working with threads the token can be passed in as a parameter to the method executing on the thread. With Tasks, support for CancellationTokens is built into several of the Task constructors. Cancellation tokes are discussed in more detail in this MSDN article.

Answer 5

The following code will close/abort AcceptTcpClient when isRunning variable becomes false

public static bool isRunning;

delegate void mThread(ref book isRunning);
delegate void AccptTcpClnt(ref TcpClient client, TcpListener listener);

public static main()
{
   isRunning = true;
   mThread t = new mThread(StartListening);
   Thread masterThread = new Thread(() => t(this, ref isRunning));
   masterThread.IsBackground = true; //better to run it as a background thread
   masterThread.Start();
}

public static void AccptClnt(ref TcpClient client, TcpListener listener)
{
  if(client == null)
    client = listener.AcceptTcpClient(); 
}

public static void StartListening(ref bool isRunning)
{
  TcpListener listener = new TcpListener(new IPEndPoint(IPAddress.Any, portNum));

  try
  {
     listener.Start();

     TcpClient handler = null;
     while (isRunning)
     {
        AccptTcpClnt t = new AccptTcpClnt(AccptClnt);

        Thread tt = new Thread(() => t(ref handler, listener));
        tt.IsBackground = true;
        // the AcceptTcpClient() is a blocking method, so we are invoking it
        // in a separate dedicated thread 
        tt.Start(); 
        while (isRunning && tt.IsAlive && handler == null) 
        Thread.Sleep(500); //change the time as you prefer


        if (handler != null)
        {
           //handle the accepted connection here
        }        
        // as was suggested in comments, aborting the thread this way
        // is not a good practice. so we can omit the else if block
        // else if (!isRunning && tt.IsAlive)
        // {
        //   tt.Abort();
        //}                   
     }
     // when isRunning is set to false, the code exits the while(isRunning)
     // and listner.Stop() is called which throws SocketException 
     listener.Stop();           
  }
  // catching the SocketException as was suggested by the most
  // voted answer
  catch (SocketException e)
  {

  }

}