1

I'm running into the issue where the task being executed below runs asynchronously but is unable to be cancelled when the serializer reads the memory stream. When the user makes the cancellation request (by pressing a cancel button), a cancellation (the method cancel() is called fromt he token) is made but the task continues.

Service Class:
Async method called from LoadHelper() in Main class

   public async void StartTask(Action callback, CancellationToken token)
    {
        await Task.Run(callback, token);
    }

Main Class:

    private void LoadHelper()
    {
        _services.GetInstance<IThreadService>().StartTask(
            () => LoadHelperAsync(), _cancelService.Token);
    }

Method being run async

    private void LoadHelperAsync()
    {
        var serializer = new DataContractSerializer(typeof(UserDatabase));
        string selectedDir;
        ComparisonResult = null;

        _services.GetInstance<IOpenFileService>().DisplayOpenFileDialog(
            "Select a saved comparison",
            "Comparison File(*.xml; *.cmp)|*.xml;*.cmp",
            Environment.GetFolderPath(Environment.SpecialFolder.Desktop),
            out selectedDir);

        if (!string.IsNullOrEmpty(selectedDir))
        {
            _dispatchService.BeginInvoke(() => IsExecuting = true);
            using (FileStream fileStream = new FileStream(selectedDir, FileMode.Open, FileAccess.Read))
            using (DeflateStream compressedStream = new DeflateStream(fileStream, CompressionMode.Decompress))
            using (BufferedStream regularStream = new BufferedStream(fileStream))
            {
                Stream memoryStream;

                //Use filename to determine compression
                if (selectedDir.EndsWith(".cmp", true, null))
                {
                    memoryStream = compressedStream;
                }
                else
                {
                    memoryStream = regularStream;
                }

                Report("Loading comparison");

                Report(0);
                IsExecuting = true;
                ComparisonResult = (UserDatabase)serializer.ReadObject(memoryStream);

                memoryStream.Close();
                fileStream.Close();
                IsExecuting = false;
                Report("Comparison loaded");
            }
            _dispatchService.BeginInvoke(() => IsExecuting = false);
            _dispatchService.BeginInvoke(() => ViewResults.ExecuteIfAble());
        }
        else
        {
            Report("No comparison loaded");
        }

Cancellation code:

This command is binded to a "cancel" button in the view.

     CancelCompare = new Command(o => _cancelService.Cancel(), o => IsExecuting);

From the CancellationService class

    public class CancellationService : ICancellationService, IDisposable
{
    private CancellationTokenSource _tokenSource;

    public CancellationService()
    {
        Reset();
    }

    public CancellationToken Token { get; private set; }

    public void Cancel()
    {
        _tokenSource.Cancel();
    }

    public void Reset()
    {
        _tokenSource = new CancellationTokenSource();
        Token = _tokenSource.Token;
    }

    protected virtual void Dispose(bool disposing)
    {
        if (disposing)
        {
            _tokenSource.Cancel();
            _tokenSource.Dispose();
        }
    }

    public void Dispose()
    {
        Dispose(true);
        GC.SuppressFinalize(this);
    }
}
  • 3
    Why do you have async void? Why did you name `LoadHelperAsync` with the -Async suffix convention when it's not async? – mason Jul 30 '18 at 14:34
  • Please add how you are cancelling the task – Camilo Terevinto Jul 30 '18 at 14:34
  • 2
    @RickyThomas cancellation is *cooperative*. Calling `.Cancel()` won't abort the thread, it will signal the token. *Your* action has to receive the token and check it it's raised. If you want deserialization to be cancelled you should use async methods inside `LoadHelperAsync` and pass the token to them as well. *Not* BeginInvoke, that's essentially a `Thread.Start` or a direct call to another thread, depending on what `_dispatchService` is – Panagiotis Kanavos Jul 30 '18 at 14:49
  • @mason the code is run async from the service class. I think the original writer did it because of that but I cannot speak of his choice in suffix convention unfortunately. – Ricky Thomas Jul 30 '18 at 14:50
  • Possible duplicate of [How to cancel a Task in await?](https://stackoverflow.com/questions/10134310/how-to-cancel-a-task-in-await) – Liam Jul 30 '18 at 14:51
  • @PanagiotisKanavos how should I go about checking if the token has been changed when the serializer is reading the memory stream? – Ricky Thomas Jul 30 '18 at 14:52
  • You should stop what you're doing and read [Best Practices in Async/Await](https://msdn.microsoft.com/en-us/magazine/jj991977.aspx). That `async void` is going to get you in a lot of trouble. And you should rename your method appropriately. – mason Jul 30 '18 at 14:53
  • 1
    @RickyThomas this code is far too complex. A cts and token aren't services, they are *primitives* that need to be passed around. Asking for input should be performed outside the async operations themselves. That's the point of `await`, it allows you to work on the UI thread before and after the `await` call. Progress reporting should use the `IProgress` interface. Check Stephen Toub's [Enabling Progress and Cancellation in Async APIs](https://blogs.msdn.microsoft.com/dotnet/2012/06/06/async-in-4-5-enabling-progress-and-cancellation-in-async-apis/) – Panagiotis Kanavos Jul 30 '18 at 14:53
  • @RickyThomas read Stephen Toub's article as it explains both cancellation and progress. *Extract* all UI code from the `LoadHelperAsync` class. You don't need `BeginInvoke` when you use `await`. The only thing that needs to be async is the call to `serializer.ReadObject` and its stream. – Panagiotis Kanavos Jul 30 '18 at 15:01
  • A cancellation token is magic. It's just a value which is checked basically every time you call a system async method, but if you're not using any system methods, you'll need to check it yourself and respond in whatever way you deem appropriate (sometimes this is throwing an exception, sometimes it's stopping a running task gracefully, sometimes it's returning early. – cwharris Jul 30 '18 at 16:36

1 Answers1

2

Calling _tokenSource.Cancel(); won't do anything to a running task. A cancellation token interrupts task execution only if it was canceled before Task.Run

Take a look:

static void Main(string[] args)
{
    using (var tokenSource = new CancellationTokenSource())
    {
        var aTask = StartTask(() =>
        {
            while (true)
            {
                Console.WriteLine("Nothing is going to happen.");
                // Some long operation
                Thread.Sleep(1000);
            }
        }, tokenSource.Token);

        tokenSource.Cancel();
        aTask.Wait();
    }

    Console.ReadKey();
}

static async Task StartTask(Action callback, CancellationToken cancellationToken)
{
    await Task.Run(callback, cancellationToken);
}

If you want to cancel the execution, you should check the cancellation token by yourself, there is no any magic which cancels the task for you:

static void Main(string[] args)
{
    using (var tokenSource = new CancellationTokenSource())
    {
        var aTask = StartTask(cancellationToken =>
        {
            while (true)
            {
                cancellationToken.ThrowIfCancellationRequested();
                Console.WriteLine("Will stop before that if canceled.");
                // Some long operation
                // Also pass the token all way down
                Task.Delay(1000, cancellationToken).Wait();
            }
        }, tokenSource.Token);

        // Try 0, 500, 1500 to see the difference
        Thread.Sleep(1500);
        tokenSource.Cancel();

        try
        {
            aTask.Wait();
        }
        catch (Exception ex)
        {
            // AggregateException with OperationCanceledException
            Console.WriteLine("Task was canceled.");
            Console.WriteLine(ex.ToString());
        }
    }

    Console.ReadKey();
}

static async Task StartTask(Action<CancellationToken> callback, CancellationToken cancellationToken)
{
    await Task.Run(() => callback(cancellationToken), cancellationToken);
}

Note that this code only illustrates how the task works, never write anything like this in production.

Kosta_Arnorsky
  • 363
  • 3
  • 7