Task.Delay didn’t get canceled?

Question

I am trying to build an interface for a game. The game runs for 1 minute. The GetStop method stops after 60 sec game. The play method starts the game and the quit method quit the game. Now ideally what I want is when I quit the game after 30 seconds, the timer should get reset and on click of the Play button, the timer should again run for 1 minute. So that the next game gets to run for 1 minute. If I press Quit button then again, the timer should be reset for the next game.

However, there seems to be a certain issue in my code. Whenever I execute quit method the timer seems to be saved at that state. So, If I quit a race in 30 seconds then the next race will last for only 30 seconds. If I quit a race in 50 seconds, the next race will last only 10 seconds. Ideally, the timer should get reset but it is not getting reset.

I am out of ideas here. Can anyone please provide some suggestions??

private async Task GetStop(CancellationToken token)
{ 
    await Task.Run(async () =>
    {
        token.ThrowIfCancellationRequested();
        await Task.Delay(TimeSpan.FromSeconds(60), token);

        token.ThrowIfCancellationRequested();
        if (!token.IsCancellationRequested)
        {
            sendMessage((byte)ACMessage.AC_ESCAPE); 
        }
    }, token);
}

public async void Play()
{         
        sendMessage((byte)ACMessage.AC_START_RACE); 
        _cts.Cancel();

        if (_cts != null)
        {
            _cts.Dispose();
            _cts = null;
        }
        _cts = new CancellationTokenSource(); 
        await GetStop(_cts.Token);
   }

public void Quit()
{
        _cts.Cancel();
        if (_cts != null)
        {
            _cts.Dispose();
            _cts = null;
        }
    //
}

Answer 1

I can see that your code may throw exceptions at several places. If you are catching and ignoring all exceptions, you may not be able to see the reason why the time, cancellation token and tasks are not working correctly.

At a first moment, I could identify the following:

private async Task GetStop(CancellationToken token)
{ 
    await Task.Run(async () =>
    {
        // I think you don't need to throw here
        token.ThrowIfCancellationRequested();

        // this will throw an Exception when cancelled
        await Task.Delay(TimeSpan.FromSeconds(60), token); 

        // again, I think you don't need to throw here
        token.ThrowIfCancellationRequested();

        if (!token.IsCancellationRequested)
        {
            sendMessage((byte)ACMessage.AC_ESCAPE); 
        }
    }, token);
}

public async void Play()
{         
        sendMessage((byte)ACMessage.AC_START_RACE); 

        // at some scenarios this may be null
        _cts.Cancel();

        if (_cts != null)
        {
            _cts.Dispose();
            _cts = null;
        }
        _cts = new CancellationTokenSource(); 
        await GetStop(_cts.Token);
   }

public void Quit()
{
        _cts.Cancel();
        if (_cts != null)
        {
            _cts.Dispose();
            _cts = null;
        }
}

I created a Console application, did some small modifications, and here it seems to work just fine. Please take a look:

public static class Program
{
    public static void Main(string[] args)
    {
        var game = new Game();

        game.Play();
        Task.Delay(5000).Wait();
        game.Quit();

        game.Play();
        Task.Delay(15000).Wait();
        game.Quit();

        game.Play();
        Task.Delay(65000).Wait();

        Console.WriteLine("Main thread finished");
        Console.ReadKey();

        // Output:
        //
        // Start race (-00:00:00.0050018)
        // Quit called (00:00:05.0163131)
        // Timeout (00:00:05.0564685)
        // Start race (00:00:05.0569656)
        // Quit called (00:00:20.0585092)
        // Timeout (00:00:20.1025051)
        // Start race (00:00:20.1030095)
        // Escape (00:01:20.1052507)
        // Main thread finished
    }
}

internal class Game
{
    private CancellationTokenSource _cts;

    // this is just to keep track of the behavior, should be removed
    private DateTime? _first;
    private DateTime First
    {
        get
        {
            if (!_first.HasValue) _first = DateTime.Now;
            return _first.Value;
        }
    }


    private async Task GetStop(CancellationToken token)
    {
        await Task.Run(async () =>
        {
            try
            {
                // we expect an exception here, if it is cancelled
                await Task.Delay(TimeSpan.FromSeconds(60), token);
            }
            catch (Exception)
            {
                Console.WriteLine("Timeout ({0})", DateTime.Now.Subtract(First));
            }

            if (!token.IsCancellationRequested)
            {
                Console.WriteLine("Escape ({0})", DateTime.Now.Subtract(First));
            }
        }, token);
    }

    public async void Play()
    {
        Console.WriteLine("Start race ({0})", DateTime.Now.Subtract(First));

        CancelAndDisposeCts();

        _cts = new CancellationTokenSource();
        await GetStop(_cts.Token);
    }

    public void Quit()
    {
        Console.WriteLine("Quit called ({0})", DateTime.Now.Subtract(First));
        CancelAndDisposeCts();
    }

    private void CancelAndDisposeCts()
    {
        // avoid copy/paste for the same behavior
        if (_cts == null) return;

        _cts.Cancel();
        _cts.Dispose();
        _cts = null;
    }
}

I would also suggest to take a look on System.Threading.Timer, maybe if can be useful for some scenarios...

Good luck with your game!