Proposal: Deprecate async void

[tenor]

I propose that the async void C# and VB language constructs be deprecated, for the following reasons.

1. Its purpose can be achieved via other means

The code below

async void AsyncVoidMethod()
{
    //Do some work
    await Task.Yield();
    //Do some more work
}

//Call method
AsyncVoidMethod();

is more or less the same as

async Task AsyncVoidHelper(Action action)
{
    Action outerAction = () => {
        SynchronizationContext.Current?.OperationStarted();
        action();
        SynchronizationContext.Current?.OperationCompleted();
    };

    await Task.Factory.StartNew(outerAction,
        CancellationToken.None,
        TaskCreationOptions.DenyChildAttach,
        TaskScheduler.Current);
}

async Task WorkMethod()
{
    //Do some work
    await Task.Yield();
    //Do some more work
}

//Call method
AsyncVoidHelper(async () => await WorkMethod());

2. async void is widely misunderstood
Many tutorials on the web describe async void as a fire and forget feature but that's not completely accurate. async void methods can still block the caller if execution doesn't run into an await statement, and even then, it can still block the caller if the await call runs synchronously.
Thus, async void methods have inconsistent return semantics. It can (a) return immediately -- while the rest of the method is processed by another task/thread, (b) block for a while and return -- while the rest of the method is processed by another task/thread, or (c) block indefinitely. The caller has no way of knowing what to expect.
In actuality, async void methods when factored with its weird exception handling behavior (discussed next) is really a fire and hope to forget feature.

3. async void can be disastrous
If a developer unwittingly adds an async keyword to a void method, making the method an async void method. That developer has just added a time bomb to their application. Any exception that gets thrown in that method can take down the entire process. That exception cannot be caught by callers to the async void method.
Also note that this applies to any exception thrown by methods the async void method calls.
The code will still compile and run fine after the void to async void change and is small enough to easily slip through code review.

4. async void is too advanced to be a language level feature
For anyone to use the async void feature safely, they have to be aware of its pitfalls and unique scenarios. It's the only language construct (that I'm aware of) that has a unique exception handling behavior, and that can stump even experienced developers.

Removal of this construct will make the language a little more safer. Asynchronous event handlers can still be written using a helper as illustrated in [1].

[HaloFour]

An analyzer, style guide or a warning wave might be a good way to discourage the use of said feature. Outright disabling it would be a breaking change. As well noted this form of async method was only added to make it easier to write asynchronous event handlers, particularly for UI apps. It's discouraged in all other scenarios.

[Joe4evr]

This requires a change in the CLR. The _only_ reason async void is even allowed is because the CLR demands anything subscribing to EventHandler/EventHandler<T> return void and event handlers are the places you want to take advantage of async the most (think about WinForms/WPF: users will complain your software is broken if the form hangs after they click a button).

There is actually a way around this limitation. but it requires writing your own event handling class and a bit of boilerplate code for each event. Backporting a pattern like this to existing libraries would also break just about every one of its consumers, but a brand new library could take advantage of it and provide async-safe events to its consumers.

[HaloFour]

@Joe4evr

This requires a change in the CLR.

The CLR imposes no restrictions at all on the types of delegates that may be used as an event. Neither does C# (although VB.NET does). It is simply a convention that events return void and accept a sender and an event arguments type.

public delegate int Adder(int x, int y);

public class Calculator {
    public event Adder Add;
}

var calc = new Calculator();
calc.Add += (x, y) => x + y;

But that's a bit of a tangent as it's not really the responsibility of the event producer to know that the subscriber wants to do something asynchronous with the notification.

[tenor]

Outright disabling it would be a breaking change.

I don't think any existing feature has ever been disabled in a future version of C# before, but this one might be worth it because IMO the drawbacks outweigh the benefits.

event handlers are the places you want to take advantage of async the most (think about WinForms/WPF: users will complain your software is broken if the form hangs after they click a button).

Perhaps a new task-like type similar to ValueTask can be introduced for event handlers. Event handlers can either return void or this new type. The state of this new type cannot be queried by a caller. The type will throw an InvalidOperationException when properties/methods like .IsCompleted or .Result are called.

[mburbea]

Well sure, but delegates in multicast form, rarely have an output only because there's no way to capture the actual results of the funcs in the invocation list.

Func<int, Task<int>> one = async x => { await Task.Yield(); Console.WriteLine("called one"); return 1; };
. Func<int, Task<int>> two = async x => { await Task.Yield(); Console.WriteLine("called two");  return 2; };
. Func<int, Task<int>> three = one + two;
. await three.Invoke(3)

