

Beware of “async void” in C# - theburningmonk
http://theburningmonk.com/2012/10/c-beware-of-async-void-in-your-code/

======
mhomde
To make it perfectly clear , "async void" should be replaced with "async Task"

The async stuff is interesting in that akin to generics it has an almost
cancerous effect on your code. When you start using them they tends to spread
to large parts of your code base if you're not careful.

It's awesome that C# now has such good support for parallelism and
asynchronicity... but I remain in the opinion that a lot of this stuff is
grafted on to the language and at some point we need to start over from
scratch. Already more and more of the code is taken up by crud that should be
abstracted away.

~~~
saosebastiao
I've been pleasantly surprised by the async workflows in F#. Behind the scenes
it is one big ass monadic construct that has been grafted onto the language,
but it doesn't feel that way at all when you use it...it is incredibly elegant
and easy to read/write and understand exactly what is happening in terms of
control flow. I think it is perfectly possible for C# to improve it without
starting over from scratch, simply for the fact that F# has already done it.

EDIT: See this link for more info on how F# has solved this problem, as well
as others. [http://tomasp.net/blog/csharp-async-
gotchas.aspx/](http://tomasp.net/blog/csharp-async-gotchas.aspx/)

~~~
mhomde
Yeah, but the problem with C# is not that it isn't working, and it is pretty
legible and easy to understand.

The problem is the syntax and lack of some abstractions. F# syntax is probably
more suited for a certain style of programming. You could try to rework C#
without concern for breaking current behavior and making it more cohesive and
consistent... but at that point it won't be C# I think.

What I'd really like though is a switch from an explicit parallelism/multi-
thread/asynchronous model to an implicit declarative model with optional
explicitness.

Let me explain what I mean. In many cases how code is executed should follow
from the constraints of the computer/the types/side effects etc. If a
programming language was structured so it was more about annotating those
constraints then explicitly saying what to do, then the compiler and runtime
should be able to optimize for us. Less risk of bugs and greater potential for
advanced optimization.

Also a software program might need to run different on a computer with many
cores and a single core, or one with a RAM drive rather than a HD, or whatever
hardware constraints we'll have in the future. Implicitness in that way is
much more agile.

Software built like this could even adapt and learn on it's own. Imagine you
have a loop or something that performs an operation on a number of items. That
loop could be parallelized, but should it? Right now that's a tricky question,
it depends on the number of items and how demanding the operation. How much
should it be parallelized? 2 operations simultanenously? 4? 24? Another tricky
question. A smart program could profile itself continuously, see what typical
input is and adapt itself accordingly.

I know many are afraid of a "black box" a 'la ORM's but I think at some point
we have to let go to a certain extent and move to a higher level of
abstraction, in the same way we went from assembler to c. Controlling specific
cases explicitly when needed should be possible of course

~~~
solarexplorer
What you describe really sounds like functional programming.

~~~
mhomde
Yes, it's probably closer to functional programming than OOP, but I feel that
strict functional programming doesn't adapt well to some use-cases, and there
really needs to be a modular model involved as well. I think a new mainstream
language would be best to be a hybrid of some sort.

------
JonathonW
Async void methods are for asynchronous event handlers. [1] That's an
unfortunate consequence of the way event handlers work in C# (event handlers
that return a type don't work well in the language).

Any other use of async void is incorrect; if you see an async void method in
any other context, replace it with async Task. (Would be nice if calling an
async void method directly threw a compiler warning to make this completely
clear.)

[1] [https://msdn.microsoft.com/en-
us/magazine/jj991977.aspx?f=25...](https://msdn.microsoft.com/en-
us/magazine/jj991977.aspx?f=255&MSPPError=-2147217396)

~~~
pionar
"Would be nice if calling an async void method directly threw a compiler
warning to make this completely clear."

With VS 2015 and Roslyn, you can easily achieve this. In fact, there's already
sample code to do this in the roslyn samples project[0].

[0][https://github.com/dotnet/roslyn/tree/master/src/Samples/CSh...](https://github.com/dotnet/roslyn/tree/master/src/Samples/CSharp/AsyncPackage)

~~~
JonathonW
The ability to write my own analyzers shouldn't be a substitute for a well-
curated set of code analysis checks out of the box, though. I want to spend my
time building my product, not building the tooling so I can build my product.

(Note that I haven't had the opportunity to play with VS2015 yet; for all I
know, these checks from the Roslyn samples might already be provided with the
IDE.)

~~~
Locke1689
Analyzers are available as NuGet packages, so it should be a trivial process
to install one of these in your project.

We're definitely going to start shipping some high-value analyzers in the box
(probably including the async-void one).

------
kogir
The article is not strictly correct. A more detailed explanation can be found
here: [https://msdn.microsoft.com/en-
us/magazine/jj991977.aspx](https://msdn.microsoft.com/en-
us/magazine/jj991977.aspx)

Exceptions actually bubble up to the active SynchronizationContext, and won't
necessarily kill your process. In ASP.Net, aborting a request on an error is
often desired behavior.

Still generally good advice though. Issues arising from accidental async void
use are often very tricky to track down, and in my own code I only use it for
fire-and-forget-best-effort event handling and run them all in a
SynchronizationContext that captures and logs the exceptions.

~~~
nigelsampson
It looks like the article was written in 2012 where the crashing behaviour was
normal in Windows 8.0, this changed in Windows 8.1

------
jongalloway2
For completeness, isn't there a third option of ensuring you're catching and
handling all exceptions in an async void handler?

    
    
       private static async void OnTimerFired(object sender, ElapsedEventArgs args)
    
       {
    	Exception exception = null;
    	try
    	{
    		await Task.Delay(1000);
    
    		// this is NOT going to terminate your process!
    		throw new Exception();
    	}
    	catch (Exception ex)
    	{
    	  exception = ex;
    	}
    
    	if (exception != null)
    	{
    	  ...
    	}
      }
    

And in C#6 you can await in catch or finally blocks, so this gets even
simpler.

------
typedef_struct
You should really just use Rx.NET
([http://www.introtorx.com/content/v1.0.10621.0/01_WhyRx.html](http://www.introtorx.com/content/v1.0.10621.0/01_WhyRx.html))
if you want async even handling

------
zamalek
To be a bit facetious: remember that an unhandled exception in your main
program thread will also crash your program.

The article holds a very valuable lesson, though: just because you don't see
"Task" doesn't mean that there aren't threads involved. In this way `async
void` is _deceptive_ but is certainly not something that is broken or
something that you should never use. There is actually a case where an
exception in one of these methods will not bring your process down:

    
    
        async void button1_Click(object sender, EventArgs e) {
            await Task.Delay(1000);
            throw new Exception("No crash here!");
        }
    

Why? A lesser known artifact from the .Net 2.0 days. In an unbelievable feat
of foresight Microsoft wrote a class called SynchronizationContext[1]. It's a
TLS object that defines a way to invoke methods on a well-known target. In the
case of the one provided by System.Windows.Forms (which is active in the UI
thread) it will dispatch callbacks back onto the main thread. Essentially what
is happening with my above code is:

    
    
        void button1_Click(object sender, EventArgs e) {
            var sc = SynchronizationContext.Current;
            Task.Delay(1000).GetAwaiter().OnCompleted(() => sc.Send(_ => {
                // Back on the UI thread, thanks SynchronizationContext!
                throw new Exception("No crash here!");
            }, null));
        }
    

The WindowsSynchronizationContext object (which is what
SynchronizationContext.Current will point to on a UI thread) simply has a
reference to the dispatcher and calls something similar to Control.Invoke().
This means that the code is re-routed back to the UI thread and because it
occurs there you get the standard Windows.Forms error dialog with the option
to "Quit" or "Continue" \- ultimately preventing a crash. TPL leverages
SynchronizationContext which is why this all works. Indeed, the very final
example in the article (it looks like Windows.Timer) _won 't_ crash with
`async void`.

As far as I remember, ASP.Net provides a SynchronizationContext for requests
but I am unsure as to how it deals with exceptions (if it does at all). It's
implementation specific.

Finally SynchronizationContext can be implemented by you allowing you to also
deal with these exceptions in a clean/proper way[2]. Please don't ever do
anything along the lines of `SwallowException`. Keep in mind that not
everything takes advantage of SynchronizationContext (e.g. Threading.Timer) so
it's definitely not a catch-all.

[1]: [https://msdn.microsoft.com/en-
us/library/system.threading.sy...](https://msdn.microsoft.com/en-
us/library/system.threading.synchronizationcontext.aspx) [2]:
[https://gist.github.com/jcdickinson/6aa4a09b60bd2469baf5](https://gist.github.com/jcdickinson/6aa4a09b60bd2469baf5)

~~~
mhomde
On a related note, I really dislike the dispatcher paradigm, especially with
the inclusion of async-await.

For good performance of IO async calls you usually want to call
.ConfigureWait(false), otherwise it will wait for the UI context to switch
back to, you might even risk thread starvation.

But now each async call is a potential switch out of the UI thread and certain
crasch down the road, meaning you have to put Dispatcher.HasThreadAccess
checks on all your UI methods. It just feels... clunky and unnecessarily
explicit.

In my opinion UI controls should be able to handle switching to UI context
themselves and not crash the whole program, or if nothing else you should be
able to annotate methods as UI threads rather than the clunky HasThreadAccess
syntax.

There's also these annoying "barriers" that you can't do certain UI stuff on
background threads without some ugly hacks, like creating and working with
images.

The Dispatcher is cool for being able to prioritize UI operations though

~~~
zamalek
> the dispatcher paradigm is one of the thing I dislike the most about coding
> UI stuff in .Net

It inherits it from Windows. As far as I know, most GUIs have a dispatcher
because <technical reasons>. I definitely think that there could have been a
better way to solve the problem (method interception), however, I think that
TPL does add value.

In your case you cited intense IO as one reason to call
`.ConfigureAwait(false).` The thing is: you have made a conscious decision to
not return to the UI thread and therefore are more likely to tread carefully
when writing the remainder of the method. `.ConfigureAwait(false)` declares
"dragons be here."

Previously someone who didn't know better would update the UI from the wrong
thread, weird things happen and they don't know why.

Here's something to make things simpler for you:
[https://gist.github.com/jcdickinson/f875229e671710cf1b34](https://gist.github.com/jcdickinson/f875229e671710cf1b34)

~~~
asveikau
> It inherits it from Windows. As far as I know, most GUIs have a dispatcher
> because <technical reasons>.

This is a kind of lame elision. The classical Windows UI threading model is
that every window has an owning thread [the thread it was created on], and
that owning thread is supposed to drain its message queue periodically. You
can send messages to another thread's object [either in blocking, via
SendMessage, or enqueue without blocking for result, via PostMessage].

------
scrabble
This is a rough one, because while Microsoft's own docs tell you not to use
async void, all of their demo code that I've seen does.

Granted, it's demo code and not production code, but less experienced
developers do not always pick up on that.

~~~
douche
There are often some startlingly bad practices in some of the Microsoft sample
code. I've worked with an SDK where the majority of the example projects would
crash out before illustrating the point of the example, because event
callbacks that were hit by the code executed in the sample were just stubbed
in with

    
    
        throw new NotImplementedException();

