Giardino
Giardino

Reputation: 1387

Why does this wrapping in Task.Run remove deadlock from asp.net core?

I am trying to fix a method that is supposed to be retrieving arbitrary data from a MemoryCache using the built-in GetOrCreateAsync. Here is Microsofts example (from https://learn.microsoft.com/en-us/aspnet/core/performance/caching/memory?view=aspnetcore-3.0) :

public async Task<IActionResult> CacheGetOrCreateAsynchronous()
{
    var cacheEntry = await
        _cache.GetOrCreateAsync(CacheKeys.Entry, entry =>
        {
            entry.SlidingExpiration = TimeSpan.FromSeconds(3);
            return Task.FromResult(DateTime.Now);
        });

    return View("Cache", cacheEntry);
}

Here is mine:

/// <summary>
/// This deadlocks :(
/// </summary>
/// <param name="dayKey"></param>
/// <returns></returns>
public async Task<IEnumerable<Document>> GetEventsForDay(string dayKey)
{
    CheckCacheKeyExists(dayKey);
    return await _cache.GetOrCreateAsync(dayKey, async entry =>
    {
        entry.SetOptions(_cacheEntryOptions);
        var events = await EventsForDay(dayKey).ConfigureAwait(false);
        return events;
    }).ConfigureAwait(false);
}

Where EventsForDay has a signature of

private async Task<IEnumerable<Document>> EventsForDay(string dayKey)

And all async operations within EventsForDay are await and ConfigureAwait(false) to try prevent deadlocks. However even with those ConfigureAwait(false), the async lambda above still deadlocks and never returns.

However, if I use the solution detailed here: https://stackoverflow.com/a/40569410 it no longer deadlocks:

/// <summary>
/// This does not deadlock. Credit here: https://stackoverflow.com/a/40569410
/// </summary>
/// <param name="dayKey"></param>
/// <returns></returns>
public async Task<IEnumerable<Document>> GetEventsForDay(string dayKey)
{
    CheckCacheKeyExists(dayKey);
    var cachedItem = await Task.Run(async () =>
    {
        return await _cache.GetOrCreateAsync(dayKey, async entry =>
        {
            entry.SetOptions(_cacheEntryOptions);
            var events = await EventsForDay(dayKey).ConfigureAwait(false);
            return events;
        }).ConfigureAwait(false);
    }).ConfigureAwait(false);
    return cachedItem;
}

My questions are:

1.) Why does wrapping my async lambda in a Task.Run remove the deadlock?

2.) Why does my async lambda deadlock in the first place if the entire call stack after GetEventsForDay always uses ConfigureAwait(false) for any awaited methods in addition to the lambda itself being awaited with its own ConfigureAwait(false)?

Upvotes: 2

Views: 732

Answers (1)

Stuart
Stuart

Reputation: 5506

Great question.

When you do Task.Run, the delegate that you pass to it will be run on the threadpool, but importantly here for you, not with a SynchronizationContext. This is what would usually give you a deadlock, when you are blocking on the result of some code, which itself has exclusive access to that context.

So Task.Run "fixes it", except it doesn't really, it just masks another problem, you are blocking threads on async work. This can eventually lead to thread pool starvation at load, where .NET cannot add any more threads to unblock the current work.

.ConfigureAwait(false) when using await means that if the Task was incomplete then the following code doesn't need to run on the current SynchronizationContext, this also contributes to the deadlock scenario mentioned earlier.

An oversight people often have is that it effectively does nothing when then Task is already complete, for example Task.FromResult, or maybe you have some cached result. In this case the SynchronizationContext flows further into your code, increasing the chance of some subsequent code blocking an async Task with it still present, resulting in a deadlock.

Going back to your example, it doesn't matter how many .ConfigureAwait(false) you put into your code, if the code that runs before the await does some blocking over async code then you are still susceptible to deadlocking. This would also fix it for you, assuming that the source of the deadlock occurs in EventsForDay (don't do this):

public async Task<IEnumerable<Document>> GetEventsForDay(string dayKey)
{
    CheckCacheKeyExists(dayKey);
    var cachedItem = await _cache.GetOrCreateAsync(dayKey, async entry =>
        {
            await Task.Delay(100).ConfigureAwait(false); // this would also stop the deadlock
            entry.SetOptions(_cacheEntryOptions);
            var events = await EventsForDay(dayKey);
            return events;
        });
    return cachedItem;
}

and so would this:

public async Task<IEnumerable<Document>> GetEventsForDay(string dayKey)
{
    CheckCacheKeyExists(dayKey);
    var cachedItem = await _cache.GetOrCreateAsync(dayKey, async entry =>
        {
            SynchronizationContext.SetSynchronizationContext(null); // this would stop it too
            entry.SetOptions(_cacheEntryOptions);
            var events = await EventsForDay(dayKey);
            return events;
        });
    return cachedItem;
}

Upvotes: 2

Related Questions