0
\$\begingroup\$

There were multiple times in different applications that I needed to accomplish the following behavior with C# Task and I did it in a certain way, and would like to receive an insight whether it's the best way to achieve the desired effect, or there are other better ways.

The issue is that in certain circumstances I would like a specific Task to exist only in one instance. For example, if someone requests, let's say a list of products by executed a method like Task GetProductsAsync(), and someone else tries to request the same thing, it wouldn't fire another task, but rather return already existing task. When the GetProductsAsync finishes, all of those callers who had previously requested the result will receive the same result. So, there should ever be only one GetProductsAsync execution at a given point of time.

After failed trials to find something similar and well known design pattern to solve this issue, I came up with my own implementation:

public class TaskManager : ITaskManager
    {
        private readonly object _taskLocker = new object();
        private readonly Dictionary<string, Task> _tasks = new Dictionary<string, Task>();
        private readonly Dictionary<string, Task> _continuations = new Dictionary<string, Task>();

        public Task<T> ExecuteOnceAsync<T>(string taskId, Func<Task<T>> taskFactory)
        {
            lock(_taskLocker)
            {

                if(_tasks.TryGetValue(taskId, out Task task))
                {
                    if(!(task is Task<T> concreteTask))
                    {
                        throw new TaskManagerException($"Task with id {taskId} already exists but it has a different type {task.GetType()}. {typeof(Task<T>)} was expected");
                    }
                    else
                    {
                        return concreteTask;
                    }
                }
                else
                {
                    Task<T> concreteTask = taskFactory();
                    _tasks.Add(taskId, concreteTask);
                    _continuations.Add(taskId, concreteTask.ContinueWith(_ => RemoveTask(taskId)));
                    return concreteTask;
                }
            }
        }

        private void RemoveTask(string taskId)
        {
            lock(_taskLocker)
            {
                if(_tasks.ContainsKey(taskId))
                {
                    _tasks.Remove(taskId);
                }

                if(_continuations.ContainsKey(taskId))
                {
                    _continuations.Remove(taskId);
                }
            }
        }
    }

The idea is that we will have a single instance of TaskManager throughout the application lifetime. Any async Task request that should be executed only once at a given point in time, will call ExecuteOnceAsync providing the factory method to create the Task itself, and desired application wide unique ID. Any other task that will come in with the same ID, the Task manager with reply with the same instance of Task created before. Only if there are no other Tasks with that ID, the manager will call the factory method and will start the task. I have added locks around code task creation and removal, to ensure thread safety. Also, in order to remove the task from the stored dictionary after Task has been completed, I've added a continuation task using ContinueWith method. So, after a task has been completed, both the task itself, and its continuation will be removed.

From my side this seems to be a pretty common scenario. I would assume there is a well established design pattern, or perhaps C# API that accomplishes this exact same thing. So, any insights or suggestions will be very appreciated.

\$\endgroup\$
  • \$\begingroup\$ I'm not sure it's such a good idea... what if one method that requested a list of something and another one too and the first one modifies it so it will also be modified for the second method... this would get nasty pretty quickly. Usually we protect resources from being accessed at the same time, not returning the same result to multiple callers... unless the results are immutable I wouldn't want to use this manager or look for bugs it causes. \$\endgroup\$ – t3chb0t Feb 7 at 15:54
  • \$\begingroup\$ @t3chb0t Thanks for the reply. In our case we do need to return the same result if asked multiple times at the same time, instead of just protecting the resource from getting accessed simultaneously. So, I do need this type of behavior. \$\endgroup\$ – kyurkchyan Feb 8 at 7:47
  • \$\begingroup\$ Well, this looks much more like you need a cache... \$\endgroup\$ – t3chb0t Feb 8 at 7:48
  • \$\begingroup\$ Good point, I thought about that, but then I realized it's not cache either. Cause when all requests are completed, if someone tries to do the same request, it should be re-executed, instead of being fetched from cache. \$\endgroup\$ – kyurkchyan Feb 8 at 7:50
  • \$\begingroup\$ Could you reveal your real use case scenario? I think this way you would get much better feedback if we knew what we are actually solving here. \$\endgroup\$ – t3chb0t Feb 8 at 7:51
1
\$\begingroup\$

What's the point of _continuations? I can see three options which use a single dictionary:

  1. Just store and return concreteTask. If this fails because the continuation is GC'd, document this for the benefit of maintainers.
  2. Just store and return the continuation. If this fails because the continuation is executed multiple times, document it.
  3. Store the task and return the continuation, so the first caller has the hard reference. I can't think how this could fail unless the caller actively discards the task.

TL;DR: if there are obvious ways of simplifying the code which break for non-obvious reasons, document the reasons so that the maintainer doesn't break it.


A further advantage of using a single dictionary would be that you could use ConcurrentDictionary instead of the manual locking. (In fact you could do this even if you do need both copies of the task, using ConcurrentDictionary<string, Tuple<Task, Task>>). The code might be as simple as

    public class TaskManager : ITaskManager
    {
        private readonly ConcurrentDictionary<string, Task> _tasks = new ConcurrentDictionary<string, Task>();

        public Task<T> ExecuteOnceAsync<T>(string taskId, Func<Task<T>> taskFactory)
        {
            var task = _tasks.GetOrAdd(
                taskId,
                id => taskFactory().ContinueWith(_ => _tasks.TryRemove(taskId, out var _)));
            return (task as Task<T>) ??
                throw new Exception($"Task with id {taskId} already exists but it has a different type {task.GetType()}. {typeof(Task<T>)} was expected");
        }
    }

I would like to add: I like the level of detail in the exception message. That should be very helpful in tracking down bugs if it is thrown and logged.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.