Koji
Koji

Reputation: 589

Trying to Understand Task.Run + async + Wait()/Result

I'm trying to understand how Task.Run + Wait() + async + await work.
I have read this page: Understanding the use of Task.Run + Wait() + async + await used in one line but don't understand it quite well.

In my code, I receive events from Microsoft EventHub and process them using a class that implements IEventProcessor. I call DoAnotherWork() method, which is an async method, in ConvertToEntity(), which is a sync method. Since the method is async, I use Task.Run() and async to delegate. (i.e. Task.Run(async () => entities = await DoAnotherWork(entities)).Wait())
The code has been working for a while but now my team member removed Task.Run() and changed it to DoAnotherWork(entities).Result;. I'm not sure this won't cause deadlocks.
I haven't asked him why he changed it but this change got me thinking "Is my code fine? Why?".

My questions are:
* What is the difference between the two?
* Which code is appropriate and/or safe (= won't cause a deadlock)?
* In what situation is a deadlock caused if it is?
* Why Task.Run() resolve deadlock I had? (see below for detail)

Note: I'm using .NET Core 3.1.

Why I Use Task.Run()
My team had deadlock issues a few times when we used AbcAsync().Result or .Wait() (the method was called in a NET Core Web API methods and deadlocks occurred mostly when we ran unit tests that execute the method), so we've used Task.Run(async () => await AbcAsync()).Wait()/Result and we haven't seen any deadlock issues since then.
However, this page: https://medium.com/rubrikkgroup/understanding-async-avoiding-deadlocks-e41f8f2c6f5d says that the delagation will cause a deadloock in certain conditions.

public class EventProcessor : IEventProcessor
{
    public async Task ProcessEventsAsync(PartitionContext context, IEnumerable<EventData> messages) 
    {
        ...
        var result = await eventHandler.ProcessAsync(messages);
        ...
    }
}

public Task async ProcessAsync(IEnumerable<EventData> messages)
{
    ...
    var entities = ConvertToEntity(messages);
    ...
}

public List<Entity> ConvertToEntity(IEnumerable<EventData> messages)
{
    var serializedMessages = Serialize(messages);
    var entities = autoMapper.Map<Entity[]>(serializedMessages);

    // Task.Run(async () => entities = await DoAnotherWork(entities)).Wait(); // before change
    entities = DoAnotherWork(entities).Result; // after change

    return entities;
}    

public Task async Entity[] DoAnotherWork(Entity[] entities)
{
    // Do stuff async
    await DoMoreStuff(entities)...
}

Upvotes: 1

Views: 1747

Answers (1)

Stephen Cleary
Stephen Cleary

Reputation: 456407

What is the difference between the two?

Task.Run starts running the delegate on a thread pool thread; invoking the method directly starts running the delegate on the current thread.

When learning async, it's helpful to separate out everything so you can see exactly what's going on:

entities = DoAnotherWork(entities).Result;

is equivalent to:

var entitiesTask = DoAnotherWork(entities);
entities = entitiesTask.Result;

and this code:

Task.Run(async () => entities = await DoAnotherWork(entities)).Wait();

is equivalent to:

async Task LambdaAsMethod()
{
  entities = await DoAnotherWork(entities);
}
var runTask = Task.Run(LambdaAsMethod);
runTask.Wait();

Which code is appropriate and/or safe (= won't cause a deadlock)?

You should avoid Task.Run in an ASP.NET environment because it will interfere with the ASP.NET handling of the thread pool and force a thread switch when none is necessary.

In what situation is a deadlock caused if it is?

The common deadlock scenario requires two things:

  1. Code that blocks on asynchronous code instead of properly using await.
  2. A context that enforces synchronization (i.e., only allows one block of code "in" the context at a time).

The best solution is to remove the first condition; in other words, use "async all the way". To apply that here, the best resolution is to remove the blocking completely:

public Task async ProcessAsync(IEnumerable<EventData> messages)
{
    ...
    var entities = await ConvertToEntityAsync(messages);
    ...
}

public async Task<List<Entity>> ConvertToEntityAsync(IEnumerable<EventData> messages)
{
    var serializedMessages = Serialize(messages);
    var entities = autoMapper.Map<Entity[]>(serializedMessages);

    entities = await DoAnotherWork(entities);

    return entities;
}

Why Task.Run() resolve deadlock I had? (see below for detail)

.NET Core does not have a "context" at all, so it uses the thread pool context. Since .NET Core doesn't have a context, it removes the second condition for the deadlock, and the deadlock will not occur. If you're running this in an ASP.NET Core project.

My team had deadlock issues a few times when we used AbcAsync().Result or .Wait() (the method was called in a NET Core Web API methods and deadlocks occurred mostly when we ran unit tests that execute the method)

Some unit test frameworks do provide a context - most notably xUnit. The context provided by xUnit is a synchronizing context, so it acts more like a UI context or ASP.NET pre-Core context. So when your code is running in a unit test, it does have the second condition for the deadlock, and the deadlock can happen.

As noted above, the best solution is to remove the blocking completely; this will have the nice side effect of making your server more efficient. But if the blocking must be done, then you should wrap your unit test code in a Task.Run, not your ASP.NET Core code.

Upvotes: 4

Related Questions