'await' works, but calling task.Result hangs/deadlocks
Asked Answered
O

6

159

I have the following four tests and the last one hangs when I run it. Why does this happen:

[Test]
public void CheckOnceResultTest()
{
    Assert.IsTrue(CheckStatus().Result);
}

[Test]
public async void CheckOnceAwaitTest()
{
    Assert.IsTrue(await CheckStatus());
}

[Test]
public async void CheckStatusTwiceAwaitTest()
{
    Assert.IsTrue(await CheckStatus());
    Assert.IsTrue(await CheckStatus());
}

[Test]
public async void CheckStatusTwiceResultTest()
{
    Assert.IsTrue(CheckStatus().Result); // This hangs
    Assert.IsTrue(await CheckStatus());
}

private async Task<bool> CheckStatus()
{
    var restClient = new RestClient(@"https://api.test.nordnet.se/next/1");
    Task<IRestResponse<DummyServiceStatus>> restResponse = restClient.ExecuteTaskAsync<DummyServiceStatus>(new RestRequest(Method.GET));
    IRestResponse<DummyServiceStatus> response = await restResponse;
    return response.Data.SystemRunning;
}

I use this extension method for restsharp RestClient:

public static class RestClientExt
{
    public static Task<IRestResponse<T>> ExecuteTaskAsync<T>(this RestClient client, IRestRequest request) where T : new()
    {
        var tcs = new TaskCompletionSource<IRestResponse<T>>();
        RestRequestAsyncHandle asyncHandle = client.ExecuteAsync<T>(request, tcs.SetResult);
        return tcs.Task;
    }
}
public class DummyServiceStatus
{
    public string Message { get; set; }
    public bool ValidVersion { get; set; }
    public bool SystemRunning { get; set; }
    public bool SkipPhrase { get; set; }
    public long Timestamp { get; set; }
}

Why does the last test hang?

Obligate answered 22/6, 2013 at 8:10 Comment(5)
You should avoid returning void from async methods. It's only for backward compatibility with exisiting event handlers, mostly in interface code. If your async method does not return anything, it should return Task. I had numerous problems with MSTest and void returning async tests.Rightward
@ghord: MSTest doesn't support async void unit test methods at all; they simply won't work. However, NUnit does. That said, I agree with the general principle of preferring async Task over async void.Grunenwald
@StephenCleary Yes, though it was allowed in betas of VS2012, which was causing all kinds of issues.Rightward
@StephenCleary: By now at least in NUNit 3, async void tests are not supported. Which makes kind of sense, as the test runner has to wait for the test to complete, which isn't possible for async void, only if you actually get a task.Sclerotomy
@Joey: It is possible for a synchronization context to detect when an async void method completes, which was the technique used by NUnit. If they no longer support async void, then I agree with that decision.Grunenwald
G
115

You're running into the standard deadlock situation that I describe on my blog and in an MSDN article: the async method is attempting to schedule its continuation onto a thread that is being blocked by the call to Result.

In this case, your SynchronizationContext is the one used by NUnit to execute async void test methods. I would try using async Task test methods instead.

Grunenwald answered 22/6, 2013 at 8:30 Comment(7)
changing to async Task worked, now I need to read the contents of your links a couple of times, ty sir.Obligate
@MarioLopez: The solution is to use "async all the way" (as noted in my MSDN article). In other words - as the title of my blog post states - "don't block on async code".Grunenwald
@StephenCleary what if I have to call an async method inside a constructor? Constructors can't be async.Kurdistan
@RaikolAmaro: Short answer: you shouldn't do I/O in constructors. Long answer: blog.stephencleary.com/2013/01/async-oop-2-constructors.htmlGrunenwald
@StephenCleary In nearly all of your replies on SO and in your articles, all I ever see you talk about is replacing Wait() with making the calling method async. But to me, this seems to be pushing the problem upstream. At some point, something has to be managed synchronously. What if my function is purposefully synchronous because it manages long running worker threads with Task.Run()? How do I wait on that to finish without deadlocking inside my NUnit test?Kalle
@void.pointer: At some point, something has to be managed synchronously. - not at all. For UI apps, the entrypoint can be an async void event handler. For server apps, the entrypoint can be an async Task<T> action. It is preferable to use async for both to avoid blocking threads. You can have your NUnit test be synchronous or async; if async, make it async Task instead of async void. If it's synchronous, it shouldn't have a SynchronizationContext so there shouldn't be a deadlock.Grunenwald
I came into this question from trying to resolve a deadlock (Task stuck in Scheduled status) and I found that making the event handler async did not help. I had to wrap as per Task.Run(() => asyncGetValue()).ResultPositron
I
273

