Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

How to better handle disposed controls when using async/await

Consider this code that runs on the UI thread:

dividends = await Database.GetDividends();
if (IsDisposed)
    return;
//Do expensive UI work here
earnings = await Database.GetEarnings();
if (IsDisposed)
    return;
//Do expensive UI work here
//etc...

Note that every time I await I also check IsDisposed. It's necessary because say I await on a long running Task. Meanwhile the user closes the form before it completes. The Task will finish and run a continuation that attempts to access controls on a disposed form. An exception occurs.

Is there a better way to handle this or simplify this pattern? I use await liberally in UI code and it's both ugly to check for IsDisposed every time and error prone if I forget.

EDIT:

There are a few proposed solutions that don't fit the bill because they change functionality.

  • Prevent form closing until background tasks complete

This will frustrate the users. And it also still allows potentially expensive GUI work to occur that is a waste of time, hurts performance and is no longer relevant. In the case where I'm almost always doing background work this could prevent the form close for a very long time.

  • Hide the form and close it once all tasks complete

This has all the problems of preventing the form close except doesn't frustrate users. The continuations that do expensive GUI work will still run. It also adds complexity of tracking when all tasks complete and then closing the form if it's hidden.

  • Use a CancellationTokenSource to cancel all tasks when the form is closing

This doesn't even address the problem. In fact, I already do this (no point in wasting background resources either). This isn't a solution because I still need to check IsDisposed due to an implicit race condition. The below code demonstrates the race condition.

public partial class NotMainForm : Form
{
    private readonly CancellationTokenSource tokenSource = new CancellationTokenSource();

    public NotMainForm()
    {
        InitializeComponent();
        FormClosing += (sender, args) => tokenSource.Cancel();
        Load += NotMainForm_Load;
        Shown += (sender, args) => Close();
    }

    async void NotMainForm_Load(object sender, EventArgs e)
    {
        await DoStuff();
    }

    private async Task DoStuff()
    {
        try
        {
            await Task.Run(() => SimulateBackgroundWork(tokenSource.Token), tokenSource.Token);
        }
        catch (TaskCanceledException)
        {
            return;
        }
        catch (OperationCanceledException)
        {
            return;
        }
        if (IsDisposed)
            throw new InvalidOperationException();
    }

    private void SimulateBackgroundWork(CancellationToken token)
    {
        Thread.Sleep(1);
        token.ThrowIfCancellationRequested();
    }
}

The race condition happens when the task has already completed, the form has closed, and the continuation still runs. You will see InvalidOperationException being thrown occasionally. Cancelling the task is good practice, sure, but it doesn't alleviate me from having to check IsDisposed.

CLARIFICATION

The original code example is exactly what I want in terms of functionality. It's just an ugly pattern and doing "await background work then update GUI" is a quite common use case. Technically speaking I just want the continuation to not run at all if the form is disposed. The example code does just that but not elegantly and is error prone (if I forget to check IsDisposed on every single await I'm introducing a bug). Ideally I want to write a wrapper, extension method, etc. that could encapsulate this basic design. But I can't think of a way to do this.

Also, I guess I must state performance is a first-class consideration. Throwing an exception, for example, is very expensive for reasons I won't get into. So I also don't want to just try catch ObjectDisposedException whenever I do an await. Even uglier code and also hurts performance. It seems like just doing an IsDisposed check every single time is the best solution but I wish there was a better way.

EDIT #2

Regarding performance - yes it is all relative. I understand the vast majority of developers don't care about the cost of throwing exceptions. The true cost of throwing an exception is off-subject. There is plenty of information available on this elsewhere. Suffice to say it's many orders of magnitude more expensive than the if (IsDisposed) check. For me, the cost of needlessly throwing exceptions is unacceptable. I say needless in this case because I already have a solution that doesn't throw exceptions. Again, letting a continuation throw an ObjectDisposedException is not an acceptable solution and exactly what I'm trying to avoid.

like image 949
Zer0 Avatar asked Jun 25 '15 23:06

Zer0


People also ask

Why you shouldn't use async void?

Async void methods can wreak havoc if the caller isn't expecting them to be async. When the return type is Task, the caller knows it's dealing with a future operation; when the return type is void, the caller might assume the method is complete by the time it returns.

Does async await stop execution?

The await expression causes async function execution to pause until a promise is settled (that is, fulfilled or rejected), and to resume execution of the async function after fulfillment.

Does await throw AggregateException?

When using await, it's going to unwrap the first exception and return it, that's why we don't hit the catch (AggregateException e) line. But if we use something like the below code sample, we catch the AggregateException , note that it's not a good idea since we're blocking the running thread.

Does async await improve performance?

The main benefits of asynchronous programming using async / await include the following: Increase the performance and responsiveness of your application, particularly when you have long-running operations that do not require to block the execution.


1 Answers

It should be pretty straightforward to have a CancellationTokenSource owned by your form, and have the form call Cancel when it is closed.

Then your async methods can observe the CancellationToken.

like image 55
Stephen Cleary Avatar answered Sep 26 '22 09:09

Stephen Cleary