C# Multiple ASYNC requests single response (Promise like)

Question

I'm no too good with async processes in C#. I need to implement method, that can be called multiple times by async requests, but request data and serve response once. I've created this method, inspired by common JS promise pattern:

private Task<bool> DoSomething(string key)
{
 // generating unique key
 string cacheKey = "do-comething-" + key;
 // searching cache for a Task in progress
 var cachedRequest = MemoryCache.Default.Get(cacheKey, null) as Task<bool>;
 // if found - return old Task
 if (cachedRequest != null)
 {
 return cachedRequest;
 }
 TaskCompletionSource<bool> tcs = new TaskCompletionSource<bool>();
 CacheItemPolicy cip = new CacheItemPolicy()
 {
 AbsoluteExpiration = new DateTimeOffset(DateTime.Now.AddSeconds(60))
 };
 // storing Task object in cache
 MemoryCache.Default.Set(cacheKey, tcs.Task, cip);
 try
 {
 // do something long here 
 tcs.TrySetResult(true);
 }
 catch(Exception ex)
 {
 tcs.TrySetResult(false);
 }
 finally
 {
 MemoryCache.Default.Remove(cacheKey);
 }
 return tcs.Task;
}

Is it ok, or I'm totally out of it? Any advice will be highly appreciated. Thanks.

• Vogel612: Please do not update the code in your question to incorporate feedback from answers, doing so goes against the Question + Answer style of Code Review. This is not a forum where you should keep the most updated version in your question. Please see what you may and may not do after receiving answers .
• Kindzoku: @Vogel612, but how can I add details for my question if it's not completely answered?
• Vogel612: The correct solution to that is to add enough details the first time around :) Since that's not possible anymore, the next best solution is to ask a "follow-up" question. Please make sure to include appropriate context to make sure that your question isn't closed as "hypothetical code"
• Kindzoku: Well, you helped me with main idea. I'll mark an answer as answer and ask my question on SO. Was my error in the first place to post question here. But well, I flagged my question for transition to SO even before your answer.

Answer 1

Bug

You have a pretty big multithreading issue in your code. When this method can be called from different threads, the MemoryCache doesn't necessarily contain an already created task. Consider the following execution:

Thread 1 | Thread 2
string cacheKey = ... | -
var cachedRequest = .. | -
if (cachedRequest != null) | -
if (cachedRequest != null) | string cacheKey = ...
if (cachedRequest != null) | var cachedRequest = ...
TaskCompletionSource tcs = | var cachedRequest = ...
TaskCompletionSource tcs = | if (cachedRequest != null) 
TaskCompletionSource tcs = | TaskCompletionSource tcs = 
... | ...

The result should be pretty obvious: This way two tasks for the same key can be started. Depending on how strict you need that semantic, that's a problem.

Fixing this is not trivial, but should be simple enough with a lock (_lock) block onto a private static const Object _lock = new Object();

How to Task

In general I'd expect a method that returns a Task<bool> to not actually do the work, as is suggested by //do something long here. Instead the code should submit the work as an Action or something along these lines. Something like that:

Task<bool> result = new Task<bool>(() => 
{
 try
 {
 // do something long here
 return true;
 }
 catch (Exception e) 
 {
 return false;
 }
});

this shows an additional problem. The return value of that Task is basically just a flag for successful completion. But you know what? Task already has that: Task.IsFaulted

This makes for a pretty different return value. Use Task instead of Task<bool>, because it removes redundancy.

Back to the code: We're still not quite there with the lookup and caching mechanics.

CacheItemPolicy cip = new CacheItemPolicy()
...
MemoryCache.Default.Set(cacheKey, result, cip);

This is for the caching, now we just need to clean the cache when we're finished:

result.ContinueWith(() => MemoryCache.Default.Remove(cacheKey));
result.Start(); // actually start doing work
return result;

• Kindzoku: Thx, I'll try to implement your solution. It looks very clear. A lot clearer than mine.
• Kindzoku: I've updated my question with your solution. Do I implemented it right? Feel myself a bit confused. How can I use return value? The method that call DoSomething is sync. How can I get my flag outside? Thx