Acquiring a value via an async method:

var result = Task.Run(() => asyncGetValue()).Result;

Syncronously calling an async method

Task.Run( () => asyncMethod()).Wait();

No deadlock issues will occur due to the use of Task.Run.

Icterus answered 7/9, 2015 at 0:46 Comment(15)
-1 for encouraging the use of async void unit test methods and removing same-thread guarantees provided by the SynchronizationContext from the system under test.Grunenwald
@StephenCleary: there's no "enouraging" of async void. It's merely employing valid c# construct to solve the deadlock issue. The above snippet is an indispensible and simple work-around to the OP's issue. Stackoverflow is about solutions to problems, not verbose self-promotion.Icterus
It's obviously not "indispensible" since my solution works without async void, Task.Run, Result, or Wait.Grunenwald
@StephenCleary: Your articles don't really articulate the solution (at least not clearly) and even if you had a solution, you'd use such constructs indirectly. My solution doesn't explicitly use contexts, so what? The point is, mine works and it's a one-liner. Didn't need two blog posts and thousands of words to solve the issue. NOTE: I don't even use async void, so I don't really know what you're on about.. Do you see "async void" anywhere in my concise and proper answer?Icterus
Could you explain why it work's please? What is the difference between this solution and using just Result property?Economic
@HermanSchoenfeld, if you added the why to the how, I believe your answer would benefit a lot.Currie
This is the right and simple way to go when you just need to sync call an async method. For example, all of refit, restease and other api generators give only async methods. Sometimes you just wanna call the damn things and not asyncify the entire class.Fantasm
I know this is kind of late, but you should be using .GetAwaiter().GetResult() instead of .Result so that any Exception is not wrapped.Morn
That first single line of code ought to be the accepted answer. Many thanks for clearing this particular hassle up for me. You can safely disregard those other sniping comments.Fungicide
How can this solution be used to get the actual result? If the async method returns a Task<ResultObject>, how can I get the ResultObject without calling .Result? The solution proposed only calls the async method; it doesn't provide the return value.Mulligan
Both options work for me. All other solutions I'd tried either caused my function to run asynchronously, or would hang the main thread. Use the first if you need the result, the second if you don't.Tillfourd
I am using VB.net so this solution is perfect. But you need to change the lambda expression to Task.Run(function () asyncMethod() end function).Wait()Chummy
I needed that to implement void OnAuthentication(AuthenticationContext filterContext) of System.Web.Mvc.Filters.IAuthenticationFilter where I had to call some async method. So: .GetAwaiter().GetResult() was hanging up but Task.Run(...) worked.Burushaski
@Nick, the point of the whole post is that you can call Result on Task.Run’s returned Task (usually). The motivation is in the comments, as are the cases where you shouldn’t do this. The answer itself could definitely be improved, though.Borderer
Since Task.Run will run in the same threadpool and can, if that pool is exhausted, potentially lead to a deadlock -- and since launching a single, new Thread is insufficient, as the thread will terminate when the first await is hit -- would it be safer (if inefficient) to launch an entirely new TaskScheduler, as seen here? learn.microsoft.com/en-us/dotnet/api/… - I could see making a helper class to run async calls from sync code on a separate thread pool, like this.Sell
G
115

You're running into the standard deadlock situation that I describe on my blog and in an MSDN article: the async method is attempting to schedule its continuation onto a thread that is being blocked by the call to Result.

In this case, your SynchronizationContext is the one used by NUnit to execute async void test methods. I would try using async Task test methods instead.

