Why does this task cause a deadlock?

Question

.NET 4.7.2 website

using System;
using System.Threading.Tasks;
using System.Web;

public partial class _Default : System.Web.UI.Page
{
    protected void Page_Load(object sender, EventArgs e)
    {
        if (IsPostBack)
        {
            string input = Input.Text;
            bool iWantDeadlock = input == "yes";
            string key = iWantDeadlock
                ? GetHashFragmentAsync(input).GetResultSafely()
                : Task.Run(() => GetHashFragmentAsync(input)).Result;
            Response.Redirect(key);
        }
    }

    private static async Task<string> GetHashFragmentAsync(string key)
    {
        await Task.Delay(100);
        return "#" + HttpUtility.UrlEncode(key);
    }
}

public static class TaskExtensions
{
    public static T GetResultSafely<T>(this Task<T> task)
    {
        return Task.Run(() => task).Result;
    }
}

I made a very simple page, with a textbox, that puts the input in the hashfragment of the page after submitting it.

I was reading up on Tasks and deadlocks (after I ran into some code from others that was causing one).

The solution seems to be to do this:

Task.Run(() => GetHashFragmentAsync(input)).Result;

So I thought, let's make that an extension for clarity and easy use.

public static class TaskExtensions
{
    public static T GetResultSafely<T>(this Task<T> task)
    {
        return Task.Run(() => task).Result;
    }
}

This however causes a deadlock. The code is the same, but works very different. Can someone explain this behaviour?

*The solution seems to be `Task.Run(()`* the correct solution is to make your function(s) `async` — Liam, Oct 12 '18 at 13:03
Result does not deadlock by itself. it's not an automatic bug. — usr, Oct 12 '18 at 13:03
@LasseVågsætherKarlsen no, it's not. It's a combination of two things. Can't arbitrarily pick one and declare it the reason. Also, it's not always feasible to remove Task.Run for example when implementing a sync interface. When perf is not a concern it's also totally valid to just slap Task.Run on it to save a lot of refactoring work. We don't want fanatical async implementations. We want the right solution. — usr, Oct 12 '18 at 13:06
[Webforms supports `async`/`await` if you enable this](https://learn.microsoft.com/en-gb/aspnet/web-forms/overview/performance-and-caching/using-asynchronous-methods-in-aspnet-45) then you can solve a lot of these problems without the use of thread pool threads. Or better yet, use MVC which supports this out of the box — Liam, Oct 12 '18 at 13:10

FCin · Answer 1 · 2018-10-12T13:11:04.233

Because when you use GetHashFragmentAsync(input).GetResultSafely() you have actually 2 tasks not 1.

GetHashFragmentAsync(input) returns already started task. Then you call GetResultSafely() which creates another task.

When you wait for a task on UI thread you deadlock, because task cannot go back to synchronization context thread. When you have two tasks, then the second task can wait synchronously, because parent task is not on UI thread, but on ThreadPool thread, which doesn't have synchronization context.

When you call any IO based code, you should never call Task.Run. Simply write

protected async void EventHandlerMethod(object sender, EventArgs e)
{
    await GetHashFragmentAsync(input);
}

Here is what happens

GetHashFragmentAsync(input) // Start Task number 1
.GetResultSafely() // Start task number 2 from task number 1 (no synchronization context)
// .Result returns back to task 1

Second scenario

Task.Run(() => GetHashFragmentAsync(input)) // UI thread so, capture synchronization context
.Result // Wait for UI thread to finish (deadlock)

usr · Accepted Answer · 2018-10-12T13:07:44.777

-2

When you call GetHashFragmentAsync(input) the current synchronization context is captured by the C# async/await machinery. The method returns a started task which depends on the UI thread. You try to use Task.Run to move that task of the critical UI thread but it's too late.

GetHashFragmentAsync(input) must be called on a non-UI thread already. Wrap it in Task.Run.

Here's a helper method that works by taking a factory:

public static T GetResultSafely<T>(Func<Task<T>> task)
{
    return Task.Run(() => task()).Result;
}

The factory is called on the thread pool.

I should say that normally the best solution is to use async APIs through and through, or to stay totally synchronous. Mixing is problematic for correctness and performance reasons. But it absolutely can be done safely.

edited Oct 12 '18 at 13:07

answered Oct 12 '18 at 13:02

usr

168,620
35
240
369

Thanks, I did not realize that subtle difference. – WV PM Oct 12 '18 at 13:10
1

I'm not convinced this is a the right choice in a web environment. Now that HTTP request will use two threads not one. If you have high though put this can drastically increase the strain on a web server. [Webforms supports async await](https://learn.microsoft.com/en-gb/aspnet/web-forms/overview/performance-and-caching/using-asynchronous-methods-in-aspnet-45), why not use it? – Liam Oct 12 '18 at 13:15
@Liam it will use just one because the thread pool thread only runs for one nano second to create the task. We have built web apps using threads for decades. It's totally fine in most cases. Most web servers are not ever limited on threads in their lifetime. – usr Oct 12 '18 at 13:33
It really is crazy how much brainwashing has occurred with regard to async. It is seen as something you must do. The opposite is true: It is rarely beneficial in any way for typical web apps. But it has a productivity cost. Sometimes you genuinely need it, though. – usr Oct 12 '18 at 13:34
I used to work for a travel company that searched for flights, thread usage was the main scaling issue for us. So it can be an issue. – Liam Oct 12 '18 at 13:34
Definitely can. Likely, you @Liam called lots of search services. In those cases make those code path fully async and leave the remaining 99% of the app synchronous! It's not all or nothing. – usr Oct 12 '18 at 13:35

Why does this task cause a deadlock?

2 Answers2