157

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?

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
Johan Larsson
  • 17,112
  • 9
  • 74
  • 88
  • 9
    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. – ghord Jun 22 '13 at 09:02
  • 4
    @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`. – Stephen Cleary Jun 22 '13 at 17:27
  • @StephenCleary Yes, though it was allowed in betas of VS2012, which was causing all kinds of issues. – ghord Jun 22 '13 at 18:25
  • @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. – Joey Aug 09 '23 at 06:40
  • @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. – Stephen Cleary Aug 09 '23 at 10:00

6 Answers6

271

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.

Herman Schoenfeld
  • 8,464
  • 4
  • 38
  • 49
  • 19
    -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. – Stephen Cleary Oct 08 '15 at 10:21
  • 96
    @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. – Herman Schoenfeld Oct 16 '15 at 02:01
  • 5
    It's obviously not "indispensible" since my solution works without `async void`, `Task.Run`, `Result`, or `Wait`. – Stephen Cleary Oct 16 '15 at 02:44
  • 94
    @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? – Herman Schoenfeld Oct 16 '15 at 06:05
  • 3
    Could you explain why it work's please? What is the difference between this solution and using just `Result` property? – user3818229 May 21 '17 at 12:30
  • 21
    @HermanSchoenfeld, if you added the *why* to the *how*, I believe your answer would benefit a lot. – ironstone13 Sep 06 '17 at 19:39
  • 5
    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. – Andrew Sep 11 '17 at 09:32
  • 27
    I know this is kind of late, but you should be using `.GetAwaiter().GetResult()` instead of `.Result` so that any `Exception` is not wrapped. – Camilo Terevinto Sep 22 '17 at 10:30
  • 1
    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. – AndyUK Jun 15 '18 at 06:44
  • 1
    How can this solution be used to get the actual result? If the async method returns a Task, how can I get the ResultObject without calling .Result? The solution proposed only calls the async method; it doesn't provide the return value. – Nick Aug 21 '18 at 21:42
  • 1
    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. – user1751825 Oct 07 '18 at 10:57
  • 1
    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() – MichaelDarkBlue Jan 04 '19 at 15:20
  • 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. – Konstantin Konstantinov Sep 18 '20 at 11:47
  • @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. – Abel Dec 29 '22 at 01:36
  • 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? https://learn.microsoft.com/en-us/dotnet/api/system.threading.tasks.taskscheduler?view=net-7.0 - I could see making a helper class to run async calls from sync code on a separate thread pool, like this. – Corrodias Mar 01 '23 at 05:29
111

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.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
  • 7
    changing to async Task worked, now I need to read the contents of your links a couple of times, ty sir. – Johan Larsson Jun 22 '13 at 09:13
  • @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". – Stephen Cleary Dec 07 '16 at 14:24
  • 2
    @StephenCleary what if I have to call an async method inside a constructor? Constructors can't be async. – Raikol Amaro Aug 03 '18 at 21:21
  • @RaikolAmaro: Short answer: you shouldn't do I/O in constructors. Long answer: https://blog.stephencleary.com/2013/01/async-oop-2-constructors.html – Stephen Cleary Aug 04 '18 at 00:07
  • 1
    @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? – void.pointer May 14 '20 at 22:06
  • 2
    @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` 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. – Stephen Cleary May 15 '20 at 02:02
  • 2
    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()).Result` – Coxy Feb 22 '21 at 05:50
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

Vladimir
  • 1,630
  • 2
  • 18
  • 28
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.

abatishchev
  • 98,240
  • 88
  • 296
  • 433
Dark Knight
  • 819
  • 8
  • 12
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

Ogglas
  • 62,132
  • 37
  • 328
  • 418
  • **"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. – Herman Schoenfeld Jan 06 '23 at 03:06
  • @HermanSchoenfeld See this example: https://michaelscodingspot.com/c-deadlocks-in-depth-part-1/#deadlock-example-1-the-nested-lock. `Task.Run` queues the specified work to run on the ThreadPool but a deadlock can still occur. – Ogglas Jan 09 '23 at 18:06
  • 1
    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. – Herman Schoenfeld Feb 09 '23 at 02:06
  • 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. – Corrodias Mar 01 '23 at 05:10
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.

Peter Mortensen
  • 30,738
  • 21
  • 105
  • 131
  • ```ConfigureAwait``` avoids deadlocking in certain scenarios by not running in the original thread context. – davidcarr Feb 05 '19 at 01:34