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)

Kreda

Answer 1

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)
  {

  }

}

Toxophilite

I am reading this question and answer(s). Now I see this answer got +4 and -5. I do not know if the answer is right or not. I am sure this scenario it would be helpful if the minus givers wrote a few words, so the future reader (me this case) should not guess, instead could do its job more efficiently. (what is one of the main purpose of this site) – Sorensen

I did not downvote, but I'm guessing that in your solution the call to AccptTcpClnt blocks with no way to cancel. – Manyplies

Personally, any example including a Thread.Abort() without thorough justification is deserving of a downvote. Not to mention there are cleaner, simpler solutions as shown in other answers – Ideation

Answer 2

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.

Counterinsurgency

I like the part about checking for SocketException.SocketErrorCode – Furuncle

The answer has an "Async..." part. After reading carefully the reader must conclude, that both variations are async because the elegant "..." in the first code exhibit implies an other thread or Task. I know that using Listener hints a "Server" context, and Server hints multithreading. Still it is possible to have a single threaded peer-to-peer TCP communication. So it worth to mention that answer is only for multi threaded architecture, with async execution. – Sorensen

Is there anything else that can fire a SocketError.Interrupted? The docs only say "A blocking Socket call was canceled." but not if there are different things that could cancel it. Could a Client or random Internet Connection Failiure cause this in any way? – Teleplay

Answer 3

For completeness, async counterpart of the answer above, usings @Mitch's suggestion (confirmed here confirms).

In contrast to the synchronous function awaiting AcceptTcpClientAsync seems to 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(ct);
        }
        catch (ObjectDisposedException) when (ct.IsCancellationRequested)
        {
            throw new OperationCanceledException(ct);
        }
    }
}

---

Update from 2021: .NET 5 throws SocketException, whereas the .NET Framework (tested with versions 4.5-4.8) and .NET Core 2.x-3.x throws an ObjectDisposedException. So as of today, the correct code would be

#if NET5_0
catch (SocketException ex) when (ct.IsCancellationRequested &&
                                 ex.SocketErrorCode == SocketError.OperationAborted)
#elif (NETFRAMEWORK && NET40_OR_GREATER) || NETCOREAPP2_0_OR_GREATER
catch (ObjectDisposedException ex) when (ct.IsCancellationRequested)
#else
#error Untested target framework
#endif
{
    throw new OperationCanceledException(ct);
}

---

Update from 2023: .NET 6+ has now AcceptTcpClientAsync overload with cancellation token, which should be used for cancellation, so the final cross-version polyfill code would check

#if NET6_0_OR_GREATER
return await listener.AcceptTcpClientAsync(ct);
#else
using (ct.Register(listener.Stop))
...

---

The synchronous counterpart (listener.AcceptTcpClient()) throws consistently SocketException with SocketErrorCode == Interrupted, so the following would do in all frameworks:

try
{
    return serverSocket.AcceptTcpClient();
}
catch (SocketException e) when (e.SocketErrorCode == SocketError.Interrupted)
{
    throw new OperationCanceledException(ct);
}

Arapaima

With new .NET versions, this is the desired solution. (well, the true desired solution would be for AcceptTcpClientAsync() to take a cancellation token). – Thyestes

I have also had AcceptTcpClientAsync throw ObjectDisposedException post-Stop(), so you may want to trap for both. – Ramp

@KFL: In .NET 6+ it really take takes a cancellation token now. – Arapaima

Answer 4

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 :)

Dispensation

Answer 5

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);

    }
    }

Eloiseelon

Unless CancellationTokens work differently than I think, this is still going to give the same issue. AcceptTcpClient is still a blocking method and it's going to block until something tries to connect. – Toile