Search code examples
c#taskcancellation

Wait for Task to cancel waits forever


I have a Form in which I start a Task to load the content. If the User clicks Cancel this Task needs to be cancelled of course. But it seems I'm doing something wrong. The form never closes and keeps waiting for the task:

public partial class Designer : Form
{
    private CancellationTokenSource _cancellationTokenSource;
    private Task _loadTask;

    private async void Designer_Shown(object sender, EventArgs e)
    {
        _cancellationTokenSource = new CancellationTokenSource();
        try
        {
            _loadTask= Workbench.Instance.CurrentPackage.LoadObjects(_cancellationTokenSource.Token);
            await _loadTask;
        }
        catch (Exception ex)
        {
            Debug.Print(ex.ToString());
        }
    }

    private void btnCancel_Click(object sender, EventArgs e)
    {
        _cancellationTokenSource.Cancel();
        _loadTask.Wait(); //Waits forever
        this.DialogResult = DialogResult.Cancel;
        this.Close();
    }
}

Where's my fault?

Edit The Code Of LoadObjects()

public Task LoadObjects(CancellationToken cancelToken)
{
    return Task.Run(() =>
    {
        LoadParameters(cancelToken);
        LoadConditionChecks(cancelToken);
        LoadConditonRules(cancelToken);
        LoadOperations(cancelToken);
    }, cancelToken);
}

I pass the Token the Sub-Methods since the loops are actually there...


Solution

  • You are hitting a deadlock on the UI thread by awaiting it and calling Task.Wait(). Avoid Task.Wait at all costs.

    Service the result of the cancellation in an asynchronous continuation, like this:

    private async void btnCancel_Click(object sender, EventArgs e)
    {
        _cancellationTokenSource.Cancel();
        await _loadTask;
        this.DialogResult = DialogResult.Cancel;
        this.Close();
    }
    

    This is the only time async void is acceptable.

    My favourite async code guy Stephen Cleary provides a brilliant blog post explaining why you should avoid Task.Wait and Task.Result as blocking mechanisms - Don't Block on Async Code


    For what its worth, when I do cancellation, I never wait for the task to finish. I respond immediately to the cancellation and let the task finish in the background. This provides a responsive UI experience for the user. If I need to get a result from a cancelled task I will put some UI work in to communicate 'waiting for end of operation' after the user clicks cancel.