Grunenwald answered 22/6, 2013 at 8:30 Comment(7)
changing to async Task worked, now I need to read the contents of your links a couple of times, ty sir.Obligate
@MarioLopez: The solution is to use "async all the way" (as noted in my MSDN article). In other words - as the title of my blog post states - "don't block on async code".Grunenwald
@StephenCleary what if I have to call an async method inside a constructor? Constructors can't be async.Kurdistan
@RaikolAmaro: Short answer: you shouldn't do I/O in constructors. Long answer: blog.stephencleary.com/2013/01/async-oop-2-constructors.htmlGrunenwald
@StephenCleary In nearly all of your replies on SO and in your articles, all I ever see you talk about is replacing Wait() with making the calling method async. But to me, this seems to be pushing the problem upstream. At some point, something has to be managed synchronously. What if my function is purposefully synchronous because it manages long running worker threads with Task.Run()? How do I wait on that to finish without deadlocking inside my NUnit test?Kalle
@void.pointer: At some point, something has to be managed synchronously. - not at all. For UI apps, the entrypoint can be an async void event handler. For server apps, the entrypoint can be an async Task<T> action. It is preferable to use async for both to avoid blocking threads. You can have your NUnit test be synchronous or async; if async, make it async Task instead of async void. If it's synchronous, it shouldn't have a SynchronizationContext so there shouldn't be a deadlock.Grunenwald
I came into this question from trying to resolve a deadlock (Task stuck in Scheduled status) and I found that making the event handler async did not help. I had to wrap as per Task.Run(() => asyncGetValue()).ResultPositron
C
20

You can avoid deadlock adding ConfigureAwait(false) to this line:

IRestResponse<DummyServiceStatus> response = await restResponse;

=>

IRestResponse<DummyServiceStatus> response = await restResponse.ConfigureAwait(false);

I've described this pitfall in my blog post Pitfalls of async/await

Chlordane answered 20/12, 2014 at 15:5 Comment(0)
B
9

You are blocking the UI by using Task.Result property. In MSDN Documentation they have clearly mentioned that,

"The Result property is a blocking property. If you try to access it before its task is finished, the thread that's currently active is blocked until the task completes and the value is available. In most cases, you should access the value by using Await or await instead of accessing the property directly."

The best solution for this scenario would be to remove both await & async from methods & use only Task where you're returning result. It won't mess your execution sequence.

Bisexual answered 30/10, 2015 at 11:20 Comment(0)
S
4

An addition to the answer given by @HermanSchoenfeld. Unfortunately the quote below is not true:

No deadlock issues will occur due to the use of Task.Run.

public String GetSqlConnString(RubrikkUser user, RubrikkDb db) 
{ 
    // deadlock if called from threadpool, 
    // works fine on UI thread, works fine from console main 
    return Task.Run(() => 
        GetSqlConnStringAsync(user, db)).Result; 
}

The execution is wrapped inside a Task.Run, this will schedule the task on the threadpool the block the calling thread. This is okay, as long as the calling thread is not a threadpool thread. If the calling thread is from the threadpool then the following disaster happens: A new task is queued to the end of the queue, and the threadpool thread which would eventually execute the Task is blocked until the Task is executed.

In library code there is no easy solution as you cannot assume under what context your code is called. The best solution is to only call async code from async code, blocking sync APIs from sync methods, don’t mix them.

Source:

https://medium.com/rubrikkgroup/understanding-async-avoiding-deadlocks-e41f8f2c6f5d

Sphenoid answered 8/2, 2021 at 9:39 Comment(4)
"the threadpool thread which would eventually execute the Task is blocked until the Task is executed" This does not follow. Just because a threadpool thread is blocking, it doesn't mean all other thread pool threads get blocked. That's the point of a thread pool, multiple concurrent threads doing work.Icterus
@HermanSchoenfeld See this example: michaelscodingspot.com/c-deadlocks-in-depth-part-1/…. Task.Run queues the specified work to run on the ThreadPool but a deadlock can still occur.Sphenoid
That is a user-level deadlock caused by the users logic not because of underlying framework/event-pump implementation logic causing Task.Result to deadlock. Different issue.Icterus
I must agree with Herman; you can launch two, independent threads in Java with the same workload and deadlock them. This is not a result of the async framework, and async is usually not used for locks (though they're something to be aware of!) but rather for blocking I/O. It's unfortunate that this causes 2 threads to be used instead of just the 1, though. I wish there were a better way to make sync calls to async functions.Sell
I
2

If you don't get any callbacks or the control hangs up, after calling the service/API async function, you have to configure Context to return a result on the same called context.

Use TestAsync().ConfigureAwait(continueOnCapturedContext: false);

You will be facing this issue only in web applications, but not in static void main.

Incorporeal answered 3/11, 2017 at 10:59 Comment(1)
ConfigureAwait avoids deadlocking in certain scenarios by not running in the original thread context.Phrenology

© 2022 - 2024 — McMap. All rights reserved.