Did it call both funcs? I have no idea.
But I digress. There's no reason that events couldn't have a return of Task (provided both returning tasks get evaluated and executed). The problem is backward compatibility. The event types were declared, and for the sake of backward compatibility it seemed like a reasonable thing, as otherwise you would need to have SomeEvent & SomeEventAsync.

[alrz]

I think this is safe to do,

Task DoSomethingAsync() { throw new Exception(); }

async void Interface.Foo()  {
    try { await DoSomethingAsync(); }
    catch { Console.WriteLine("report exception"); } // will print
}

void DoFoo(Interface obj) {
    try { obj.Foo(); }
    catch {  Console.WriteLine("caught"); } // won't print, of course
}

Comes in handy when you want to implement interface void-returning methods in asynchronously and also take advantage of await keyword; it is safe as long as your exception reporting is thread-safe.

[tenor]

@alrz Yes, but you know the consequence of removing that try block. It's something many developers may not be aware of.
Plus, you can always move all await code to a Task returning async method and call the async method from a void returning one.

[HaloFour]

@mburbea

The only way you'd be able to have the event return Task (or any awaitable) would be to have the compiler unwrap the multicast delegate into all of its target invocation, invoke them all individually and then join on all of their results. I doubt that will happen. Not to mention, as I stated before, why should the producer know nor care that the subscriber happens to want to do something asynchronous? Most are one-way and only happen to be synchronous as an implementation detail.

@alrz @tenor

An analyzer can identify unsafe use of async void and warn/error on it.

[tenor]

@mburbea

The only way you'd be able to have the event return Task (or any awaitable) would be to have the compiler unwrap the multicast delegate into all of its target invocation, invoke them all individually and then join on all of their results.

You can return a special purpose task-like Task that doesn't allow its state to be queried. Seems dumb but a lot safer than async void.

[HaloFour]

@tenor

Without querying its state how can it be awaited or its exception be handled? You'd be in the same boat as using async void, which isn't any different than async Task where you ignore the return type.

The only safe way to bridge event handlers with async methods is through a pattern similar to that described by @alrz. I'd be fine if the compiler or analyzers would shove people not-so-gently in that direction. Perhaps even a warning wave could make it illegal to have an async void method where any use of await isn't wrapped in a try/catch. Of course the risk you take there is that people will likely just ignore the exception anyway.

[benaadams]

async Task with an ignored return and an unhandled exception shows up in TaskScheduler.UnobservedTaskException
async void with an unhandled exception brings down the process

[tenor]

async Task with an ignored return and an unhandled exception shows up in TaskScheduler.UnobservedTaskException
async void with an unhandled exception brings down the process

@benaadams Yes!

Without querying its state how can it be awaited or its exception be handled?

@HaloFour Dang! Yeah the internal state needs to be queried. Can the completion semantics of the proposed Task type mean: The event handler task has successfully invoked all delegates attached to the event but is not keeping track of asynchronous operations performed by those delegates?

This way the Task is still fire and forget but can still be queried at some level.

[HaloFour]

@benaadams

My understanding is that if there is a SynchronizationContext then the unhandled exception would be posted to that context, so a WinForm app should handle it like it would any unhandled exception in a synchronous event handler. However, exactly how an unhandled exception is handled in an async void method is a matter of the BCL, not the compiler. The compiler just uses AsyncVoidMethodBuilder.SetException. If we don't like how it works we could argue that it be changed.

I'm definitely not arguing that using async void is a good idea, but I understand why it's there and I don't really see a good alternative way to bridge async methods and event handlers, particularly where the event publisher isn't aware. I think just tossing async void away is a non-starter just like any other breaking suggestion, regardless of how bad of an idea it might be.

@tenor

Queried by who? Without a continuation on that Task you're still expecting any exceptions to bubble up to some global handler. Failing that the best route is to kill the process.

[tenor]

Queried by who? Without a continuation on that Task you're still expecting any exceptions to bubble up to some global handler. Failing that the best route is to kill the process.

Yes, the behavior will be the same, however, there is an important difference.
With async void, you can easily unintentionally shoot yourself in the foot (Imagine another developer refactoring @alrz sample code and removing the try-block).
With a specialized Task, you're going out of your way to opt into this behavior.
Also, if you're calling methods that return the specialized Task, you're most likely working in a WPF, ASP.NET or WinForms application and those kinds of applications have safeguards to protect against exceptions in asynchronous handlers.

