2

I have read that I should only use Result instead of await when I am absolutely sure that an operation is completed. I am not really sure what happens underneath and would like to ask experienced programmers if this is a perfectly safe usage of await / Result / async.

public static bool Success()
{
    return 0 < Execute("DELETE FROM Table WHERE Id = 12").Result;
}

public static async Task<int> Execute(string sql)
{
    using (var con = Connection)
    {
        con.Open();
        return await con.ExecuteAsync(sql);
    }
}
i3arnon
  • 113,022
  • 33
  • 324
  • 344
ebvtrnog
  • 4,167
  • 4
  • 31
  • 59
  • possible duplicate of [await works but calling task.Result hangs/deadlocks](http://stackoverflow.com/questions/17248680/await-works-but-calling-task-result-hangs-deadlocks) – sstan Jul 03 '15 at 20:58
  • `I have read that I should only use Result instead of await when I am absolutely sure that an operation is completed.` Where did you read that? It's completely wrong. – Stephen Cleary Jul 04 '15 at 13:12
  • @StephenCleary well... it was your answer here... http://stackoverflow.com/questions/24623120/await-on-a-completed-task-same-as-task-result – ebvtrnog Jul 04 '15 at 14:30
  • @Randolph: Um, that whole answer is about why you should use `await` instead of `Result`, not the other way around. – Stephen Cleary Jul 04 '15 at 18:55

2 Answers2

4

This is not safe. The operation hasn't completed yet when you use Task.Result

When you call an async method it runs synchronously until it reaches an await and then returns to the caller with a task that represents the asynchronous operation.

The task isn't completed and using Task.Result will block the calling thread.

You should await the returned task instead, or use the synchronous option.

public static Task<bool> SuccessAsync()
{
    return 0 < await ExecuteAsync("DELETE FROM Table WHERE Id = 12");
}

public static async Task<int> ExecuteAsync(string sql)
{
    using (var con = Connection)
    {
        con.Open();
        return await con.ExecuteAsync(sql);
    }
}
i3arnon
  • 113,022
  • 33
  • 324
  • 344
  • In scenarios were you can't await, you should just `.Wait()` on the Task, or let the Task do its thing and add what ever additional code you want to run as a continuation on the running task. You can't always await, there are times where that is not an option – Johnathon Sullinger Jul 03 '15 at 20:41
  • 3
    @JohnathonSullinger doing `.Wait()` is just as bad as doing `.Result`. **In situations where you can't await you should not be using async/await anywhere in that call stack**. You should be performing synchronous calls instead. – Scott Chamberlain Jul 03 '15 at 20:45
  • `.Wait()` will not cause a dead lock will it? There are times were you can't get around it. An example I ran into recently was a void event handler on the UI that I had to call a 3rd party async method in. If I made the method `async void`, the view shut down process continued and I couldn't complete my interaction with the 3rd party framework. There are cases were you have to wait because of things outside of your control. – Johnathon Sullinger Jul 03 '15 at 20:49
  • 2
    @JohnathonSullinger it definitely will. – i3arnon Jul 03 '15 at 20:49
  • Can you point me to documentation explaining why that method will deadlock? I'm not sure I understand why it would, as I'm not accessing the result directly. I would think .Wait() would be thread-safe. – Johnathon Sullinger Jul 03 '15 at 20:51
  • 1
    `async void` is the same as starting a async operation and never calling `await` on it. As for your deadlock, from [the MSDN magazine](https://msdn.microsoft.com/en-us/magazine/jj991977.aspx) see **"Figure 3 A Common Deadlock Problem When Blocking on Async Code"** – Scott Chamberlain Jul 03 '15 at 20:54
  • 1
    @JohnathonSullinger Both are thread-safe. That's besides the point. The deadlock happens because you're synchronously waiting on a task (i.e. blocking a thread) that can only complete using the same thread. So the thread waits for itself. It doesn't matter whether you wait with `Wait` or with `Result` – i3arnon Jul 03 '15 at 20:55
3

It is safe, though it ends up blocking until the Execute is complete. Accessing Task.Result is equivalent to:

 Task<int> task = Execute("...");
 task.Wait(); // blocks until done
 return 0 < task.Result;

If you wanted to further the await chain, you could have Success return a Task<bool> instead.

Note that there is a potential for a deadlock depending on the current SynchronizationContext. If your SynchronizationContext is single-threaded, then your call to Task.Result ends up blocking and waiting for Execute to finish, but Execute is waiting for you to release the thread so it can continue. If you're in a GUI app (using a single-threaded SynchronizationContext by default) or ASP.NET, then you'd want to consider adding ConfigureAwait(false) to have Execute run on a thread pool thread instead.

Mark Brackett
  • 84,552
  • 17
  • 108
  • 152
  • @i3arnon - I classify it as "safe" since there's no undefined behavior or exception. Blocking isn't unsafe, it's just the way it is - sometimes you want it, sometimes you don't. – Mark Brackett Jul 03 '15 at 20:37
  • 1
    This definitely isn't "safe". Using `.Result` from a UI thread can deadlock. Half the `async-await` questions on SO are because people don't understand what goes on when you block an async method. – Yuval Itzchakov Jul 03 '15 at 20:38
  • Can't accessing .Result from across threads (not just UI) potentially deadlock as well? – Johnathon Sullinger Jul 03 '15 at 20:40
  • @Jonathan It's all about synchronization context. Not particularly threads. – Yuval Itzchakov Jul 03 '15 at 20:42
  • 2
    @JohnathonSullinger The deadlock occurs when you block on the thread the `SyncronisationContext` would have used to perform the completion. In the "stock" .NET stuff the UI sync contexts (There is one for Winforms and one for WPF) are the only single threaded sync contexts. The "default context" uses the thread pool so you would not deadlock unless you blocked every thread in the threadpool. – Scott Chamberlain Jul 03 '15 at 20:43
  • You're right, that's what I meant. Thanks for clarifying – Johnathon Sullinger Jul 03 '15 at 20:43
  • The potential async deadlock depends on how/where this code is being run and the SynchronizationContext. I see your point though, so I'll add an edit. – Mark Brackett Jul 03 '15 at 20:48
  • @ScottChamberlain the asp.net one may deadlock as well when it tries to post back to the same context. – i3arnon Jul 03 '15 at 20:49
  • The asp one uses the threadpool also (I just tried to add that info to my last comment but my 5 min ran out). Doing async/await in the asp context just flows the user context across the await, it does not keep the same thread. – Scott Chamberlain Jul 03 '15 at 20:50
  • 1
    @ScottChamberlain It uses threads from the threadpool, but it has its own SynchronizationContext and the deadlock here isn't because of a single thread, but a single request context. – i3arnon Jul 03 '15 at 20:57
  • Ah, sorry. I don't do much ASP.NET stuff. – Scott Chamberlain Jul 03 '15 at 20:57