0

I've just a question that came to my mind since I've started working on an existing project: Consider the following snippet of code

public async Task<IHttpActionResult> DoSomething(Guid token)
{
 var data= await repository.GetDataAsync(token);

 return await Task.FromResult(Ok(data));
}

As far I've seen when using await I can just have return Ok(data)

But I've been told that not using that Task.FromResult would lead to async problems... where can I find more info on this point? It's used in Web API Full Framework (not.NET Core)

Theodor Zoulias
  • 34,835
  • 7
  • 69
  • 104
advapi
  • 3,661
  • 4
  • 38
  • 73
  • 4
    No, it's very unnecessary. It won't cause any problems, but it is a lot of completely unnecessary cost – canton7 Jul 14 '21 at 10:39
  • 2
    `Task.FromResult` is generally used when you have an impedence mismatch - you're being forced to implement a method that has to return a `Task` (due to inheritance or an interface definition) but for various reasons your code doesn't have any reason to be async and is already capable of producing a `T`. – Damien_The_Unbeliever Jul 14 '21 at 12:32

1 Answers1

3

Absolutely no!

You should use Task.FromResult(Ok(data)) only if you receive data from the repository synchronously, else if you have at least one await keyword it's an unnecessary resources waste.

So, you can rewrite your method this way:

public async Task<IHttpActionResult> DoSomething(Guid token)
{
    return Ok(await repository.GetDataAsync(token));
}
Vladyslav Bardin
  • 49
  • 2
  • 5
  • `.Result`: [Don't Block on Async Code](https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html). Your answer would be improved if you just deleted everything from the phrase *"In this case,"* and below. – Theodor Zoulias Jul 14 '21 at 11:42
  • 1
    Yep, you're right. It's my fault. I'll correct my answer with this [snippet](https://sharplab.io/#v2:EYLgxg9gTgpgtADwGwBYA0AXEUCuA7AHwAEAmARgFgAoIgBgAIiyBWAbmuqIGZGTGyA7PQDe1euMY8mSRgA5GMgLIBDAJZ4AFAEoRYifoD0B+gGc8qgA4WYGepSr79AN2VR6AE2UZlAMSgQAWwBlcysbMnoAXkYATnoAJRgLCBNVDGgATwA6AHEbABEvZQBBEwy8MG12B0dxJhiNT28/QJDLawwIgGp6ACJ6ADN/ANNQjrterWra8T1ao1H2m3oSOccXNybfYbawjD5ooiQsloDEkxwAGwwNROTU9KhsvIxC71Lyyq0s86uMb9+12mM3qjSKp12HT4PX6Q0Ciz2K0mwPEAF81hiqOiqJwpGQZKQEkkUmlMvRqKIanU8QSkAAeJi0AB89BebxKZQq2nJVN0vP0RCERxOw0BN16QWUAQslxgHiKyLW2OxQA===). – Vladyslav Bardin Jul 14 '21 at 11:49
  • @TheodorZoulias, maybe I'm not right, but there is [no synchronization context](https://blog.stephencleary.com/2017/03/aspnetcore-synchronization-context.html) at ASP.NET Core, so in that case, it won`t be any deadlock. Moreover, at the link in my previous comment, I add a code snippet and their asynchronous code works pretty well. – Vladyslav Bardin Jul 14 '21 at 12:07
  • Vladislav you are right, Stephen Cleary's [article](https://blog.stephencleary.com/2012/07/dont-block-on-async-code.html) is about preventing deadlocks. But there are also other problems associated with blocking on async code, beyond deadlocks, like `ThreadPool` exhaustion and loss of scalability. – Theodor Zoulias Jul 14 '21 at 12:12
  • Thanks to all, but maybe it's used since in the repository there can be a sync access to some resources?? – advapi Jul 14 '21 at 12:42
  • I don't think so. If your repository returns `Task` you can use the code from snippet 1, and it should be okay – Vladyslav Bardin Jul 14 '21 at 15:09