10

I'm trying to populate my cache asynchronously

static ConcurrentDictionary<string, string[]> data = new ConcurrentDictionary<string, string[]>();

public static async Task<string[]> GetStuffAsync(string key)
{
    return data.GetOrAdd(key, async (x) => {
        return await LoadAsync(x);
    });
}

static async Task<string[]> LoadAsync(string key) {....}

but this gives me the error:

Cannot convert async lambda expression to delegate type 'System.Func'.

An async lambda expression may return void, Task or Task, none of which are convertible to 'System.Func'.

As I understand this is because GetOrAdd() is not asynchronous. How can I fix the issue?

Update: LazyAsync suggested in the comments will work in my trivial example. Or, workaround like this (can definitely live with some overhead it introduces):

public static async Task<string[]> GetStuffAsync(string key)
{
    string[] d = null;
    if (!data.ContainsKey(key))
        d = await LoadAsync(key);
    return data.GetOrAdd(key, d);
}

The question then becomes did Microsoft just have no time to update all interfaces to support async or I'm trying to do something deeply wrong (and ConcurrentDictionary shouldn't have GetOrAddAsync()) ?

Erik Philips
  • 53,428
  • 11
  • 128
  • 150
UserControl
  • 14,766
  • 20
  • 100
  • 187
  • 2
    not an answer, but maybe interesting for you: have you seen [AsyncLazy](http://blogs.msdn.com/b/pfxteam/archive/2011/01/15/10116210.aspx)? – default Jan 21 '15 at 12:27
  • 1
    Well, you can't do it using the interface you have. `await` only works when it's across async layers, and you're missing one - the method `GetOrAdd` itself. You would need a version that's asynchronous itself, and await the whole `GetOrAdd` method (note how you're not actually doing that right now - that's another problem). Sadly, this means writing your own ConcurrentDictionary, which is going to hurt. – Luaan Jan 21 '15 at 13:02
  • Why should `ConcurrentDictionary` have `GetOrAddAsync` ? It is a synchronous in memory collection. There is no reason for it to have such a method. – Sriram Sakthivel Jan 21 '15 at 15:58
  • @Sriram Sakthivel, Then I must be doing it fundamentally wrong and I want to understand what exactly (may be I need to use another caching pattern or something). The whole goal of async/await is simple asynchronous code, right? And although your solution is pretty smart I cannot justify its complexity. – UserControl Jan 21 '15 at 17:24
  • @UserControl your assumption is correct. *The whole goal of async/await is simple asynchronous code* it applies for asynchronous methods(API) but not for APIs which isn't asynchronous. You're trying to access synchronous API asynchronously; which makes no sense actually. Ok, if you're not convinced with my answer you may work a little on your `GetStuffAsync` version2 to make it thread safe, and that should be fine. – Sriram Sakthivel Jan 21 '15 at 18:16
  • http://stackoverflow.com/questions/21626014/caching-asynchronous-operations – UserControl Jan 21 '15 at 20:06
  • @UserControl: How do you use AsyncLazy to solve your issue? I cannot figure out how to magically avoid compilation errors? – sonatique Jan 22 '16 at 10:18

1 Answers1

24

Async methods (or lambda) can only return void or Task or Task<T> but your lambda returns string[] and thus compiler prevents you.

await keyword is optimized to continue synchronously when the Task is already completed. So, one option is to store the Task itself in dictionary and don't worry about awaiting the completed Task again and again.

private static ConcurrentDictionary<string, Task<string[]>> data =
    new ConcurrentDictionary<string, Task<string[]>>();

public static Task<string[]> GetStuffAsync(string key)
{
    return data.GetOrAdd(key, LoadAsync);
}

And when you do

var item = await GetStuffAsync(...);

first time it will (a)wait till the cached Task finishes --There after it will continue synchronously.

You'll have to think about what should happen when LoadAsync fails. Because we're caching the Task returned by LoadAsync; if that fails we'll foolishly cache the failed Task. You may need to handle this.

Sriram Sakthivel
  • 72,067
  • 7
  • 111
  • 189
  • It might actually be possible to use this approach to build a wrapper around `ConcurrentDictionary` that will expose the asynchronous `GetOrAddAsync` method and handle the failures inside of itself. It's still going to be a lot of tricky work with manual locks, so it might still be a better idea to synchronize access using one of the older synchronization patterns. – Luaan Jan 21 '15 at 13:05
  • 1
    rather than writing `async (x) => await LoadAsync(x)` you can just write `x => LoadAsync(x)`. They're the same method. – Servy Jan 21 '15 at 15:52
  • @Servy yes, that looks redundant for me too. Thanks. I'll update my answer. – Sriram Sakthivel Jan 21 '15 at 15:53
  • 2
    Actually, you can even write `return data.GetOrAdd(key, LoadAsync);` :) – UserControl Jan 21 '15 at 15:56
  • @UserControl ah you're right.. Thanks for implicit delegate conversion :) Updated my answer as well. – Sriram Sakthivel Jan 21 '15 at 15:59
  • 1
    One down-side to this approach is that if the Task fails once (maybe due to a network error), the ConcurrentDictionary entry will be a task that throws an exception whenever it's awaited. You may want some extra logic to invalidate cache entries when this happens. – StriplingWarrior May 10 '18 at 21:27
  • 1
    @StriplingWarrior good point I've also added that in my answer as remark. That can be handled with setting a continuation on faulted before returning the task(just created ones), then remove it from the cache. – Sriram Sakthivel May 12 '18 at 07:00