1

I'm confused why the output of these 2 programs differs:

private async void button1_Click(object sender, EventArgs e)
{
    for (int i = 0; i < 33; i++)
    {
        await LongProcess();
    }
}

private async Task LongProcess()
{
    await Task.Delay(1000);
    progressBar1.Value += 3;
}

and

private async void button1_Click(object sender, EventArgs e)
{
    for (int i = 0; i < 33; i++)
    {
        await Task.Run(() => LongProcess());
    }
}

private async void LongProcess()
{
    await Task.Delay(1000);
    progressBar1.Value += 3;
}

I realize the first example returning Task is more correct, but I don't understand why wrapping the void function in a Task.Run doesn't produce the same output? The first function does what I expect, updates the progress bar every 1 second. The second code attempts to update the progress bar all at once, causing problems attempting to update the same UI element from multiple threads.

My assumption was since the buttonClick method awaits the long process to complete, both sets of code should not allow the progressBar1 update to happen until the previous process has completed. Why does the second set of code allow it to happen all at once?

Kevin DiTraglia
  • 25,746
  • 19
  • 92
  • 138

2 Answers2

6

This isn't doing what you think it is:

await Task.Run(() => LongProcess());

The code is awaiting Task.Run(), but nothing within that task is awaiting LongProcess(). So Task.Run() returns immediately in this case.

This is actually an interesting illustration of a failure to be "async all the way down", because the inline function is essentially hiding the fact that it's async. And in fact the compiler should be warning you that nothing is awaiting LongProcess() and that it would return immediately.

Contrast it with this:

await Task.Run(async () => await LongProcess());

Edit: I just noticed why the compiler probably isn't warning you. Because of this:

async void

Never, ever, ever do this :) (Well, ok, there's one valid reason to do this. And I'm sure it haunts the C# team to this day that they had to support it just for that one reason. But unless you encounter that one reason, don't do it.)

Always return a Task for async methods so that the method can be awaited.

David
  • 208,112
  • 36
  • 198
  • 279
  • 1
    You wouldn't get any warning anyway because if `LongProcess` returned a `Task`, the `Task.Run` overload used would then be `Task Task.Run(Func)` which *unwraps* the `Task` instead of returning `Task`. – Lucas Trzesniewski Jun 01 '15 at 18:13
  • @LucasTrzesniewski: Are you sure? I'm testing this now in VS2013 where my `LongProcess` is a simple `async Task Foo() { await Task.Yield(); }` and when I call it in the same way as the OP's code I get a warning in VS. – David Jun 01 '15 at 18:15
  • `await Task.Run(() => Foo());` yields no warning for me, it's strange you get one. – Lucas Trzesniewski Jun 01 '15 at 18:20
  • @LucasTrzesniewski: Interesting, I just found the difference. `await Task.Run(() => { Foo(); });` yields the warning. I forgot that I'd wrapped it in `{}` when I added the `await` and didn't remove them when reverting for this test. – David Jun 01 '15 at 18:22
  • 1
    Oh yeah, that's a typical mistake, I [wrote a whole answer](http://stackoverflow.com/a/27530043/3764814) on that one once ;-) – Lucas Trzesniewski Jun 01 '15 at 18:57
1

In the first program, LongProcess returns a Task, and Task.Run is wrapping it -- basically just launching it in the default scheduler rather than whatever context you're currently on.

You'll notice that Task.Run has overloads specifically to do this wrapping, and is not returning a Task<Task>.

In the second program, LongProcess is an async void method, which means Task.Run has nothing to wrap and will complete more or less immediately, and before the work is guaranteed to be done.

Cory Nelson
  • 29,236
  • 5
  • 72
  • 110