Managing/clearing up cancelled tasks

Question

I have the following requirements on one of my ViewModels

A fire and forget Load method to refresh some state that is retrieved on a background worker.
The Load method should do the expensive work on a task, and then do a continuewith back on the UI thread to update the current state on the viewmodel and thus the view
The Load method can be repeatedly called. If a task is already in progress, it should be cancelled, and a new one created.
Whilst a task is running, the UI should display this using a IsLoading state.
When the ViewModel is disposed, it should wait (block) on any current tasks that have not yet completed. I don't want to leave the Dispose method whilst there are still tasks running OnLoadBackground

I have it all working except for the last requirement.

If I call Load() multiple times in a row, it correctly cancels the last task before starting a new one, but if I then call Dispose(), the following code throws an Aggregate exception containing a TaskCanceledException for each task in the array.

// wait for finish (this needs to block)
Task.WaitAll(_tasks.Keys.ToArray());

Although they might be cancelled, OnLoadBackground can still running, and I need to wait for it to complete, even though it is cancelled.

public class DocumentObjectViewModel : ObservableObject, IDisposable
{
    private readonly Dictionary<Task, CancellationTokenSource> _tasks = new();

    /// <summary>
    /// shows an indeterminate progress bar in the UI
    /// </summary>
    public bool IsLoading
    {
        get => _isLoading;
        set => SetProperty(ref _isLoading, value);
    }

    private bool _isLoading;

    public ICommand RefreshCMD { get; }

    public string Value
    {
        get => _value;
        set => SetProperty(ref _value, value);
    }

    private string _value = "Not Set";

    public DocumentObjectViewModel()
    {
        RefreshCMD = new RelayCommand(Load);
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Dispose()
    {
        // cancel all
        foreach (var load in _tasks)
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;

            c.Cancel();
        }

        // wait for finish (this needs to block)

        Task.WaitAll(_tasks.Keys.ToArray());

        // dispose 
        foreach (var load in _tasks)
        {
            CancellationTokenSource c = load.Value;
            c.Dispose();
        }

        _tasks.Clear();
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Load()
    {
        // cancel previous loads
        foreach (var load in _tasks.ToArray())
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;
            c.Cancel();
        }

        CancellationTokenSource cancel = new();

        cancel.Token.Register(() =>
        {
            // task was canceled
            IsLoading = false;
        });

        IsLoading = true;

        Task task = Task.Run(() =>
        {
            string? value = OnLoadBackground(cancel.Token);
            return value;

        }, cancel.Token).ContinueWith((x) =>
        {
            IsLoading = false;

            if (x.Status == TaskStatus.Faulted)
            {
                // report error
                return;
            }

            // completed.
            OnLoadUI(x.Result);
        },
        cancel.Token,
        TaskContinuationOptions.None,
        TaskScheduler.FromCurrentSynchronizationContext());

        _tasks.Add(task, cancel);
    }

    private string? OnLoadBackground(CancellationToken cancellationToken)
    {
        if (cancellationToken.IsCancellationRequested)
            return "Canceled";



    Debug.WriteLine("Started...");

    // do work (passing in cancellationToken)
    Thread.Sleep(5000);

    Debug.WriteLine("Ended...");

    if (cancellationToken.IsCancellationRequested)
        return "Canceled";

        return $"{DateTime.Now}";
    }

    private void OnLoadUI(string? retrievedData)
    {
        Value = retrievedData ?? "NULL";
    }
}

A lot of the examples I have seen will use await. But I can't use that in my Dispose(). I just want it to block.

EDIT

I think I have it working now. I removed the ContinueWith and replaced it with a Dispatcher call. Not sure if this is the correct way though?

public class DocumentObjectViewModel : ObservableObject, IDisposable
{
    private readonly Dictionary<Task, CancellationTokenSource> _tasks = new();

    /// <summary>
    /// shows an indeterminate progress bar in the UI
    /// </summary>
    public bool IsLoading
    {
        get => _isLoading;
        set => SetProperty(ref _isLoading, value);
    }

    private bool _isLoading;

    public ICommand RefreshCMD { get; }

    public string Value
    {
        get => _value;
        set => SetProperty(ref _value, value);
    }
    
    private string _value = "Not Set";

    public DocumentObjectViewModel()
    {
        RefreshCMD = new RelayCommand(Load);
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Dispose()
    {
        // cancel all       
        foreach (var load in _tasks)
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;

            c.Cancel();
        }

        // wait for finish (this needs to block)
        Task.WaitAll(_tasks.Keys.ToArray());

        // dispose 
        foreach (var load in _tasks)
        {
            CancellationTokenSource c = load.Value;
            c.Dispose();
        }

        _tasks.Clear();
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Load()
    {
        // cancel previous loads
        foreach (var load in _tasks.ToArray())
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;
            c.Cancel();
        }

        CancellationTokenSource cancel = new();

        cancel.Token.Register(() =>
        {
            // task was canceled
            IsLoading = false;
        });

        IsLoading = true;

        Task task = Task.Run(() =>
        {
            string? value = OnLoadBackground(cancel.Token);



            Application.Current.Dispatcher.BeginInvoke(() =>
            {
                if (cancel.IsCancellationRequested)
                    return;

                IsLoading = false;
                OnLoadUI(value);
            });
        });

        _tasks.Add(task, cancel);
    }

    private string? OnLoadBackground(CancellationToken cancellationToken)
    {
        if (cancellationToken.IsCancellationRequested)
            return "Canceled";



        Debug.WriteLine("Started...");

        // do work (passing in cancellationToken)
        Thread.Sleep(5000);

        Debug.WriteLine("Ended...");

        if (cancellationToken.IsCancellationRequested)
            return "Canceled";

        return $"{DateTime.Now}";
    }

