CODEWITHSUNDEEP
c#async-await
MRP
MRP

Reputation: 609

Prevent calling async method while the same instance is under process

In my Web Application, I have created a method that creates a GET request according to the passed URL and then create a GET call using HttpClient. The problem is some GET calls may take long time to complete and while that request is under process, user can ask for the same URL again and there is of course another GET call to the same URL, for example user can call "www.google.com" many times while the former call is under process and has not completed yet.

I want to prevent this behavior, I want if there is same GET call under process, prevent any further GET calls to the same URL. Here is my code, But I don't know how to achieve this:

    var request = new RequestInfo(URL) { HttpMethod = "GET" };

    //here if the same URL is under process I want to prevent calling request.Send()
    if (await request.Send())
         {
           // other codes
         }

Upvotes: 0

Views: 1321

Answers (1)

Uladz
Uladz

Reputation: 1978

It seems pretty obvious, that some sort of registry for pending calls is required. You should consider some things before the actual implementation nevertheless.

Should this method be thread-safe? It should, if it will be invoked from multiple threads concurrently.

If multithreading issues are not the case, then simple Dictionary<string, Task> (you may want to use Task<T> here instead) will be enough. Here is a small snippet to get the idea.

private readonly IDictionary<string, Task> _requestRegistry = new Dictionary<string, Task>();

Task MakeRequestAsync(string url)
{
    if (_requestRegistry.TryGetValue(url, out var existingTask))
    {
        return existingTask;
    }

    var request = new RequestInfo(url) { HttpMethod = "GET" };
    var responseTask = request.Send();

    _requestRegistry.Add(url, responseTask);

    return responseTask;
}

Implementation will be a bit more complex for the concurrent case, as far as it's necessary to avoid race conditions for the shared registry field. We, probably, want to use existing instruments instead of manual locking, therefore we will utilizeConcurrentDictionary type here. Code may look similar to this now.

private readonly ConcurrentDictionary<string, Task> _requestRegistry = new ConcurrentDictionary<string, Task>();

Task MakeRequestAsync(string url)
{
    return _requestRegistry.GetOrAdd(url, _ =>
    {
        var request = new RequestInfo(url) { HttpMethod = "GET" };
        return request.Send();
    });
}

But actually there is a small issue with such approach — your delegate may be called twice, because current concurrent dictionary implementation invokes valueFactory before any locking is made as you can see from the source code. You should decide whether it's a problem for your scenario.

Fortunately, there is a neat and simple hack for this problem as well — our valueFactory should become lazy. It's still possible in this case that valueFactory will be invoked twice, but with the only web request.

private readonly ConcurrentDictionary<string, Lazy<Task>> _requestRegistry = new ConcurrentDictionary<string, Lazy<Task>>();

Task MakeRequestAsync(string url)
{
    var lazyRequest = _requestRegistry.GetOrAdd(url, _ => new Lazy<Task>(() =>
    {
        var request = new RequestInfo(url) {HttpMethod = "GET"};
        return request.Send();
    }));

    return lazyRequest.Value;
}

Do you expect results to become stale?

If the answer is yes, than it's not safe to return existing responses from the registry due to their age, only pending tasks may be considered healthy. It means you should remove tasks on completion. Snippet again to get the idea

request.Send()
    .ContinueWith(_ =>
    {
        _requestRegistry.Remove(url);
    });

Do you expect to have limited set of urls?

It will be required to implement some cache eviction policy, if not. Otherwise you may use excessive amount of memory for your registry.

Upvotes: 1

Related Questions