[HaloFour]

@tenor

You can't change the return type without breaking the signature and precluding the use of that method as an event handler. In other words, it completely defeats the point. Developers would then have to use additional boilerplate to bridge event handlers to async methods manually. At the end of the day they'd be writing a lot more code for zero benefit, as the behavior would still be the same.

Considering how obsessive the C# team has been about not introducing changes to how existing source builds, even to the point of not adding warnings to obviously errant code in more innocuous situations. They've never removed features from the compiler and rendered existing legal code illegal. The chances of them doing anything beyond a potential opt-in warning is unlikely. The fact that async void could be problematic was known before it was implemented and all of the literature explains when it should and shouldn't be used. I seriously don't think that you're introducing any new arguments here.

[mburbea]

I agree that you shouldn't care about your subscribers callbacks. That said, I dislike async void, but I think there are things I'd much rather deprecate first like operator true

[benaadams]

I'd much rather deprecate first like operator true

Wow, never knew about that; might start using it 😉

[tenor]

@HaloFour

I'm not introducing any new arguments. These are well known issues. However, a lot has changed since async void was introduced.

1. There's .NET Core now, which is still in development, so that ship hasn't (completely) sailed yet.
2. There's talk of arbitrary task-like Tasks.
3. There's a lot more community engagement in the direction of C# as a language.

This means there's a window of opportunity to fix a problematic feature. I understand the Microsoft mantra is to never break backwards compatibility but I think there is a path to fixing old warts without causing too much friction.