    private void OnLoadUI(string? retrievedData)
    {
        Value = retrievedData ?? "NULL";
    }
}

EDIT 2 :

ok ... I figured it out. It was all because I was waiting on the wrong task. I should have been waiting on the parent task, not the continuation task ...

public class DocumentObjectViewModel2 : ObservableObject, IDisposable
{
    private readonly Dictionary<Task, CancellationTokenSource> _tasks = new();

    /// <summary>
    /// shows an indeterminate progress bar in the UI
    /// </summary>
    public bool IsLoading
    {
        get => _isLoading;
        set => SetProperty(ref _isLoading, value);
    }

    private bool _isLoading;

    public ICommand RefreshCMD { get; }

    public string Value
    {
        get => _value;
        set => SetProperty(ref _value, value);
    }

    private string _value = "Not Set";

    public DocumentObjectViewModel2()
    {
        RefreshCMD = new RelayCommand(Load);
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Dispose()
    {
        // cancel all
        foreach (var load in _tasks)
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;

            c.Cancel();
        }

        // wait for finish (this needs to block)


        Task.WaitAll(_tasks.Keys.ToArray());

        // dispose 
        foreach (var load in _tasks)
        {
            CancellationTokenSource c = load.Value;
            c.Dispose();
        }

        _tasks.Clear();
    }

    /// <summary>
    /// This is only called from the UI thread
    /// </summary>
    public void Load()
    {
        // cancel previous loads
        foreach (var load in _tasks.ToArray())
        {
            Task t = load.Key;
            CancellationTokenSource c = load.Value;
            c.Cancel();
        }

        CancellationTokenSource cancel = new();

        cancel.Token.Register(() =>
        {
            // task was canceled
            IsLoading = false;
        });

        IsLoading = true;

        Task<string?> task = Task.Run(async () =>
        {
            string? value = OnLoadBackground(cancel.Token);
            return value;

        }, cancel.Token);
            
        task.ContinueWith((x) =>
        {
            IsLoading = false;

            if (x.Status == TaskStatus.Faulted)
            {
                // report error
                return;
            }

            // completed.
            OnLoadUI(x.Result);
        },
        cancel.Token,
        TaskContinuationOptions.None,


        TaskScheduler.FromCurrentSynchronizationContext());

        _tasks.Add(task, cancel);
    }

    private string? OnLoadBackground(CancellationToken cancellationToken)
    {
        if (cancellationToken.IsCancellationRequested)
            return "Canceled";



        Debug.WriteLine("Started...");

        // do work (passing in cancellationToken)
        Thread.Sleep(5000);

        Debug.WriteLine("Ended...");

        if (cancellationToken.IsCancellationRequested)
            return "Canceled";

        return $"{DateTime.Now}";
    }

    private void OnLoadUI(string? retrievedData)
    {
        Value = retrievedData ?? "NULL";
    }
}

What exactly exception are you getting? It should be OK to wait for a cancelled task. — arrowd, Jul 01 '23 at 12:25
What version of .NET are you targeting? I'm asking because your `Dispose()` method looks kinda gnarly (as you're using `Task.WaitAll()`) - if you're targeting .NET Core 3.0 or later, then consider using `IAsyncDisposable` instead. — Dai, Jul 01 '23 at 12:29
It's an ```AggregateException``` containing a ```TaskCanceledException``` for each task in the array. It seems to be related to the ```ContinueWith```. If I remove that, it behaves as expected :( — Vastar, Jul 01 '23 at 12:31
@Dai ```net6.0-windows``` I don't want an IAsyncDisposable, I specifically want the Dispose() to block. — Vastar, Jul 01 '23 at 12:32
@Vastar ...why do you want `Dispose()` to block? I cannot think of any good reason for making _potentially deadlocked_ `async` calls in `.Dispose` when you can use `.DisposeAsync` instead... — Dai, Jul 01 '23 at 12:55
@Dai, because then all the calling code that would call ViewModel.DisposeAsync() would need to be async — Vastar, Jul 01 '23 at 12:57
_"When the ViewModel is disposed, it should wait (block) on any current tasks that have not yet completed. I don't want to leave the Dispose method whilst there are still tasks running OnLoadBackground"_ - **This is a bad idea** and is contrary to how `IDisposable` and `IAsyncDisposable` work in .NET. Don't do this. — Dai, Jul 01 '23 at 12:57
@Dai, I'm unable to refactor the rest of the application (the code that would call DIspose()) to be async. — Vastar, Jul 01 '23 at 13:03
Why not simply catch the cancellation exception, i think it works in second code because the task library doesn't know your cancellation token and thus doesn't throw if cancelled — yassinMi, Jul 01 '23 at 13:43
@yassinMi catching the exception doesn't help, as it won't wait, it will just return straight away — Vastar, Jul 01 '23 at 13:45
Take a look at the `CancelableExecution` class in [this answer](https://stackoverflow.com/questions/6960520/when-to-dispose-cancellationtokensource/61681938#61681938 "When to dispose CancellationTokenSource?"). It might be exactly what you want. — Theodor Zoulias, Jul 01 '23 at 13:58
@yassinMi Requesting that the task be cancelled doesn't mean it's not still running. If I call Dispose() without waiting for the tasks to complete, then they could still be running afterwards. — Vastar, Jul 01 '23 at 14:07

Managing/clearing up cancelled tasks

0 Answers0