Loop won't stop with Thread and CancellationToken

Question

I'm working with a Windows socket application using async callbacks. If I use Thread to start _StartListening, when I call StopListening, the loop still stops at allDone.WaitOne(). But the Task version will be OK.

What's the difference?

My code is a modified version of this

The original version with ManualResetEvent has a race condition mentioned by felix-b. I changed it to SemaphoreSlim but the problem is still there.

I tried in debug mode and it seems that the break point never be hit at if (cancelToken.IsCancellationRequested) after I call StopListening even I don't start the client.

Sorry. I found that I accidentally started two socket servers. That's the problem.

  class WinSocketServer:IDisposable
  {
        public SemaphoreSlim semaphore = new SemaphoreSlim(0);
        private CancellationTokenSource cancelSource = new CancellationTokenSource();
        public void AcceptCallback(IAsyncResult ar)
        {
            semaphore.Release();
            //Do something
        }

        private void _StartListening(CancellationToken cancelToken)
        {
            try
            {
                while (true)
                {
                    if (cancelToken.IsCancellationRequested)
                        break;
                    Console.WriteLine("Waiting for a connection...");
                    listener.BeginAccept(new AsyncCallback(AcceptCallback),listener);
                    semaphore.Wait();
                }
            }
            catch (Exception e)
            {
                Console.WriteLine(e.ToString());
            }
            Console.WriteLine("Complete");
        }
        public void StartListening()
        {
            Task.Run(() => _StartListening(cancelSource.Token));//OK

            var t = new Thread(() => _StartListening(cancelSource.Token));
            t.Start();//Can't be stopped by calling StopListening
        }

        public void StopListening()
        {
            listener.Close();
            cancelSource.Cancel();
            semaphore.Release();
        }

        public void Dispose()
        {
            StopListening();
            cancelSource.Dispose();
            semaphore.Dispose();
        }
    }

Answer 1

Your code has a race condition that can lead to deadlock (sometimes). Let's name the threads "listener" (one that runs _StartListening) and "control" (one that runs StopListening):

1. Listener thread: if (cancelToken.IsCancellationRequested) -> false
2. Control thread: cancelSource.Cancel()
3. Control thread: allDone.Set()
4. Listener thread: allDone.Reset() -> accidentally resets the stop request!
5. Listener thread: listener.BeginAccept(...)
6. Control thread: stopListening() exits, while the listener continues to work!
7. Listener thread: allDone.WaitOne() -> deadlock! no one will do allDone.Set().

The problem is in how you use the allDone event, it should be the other way around: _StartListening should do allDone.Set() just before it exits for whatever reason, whereas StopListening should do allDone.WaitOne():

class WinSocketServer:IDisposable
{
    // I guess this was in your code, necessary to show proper stopping
    private Socket listener = new Socket(......); 

    public ManualResetEvent allDone = new ManualResetEvent(false);
    private CancellationTokenSource cancelSource = new CancellationTokenSource();

    private void _StartListening(CancellationToken cancelToken)
    {
        try
        {
            listener.Listen(...); // I guess 
            allDone.Reset(); // reset once before starting the loop
            while (!cancelToken.IsCancellationRequested)
            {
                Console.WriteLine("Waiting for a connection...");
                listener.BeginAccept(new AsyncCallback(AcceptCallback),listener);
            }
        }
        catch (Exception e)
        {
            Console.WriteLine(e.ToString());
        }

        allDone.Set(); // notify that the listener is exiting
        Console.WriteLine("Complete");
    }
    public void StartListening()
    {
        Task.Run(() => _StartListening(cancelSource.Token));
    }
    public void StopListening()
    {
        // notify the listener it should exit
        cancelSource.Cancel(); 
        // cancel possibly pending BeginAccept
        listener.Close();
        // wait until the listener notifies that it's actually exiting
        allDone.WaitOne();
    }
    public void Dispose()
    {
        StopListening();
        cancelSource.Dispose();
        allDone.Dispose();
    }
}

UPDATE

It worth noting that listener.BeginAccept won't return until there is a new client connection. When stopping the listener, it is necessary to close the socket (listener.Close()) to force BeginAccept to exit.

The difference in Thread/Task behavior is really weird, it probably can originate from the Task thread being a background thread, while the regular thread being a foreground one.