1. Implement a work around.
2. Introduce analyzers that detect the issue and inform developers to use said work around.
3. Warn developers extensively about dangers of using said feature. (C# 8 and 9 cycle)
4. Introduce a compiler warning (C# 11)
5. Introduce a compiler error (C# 15)

Yes, you can't change the return type without changing the method signature but maybe that's a good thing. It raises an important question: Is it beneficial for asynchronous event handlers to be distinctly identified, for example using a new EventHandlerAsync type? Just like async network operations are usually identified with an Async suffix?
Put another way, is it beneficial for callers to know if the call to an event handler will run synchronously or not?

[HaloFour]

@tenor

There's .NET Core now, which is still in development, so that ship hasn't (completely) sailed yet.

.NET Core was officially released in June. It's up to 1.0.1 now.

There's talk of arbitrary task-like Tasks.

Which still doesn't help if there's nothing awaiting those tasks. And it's unnecessary in the common cases. In a WinForms app if there is an unhandled exception in an async void event handler it's routed back to the SynchronizationContext which raises Application.ThreadException, just as if the exception was unhandled in a synchronous event handler.

Is it beneficial for asynchronous event handlers to be distinctly identified, for example using a new EventHandlerAsync type? Just like async network operations are usually identified with an Async suffix?

No, this doesn't make any sense. The event itself is not asynchronous. The fact that the consumer wants to do something asynchronous is a completely separate concern. There's no reason for the producer to know nor care.

[tenor]

@HaloFour

.NET Core was officially released in June. It's up to 1.0.1 now.

.NET Core is still at the stage where .NET 1.0 was when it was initially released. Is anyone seriously using .NET Core in a public facing environment in production? Did you see the security advisory from a few days ago?

Which still doesn't help if there's nothing awaiting those tasks.

It helps because it moves an unsafe language level feature to the framework.

No, this doesn't make any sense. The event itself is not asynchronous. The fact that the consumer wants to do something asynchronous is a completely separate concern. There's no reason for the producer to know nor care.

Not necessarily. What you're saying is correct for things like Winforms, where UI work is posted to the UI thread to process. The producer posts the work item and carries on.
Now imagine you're working on a high performance class library that notifies a bunch of consumers about an event. To prevent a consumer from blocking the notifier, you can post the notification on the threadpool and let a free thread handle it. If you are really concerned about being responsive to consumers, you can post the call to each consumer on the thread pool.
Now, if you know all your consumers are asynchronous in nature, you can have the notifying thread call each consumer one by one because you'd expect that each call will return almost immediately.
If a consumer still blocks the thread then that consumer is to blame because they violated the contract when they declared they were asynchronous.
In the previous case, the consumer cannot be blamed if they tie up your thread. They are free to do so.

The rundown of this discussion so far is:

1. Is this an issue? From all accounts, Yes.
2. How bad is it? IMO, really bad because an exception occurring in a third party library can take down your process.
3. Why do we have this feature? For asynchronous event handling, maybe some other lesser known scenarios.
4. Can we re-architect event handling so we don't have this issue anymore? <-- This is where the hand wringing starts.

[HaloFour]

.NET Core is still at the stage where .NET 1.0 was when it was initially released.

Officially supported? Yep.

Now imagine you're working on a high performance class library that notifies a bunch of consumers about an event. ... if you know all your consumers are asynchronous in nature ...

You certainly would not be designing such a beast using C# events with or without async void handlers.

The rundown of this discussion so far is:

1. Yes, of course.
2. I can do this right now in a third-party library by spinning up another thread and throwing on it. I wouldn't propose banning Thread.
3. Correct
4. Feel free to propose something additive, but I expect anything that involves deprecating/removing existing syntax to fly like a lead balloon, even if over several releases.

[DualBrain]

Breaking changes... slippery slope. Let's take the arguments mentioned above and apply them to "other areas"...

1. Its purpose can be achieved via other means

• goto
• unsafe
• (there are plenty more)

This is true of nearly every thing.

But let's discuss goto for a second... there is no argument that nearly every scenario (especially common ones) that there is an alternative way to accomplish the task. So this, by this argument, would suggest that goto be removed. Right? However, what about all of the people that did use it because, for them, it was a solution that worked better for them and possibly even the only solution to a problem that they were encountering. Just because it's not something that you experience regularly doesn't mean there aren't scenarios out there where using something you "dislike" isn't the "right tool for the task at hand".

So I'm going to call this one what it really is... "do it my way because I think my way is better". It's based on opinion, nothing more.

2. (feature) is widely misunderstood

• goto

I'll comment that goto is got to be one of the most misunderstood tools out there. It's evil, never use it is the mantra... however, it's still alive and it's still kicking... why? Short answer... it's very useful for some very specific scenarios; one of which is simply raw performance. Even if that wasn't the case, some people think differently than others... so goto fits them better.

3. (feature) can be disastrous

• goto

There is absolutely not argument from me that the use of goto can, if done incorrectly and non-judiciously, lead to disastrous results. The most common term for this result is "spaghetti code". No argument there. However, for the right circumstances... goto can be a lifesaver.

4. (feature) is too advanced to be a language level feature

• goto

Ummm... seriously. "too advanced"? What does that even mean? Isn't using a language like C# "somewhat advanced"... there's a lot of things you have to know in order to accomplish the most simple tasks. The barrier of entry is FAR greater than say... oh I don't know... GW-BASIC and PRINT "HELLO WORLD!". Writing the same thing in C# requires more lines of code and, more interestingly enough, figuring out which freakin' kind of project you should create. Many people will get lost at that point.

And speaking of goto... even though it's treated as the poster child of what not to do; the reality is that is HOW PROCESSORS WORK. Understanding of what is happening underneath/behind the higher abstraction of a language like C# is fundamental to understanding when/where/how goto would be of benefit. In other words... it's actually an ADVANCED topic that is periodically misused by people who don't understand what it's actually for.

Going back to the original feature in question... this is also somewhat true of async void. It's a very advanced topic; however, it's been built in such a way that, for the most part, you don't need to know the details. Much like a goto, you can use it more generically without understating what is going on. With async void existing for primarily the purpose of not having to rewrite WinForms; and why should anyone... it works. And... for the most part, WinForms events should be "fire-and-forget". There are other frameworks that try to setup a rules-based approach to events; but that is an alternative tool.

All of the above "arguments" are completely subjective. As such, none of them provide any reason to remove async void (IMO).

The other thing to keep in mind... once something has been added... people get creative. Many times these usage scenarios were never foreseen. These features can take on a life of their own. Removing them after that is, IMO, more dangerous because it becomes nearly impossible to understand all of these other scenarios.

And... at the end of the day... really... is it causing planes to fall from the skies?

Adding new features to the language is hard enough (nameof for example); let's not complicate it further by trying to guess (and ultimately piss off the target audience) how removing something is going to "help".

[sharwell]

💭 We use an analyzer to turn async void into a compiler error.

[jnm2]

There's no way I can do without this pattern for some view model properties:

public int BoundProperty
{
    get { return boundProperty; }
    set
    {
        if (boundProperty == value) return;
        boundProperty = value;
        OnBoundPropertyChanged();
    }
}

private async void OnBoundPropertyChanged()
{
    // ...
}

Not to mention the (rare) async void TrySetBoundProperty(int value) situation.
Exactly how would this be improved by banning async void? I'm using it very tastefully here and in a few other non-UI-related scenarios. In each case, it's not the caller's responsibility to know that something async has been triggered.

[alrz]

If you have to use async void like override async void this is totally safe to use:

override async void M() { try { ... } catch { ... } }

[jnm2]

@alrz No way I'm swallowing exceptions like that in production code. That's worse than crashing.

[alrz]

I didn't mean to just catch it. you should of course handle it, (log etc) and perhaps route it to UI thread, e.g. via dispatcher.

[Thaina]

I am agreeing with all reason the issuer posted. But I am thinking the opposite that we should drop Task at async but always convert async void to Task (and async anything to Task<anything>) automatically

Or maybe the opposite, we should drop async and let await be contextual whenever the return type is Task (like yield under IEnumerable)

[molinch]

What is the "right" way to fire and forget a Task?

In our app we did a helper:

        public static async void FireAndForget(this Task task)
        {
            try
            {
                await task.ConfigureAwait(false);
            }
            catch (Exception ex)
            {
                ErrorDialog.Show(ex); // helper class to show it to user
            }
        }

Such a change would break our code.

[benaadams]

@molinch something like?

public static void FireAndForget(this Task task)
{
    task.ContinueWith(
        t => 
        { 
            ErrorDialog.Show(t.Exception);  // helper class to show it to user
        }, 
        TaskContinuationOptions.OnlyOnFaulted);
    }
}

[molinch]

Will ContinueWith behave normally with Promises tasks?

Forget this, I just did a couple of checks and yes it behaves normally.
Thanks @benaadams !

[molinch]

@benaadams We used your proposal, however in the end, we got an issue in our app, where a TaskCanceledException raised by a Promises task was never observed by the continuation.
It was finally needed to await and have an async void method. Thus the exception was observed in the catch block.

I can't explain why it happened but IMO that's a serious blocker for deprecating async void.

[benaadams]

Cancelled isn't faulted; change TaskContinuationOptions.OnlyOnFaulted to TaskContinuationOptions.NotOnRanToCompletion to pickup Cancelled as well as Faulted, but ignore regularly completed.

[molinch]

Thanks again !
So the TaskCanceledException/OperationCanceledException is automatically handled differently by the TPL? It is weird that this case isn't written on MSDN. It basically says that OnFaulted is triggered for any exception, which isn't true in this case.

Do you think that the special behaviour for OperationCanceledException is handled here? http://referencesource.microsoft.com/#mscorlib/system/threading/Tasks/Task.cs,2906

[jnm2]

The fact that OperationCanceledException translates to cancellation rather than fault is an intrinsic part of the TPL API contract. If you need to work around this, you'll have to use await (or .GetAwaiter().GetResult() rather than TPL's *Faulted and Task.Exception.

[molinch]

Thanks @jnm2. Is there a chance to see which code, using sourceof.net, is responsible for it? I would be interested to understand it.

[jnm2]

This is what I'd do:

public static void FireAndForget(this Task task)
{
    task.ContinueWith(t =>
    {
        try
        {
            t.GetAwaiter().GetResult();
        }
        catch (Exception ex)
        {
            ErrorDialog.Show(ex);  // helper class to show it to user
        }
    }, TaskContinuationOptions.NotOnRanToCompletion);
}

[benaadams]

@molinch relevant TaskContinuationOptions for the 3 completion states

NotOnRanToCompletion = 0x10000,
NotOnFaulted = 0x20000,
NotOnCanceled = 0x40000,
OnlyOnRanToCompletion = NotOnFaulted | NotOnCanceled,
OnlyOnFaulted = NotOnRanToCompletion | NotOnCanceled,
OnlyOnCanceled = NotOnRanToCompletion | NotOnFaulted,

[benaadams]

@jnm2 that's probably a good idea, then will unwrap aggregate exceptions

[jnm2]

@molinch Looks like you've already found the code implementing the TPL contract. Here's the code for the await model: http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,fac5be2731353aa8,references

[molinch]

Awesome, thanks both for your kind help

[jnm2]

@molinch Very specifically http://referencesource.microsoft.com/#mscorlib/system/runtime/compilerservices/TaskAwaiter.cs,172

[jnm2]

@jnm2 that's probably a good idea, then will unwrap aggregate exceptions

When I get a spare moment, I'll be writing some final tests to get code merged to NUnit that moves the framework entirely to the awaitable contract away from the TPL contract because of things like these that have come up.

[gafter]

We added this construct to the languages intentionally, fully aware of its limitations. I doubt we'd consider changing that now.

[gafter]

If you would like to resurrect this discussion, please take it to csharplang.