0

I apologize for sounding dense, but I swear I have read up on async/await and played with examples, but I am still stumped at simple things like why the following code is giving me error:

        [HttpGet("{includeInactive}")]
        public async Task<List<Torrent>> GetTorrentsAll(bool includeInactive)
        {
            List<Torrent> torrentsAll = await _cacheStore.GetCachedTorrentsAll();

            //...potentially filter torrentsAll here

            return torrentsAll;
        }

So _cacheStore.GetCachedTorrentsAll() is not an async function. It's just a function that potentially might be taking a while in the database, so I was trying to ensure that it's not blocking. I thought that awaiting it within the async web API was the way to go about that. But I get confusing error under the await _cacheStore.GetCachedTorrentsAll() line that says 'List does not contain a definition for 'GetAwaiter' and no accessible extension method 'GetAwaiter' accepting a first argument of type 'List' could be found (are you missing a using directive or an assembly reference?).

I am very confused by this.

ruttergod
  • 167
  • 9
  • 2
    You can only use `async` on a method that returns something awaitable (typically `Task`). You're probably looking for `Task.Run` to run the method in the background (which you could then await). – Kirk Woll May 12 '22 at 21:53
  • You should probably check if the database client you are using doesn't have an overload that is async so that you can call that method instead and make GetCachedTorrentsAll async as well. You should also add a CancellationToken parameter to the GetTorrentsAll method that can then be passed down to stop any unnecessary work when the request gets cancelled. – GuyVdN May 12 '22 at 22:04
  • 3
    *"It's just a function that potentially might be taking a while in the database, so I was trying to ensure that it's not blocking."* -- What's the problem if the `GetTorrentsAll` method is blocking? Why do you care about it? Apparently you are authoring a Web-API, not an application with a GUI. – Theodor Zoulias May 12 '22 at 23:05
  • @TheodorZoulias please correct me if I am wrong. But a blocking thread takes away one thread from the thread pool. Heavy load could then exhaust the thread pool and cause issues. By using the async-await pattern, the thread is released back to the pool and can be reused, reducing the risk of thread exhaustion – Exitare May 12 '22 at 23:23
  • @Exitare this is true, but my question is targeting the OP. Their intentions/reasoning might be something different than what you or me might think. – Theodor Zoulias May 12 '22 at 23:49
  • @TheodorZoulias `GetTorrentsAll` will be used by potentially up to a couple hundred callers in a short period of time (some from UI, some from C# background workers). Typically, it will not be hit that much, but there will be times when that might happen. Perhaps I am understanding it wrong, but I thought if this entry point was not `async`, then it would hold up the thread during the potential hit to the database (?). – ruttergod May 13 '22 at 12:52
  • @TheodorZoulias But I guess maybe since `_cacheStore.GetCachedTorrentsAll()` is not `async`, it will still block the thread regardless (?) In which case `async/await` seems like a rabbit hole to me, which is why I've found myself abandoning the concept before. – ruttergod May 13 '22 at 13:01
  • 1
    @ruttergod *"But I guess maybe since `_cacheStore.GetCachedTorrentsAll()` is not `async`, it will still block the thread regardless."* -- Exactly. In order to avoid blocking [any](https://blog.stephencleary.com/2013/11/there-is-no-thread.html) thread, the underlying call must be genuinely asynchronous. Regarding using the `Task.Run` method for the purpose of keeping the UI responsive, check out [this](https://devblogs.microsoft.com/pfxteam/should-i-expose-asynchronous-wrappers-for-synchronous-methods/) article for recommendations about *where* to put it. – Theodor Zoulias May 13 '22 at 15:48

2 Answers2

1

So _cacheStore.GetCachedTorrentsAll() is not an async function. It's just a function that potentially might be taking a while in the database, so I was trying to ensure that it's not blocking.

It is reasonable to make it asynchronous, then.

I guess maybe since _cacheStore.GetCachedTorrentsAll() is not async, it will still block the thread regardless (?)

This is correct. Synchronous methods - by definition - block the calling thread until they complete.

In which case async/await seems like a rabbit hole to me,

It can be. In this case, you have correctly identified an operation that should be asynchronous (i.e., I/O-bound). But just sticking an async on the controller method isn't going to help; that's actually the last step in making this asynchronous.

The first step is to identify the lowest-level API that should be asynchronous. E.g., the database query. Make that call use the asynchronous database API first, use await, and make that method async. Then let async/await grow from that method up through your app. It will eventually end up at the controller method, and then you can make that async.

Stephen Cleary
  • 437,863
  • 77
  • 675
  • 810
-3

You can rewrite your code to look like this:

[HttpGet("{includeInactive}")]
public async Task<List<Torrent>> GetTorrentsAll(bool includeInactive, CancellationToken token)
{
    return await Task.Run(() => {
        List<Torrent> torrentsAll = _cacheStore.GetCachedTorrentsAll();

    
    return torrentsAll;
    }, token);
}
CorrieJanse
  • 2,374
  • 1
  • 6
  • 23
  • I'm a little confused by your `return await` with a `return` inside the running task. But after yours and another's comments about `Task.Run` I looked into it and came up with the following `List torrentsAll = await Task.Run(() => _cacheStore.GetCachedTorrentsAll(), cancellationToken); return torrentsAll.Where(t => includeInactive || t.IsActive).ToList();` which appears to be at least giving me correct results in Swagger :-) Is there anything about my new version that is of concern? – ruttergod May 12 '22 at 22:21
  • 4
    @ruttergod In ASP.Net this does nothing for performance or scalability except cause an extra context switch, as it's limited to 1 thread. See https://stackoverflow.com/questions/33764366/is-task-run-considered-bad-practice-in-an-asp-net-mvc-web-application – Charlieface May 13 '22 at 00:16
  • @Charlieface Thanks!! Based on the answer to the question you linked, I think it is unnecessary in this case to use `async`. – ruttergod May 13 '22 at 13:09