Asynchronous searching for files with CancellationToken

Question

I am trying to build a function that searches a directory for files and adds them to an ObservableCollection.

This task should run asynchronously so that the UI remains responsive and stops searching at any time when the method is executed again. So that the ObservableCollection is cleared and the search starts again.

My problem is that I don't know how and when exactly to cancel the CancellationTokenSource in the same method when the task is still running.
Also, I don't know exactly how to use Async and Await on the function since it is split between three methods.

Right now it looks like this:

Search Function:

CancellationTokenSource _tokenSource;

public async void SearchAsync()
{
    Files.Clear();
    FileCount = 0;

    // Cancels Task if its running
    if(_tokenSource != null)
    {
        _tokenSource.Cancel(); (throws Error right here. Task already disposed)
    }

    // Create new Cancellation Token
    _tokenSource = new CancellationTokenSource();
    CancellationToken token = _tokenSource.Token;

    try
    {
        //Starts Task
        await Task.Run(() => FindFiles(token));
    }
    catch (OperationCanceledException ex)
    {
        //When Canceled
        MessageBox.Show("Test", "Test");
    }
    finally
    {
        //When finished
        _tokenSource.Dispose();
    }
}

Find Files:

private async Task FindFiles(CancellationToken token)
{
    foreach (string filePath in Directory.EnumerateFiles(MainPath, "*.*", searchOption))
    {
        await AddFileAsync(filePath);
        FileCount++;

        if (token.IsCancellationRequested)
        {
            Files.Clear();
            FileCount = 0;
            token.ThrowIfCancellationRequested();
        }
    }
}

Add Files to ObservableCollection:

I put this into a own method because it's called multiple times in LoadFiles(). I just simplified it.

public ObservableCollection<FileModel> Files { get; set; }

private async Task AddFileAsync(string filePath)
{
    await Application.Current.Dispatcher.BeginInvoke(DispatcherPriority.Background, new Action(() =>
    {
        FileModel file = new()
        {
            FileName = Path.GetFileName(filePath),
            FilePath = filePath
        };
        Files.Add(file);
    }));
}

Answer 1

There are several issues with your code:

1. You dispose CancellationTokenSource, but with next call to SearchAsync() you call .Cancel() on the disposed CTS. This call fails of course.
2. You use async void in FindFiles(). Your await Task.Run(() => FindFiles(token)); exits immediatly because of it. Don't ever use async void unless absolutely necessary.
3. You might have a race condition, depending on whether your task scheduler is mult-threaded or not.

Have a look at this implementation:

public sealed class Searcher
{
    // Initialized here to avoid null checks.
    private CancellationTokenSource _tokenSource = new CancellationTokenSource();

    public async Task SearchAsync()
    {
        var newTokenSource = new CancellationTokenSource();

        var oldTokenSource = Interlocked.Exchange(ref _tokenSource, newTokenSource);

        if (!oldTokenSource.IsCancellationRequested)
        {
            oldTokenSource.Cancel();
        }

        try
        {
            //Starts Task
            await Task.Run(() => FindFiles(newTokenSource.Token));
        }
        catch (OperationCanceledException)
        {
            //When Canceled
        }
    }

    private async Task FindFiles(CancellationToken token)
    {
        await Task.Delay(1000, token);
    }
}

So now each call to SearchAsync() keeps track of two CTS: current and the previous one. This solves problem 1.

I've turned async void FindFiles to async Task FindFiles which solves problem 2. I've also changed the signature of SearchAsync() to async Task so that the caller can await it. Although that depends on how the function is used and is not necessary.

Finally I used a Interlocked.Exchange to solve problem 3. This call is atomic. An alternative is to use a lock.

I've removed the CTS disposing code, because it is not that easy. The main problem is that the .Dispose() should be called when you are done with CTS. And so either when the task is cancelled or when it finishes. The problem is that you cannot easily verify whether CTS was disposed (it is in unusable state afterwards). Or maybe I don't know how to do that properly. Disposing CTS is weird and hard, read this: When to dispose CancellationTokenSource? Most of the answers there claim that if the CTS is not linked, then the GC will collect and dispose it properly at some point.

One way is to wrap CTS with a custom class that tracks whether this CTS was disposed or not. And probably calls to .Cancel() and .Dispose() are wrapped with a lock. But then everything becomes complicated. An alternative is to catch ObjectDisposedException whenever you access it, but I'm not exactly sure if that is correct.

On the other hand, for this simple use case I think you don't have to worry about disposing it. Let the GC collect it and take care of that. Neither CTS nor tokens leak outside, and if you remember to keep it that wait it should be fine. I would do that based on this note:

Note

Always call Dispose before you release your last reference to the CancellationTokenSource. Otherwise, the resources it is using will not be freed until the garbage collector calls the CancellationTokenSource object's Finalize method.

which to me it sounds like it is fine to not dispose it, if immediate disposal is not absolutely needed. And that's not the case IMO.