Managing/clearing up cancelled tasks

80 views Asked by At

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";
    }
}
0

There are 0 answers