Implementation of asynchronous (?), self-refreshing cache of object collection bulk

Question

I'm trying to create a caching solution used in various parts of a clients' application. The resulting code will be open source.

For example, it will be used for caching API queries like "get all users" or "get all projects", whatever "all" may mean to the consumer (the constructor accepts a lambda as a "getter function".

The object exposes Get(IdType id) and GetAll() methods for querying the cache.

When instantiating the cache, nothing is retrieved initially (retreiving at once may be beneficial, though). Retrieving is done when calling Get(IdType id) or GetAll() for the first time.

These methods are synchronous and stop execution for however long the "getter function" takes, but adding async versions of these would be a good idea, I think.

Also, the constructor accepts a TimeSpan object for controlling if and how frequently the object collection bulk should be updated from the getter function. Here, a key point is that as long the updating getter function is not done, consumers will be able to retrieve the old objects. When the getter function is done, the instances are updated.

I'd like some thoughts about this approach, and if something similar already exists, I'd love to know. Also, I'm not that experienced at all in designing multi-threaded functionality, so any feedback regarding this would be appreciated.

It would be optimal to have some kind of expert to help out and make this a real open source library.

I have three test cases:

• Should wait for blocking Get(IdType id) or GetAll() method (first call to either one should "block thread execution" - async versions (non existent as of today) should leave a Task)
• Should let two concurrent get calls wait for first getter function run (essentially same as above)
• Should refresh asynchronously (as in, let consumers have the old objects as new ones are fetched)

Here's the code so far:

using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading;
using System.Threading.Tasks;
namespace OrganicCache
{
 public class CollectivisticCache<InstanceType> : CollectivisticCache<string, InstanceType> {
 public CollectivisticCache(Func<InstanceType, string> idFunction, Func<IEnumerable<InstanceType>> getterFunction, TimeSpan getterFunctionWaitPeriod) : base(idFunction, getterFunction, getterFunctionWaitPeriod) { }
 public CollectivisticCache(Func<InstanceType, string> idFunction, Func<IEnumerable<InstanceType>> getterFunction) : base(idFunction, getterFunction) { }
 }
 public class CollectivisticCache<IdType, InstanceType>
 {
 public CollectivisticCache(Func<InstanceType, IdType> idFunction, Func<IEnumerable<InstanceType>> getterFunction, TimeSpan getterFunctionWaitPeriod)
 : this(idFunction, getterFunction)
 {
 _getterFunctionWaitPeriod = getterFunctionWaitPeriod;
 }
 public CollectivisticCache(Func<InstanceType, IdType> idFunction, Func<IEnumerable<InstanceType>> getterFunction)
 {
 _getterFunction = getterFunction;
 _idFunction = idFunction;
 }
 #region First run of getter function
 private bool _hasRunGetterFunctionFirstTime;
 private object _hasRunGetterFunctionFirstTimeLock = new object();
 private bool _isRunningGetterFunctionFirstTime;
 private object _isRunningGetterFunctionFirstTimeLock = new object();
 private Task _getterFunctionFirstRunTask;
 #endregion
 private Func<IEnumerable<InstanceType>> _getterFunction;
 private TimeSpan _getterFunctionWaitPeriod;
 private bool _isWaitingToRunGetterFunction;
 private object _isWaitingToRunGetterFunctionLock = new object();
 private Func<InstanceType, IdType> _idFunction;
 private ConcurrentDictionary<IdType, InstanceType> _instances = new ConcurrentDictionary<IdType, InstanceType>();
 private object _instancesLock = new object();
 public ICollection<InstanceType> GetAll()
 {
 assertGetterFunctionHasRunFirstTime();
 return _instances.Values;
 }
 public InstanceType Get(IdType id)
 {
 assertGetterFunctionHasRunFirstTime();
 InstanceType instance;
 _instances.TryGetValue(id, out instance);
 return instance;
 }
 private void assertGetterFunctionHasRunFirstTime()
 {
 if (!_hasRunGetterFunctionFirstTime)
 {
 lock (_hasRunGetterFunctionFirstTimeLock)
 {
 if (!_hasRunGetterFunctionFirstTime)
 {
 if (!_isRunningGetterFunctionFirstTime)
 {
 lock (_isRunningGetterFunctionFirstTimeLock)
 {
 if (!_isRunningGetterFunctionFirstTime)
 {
 _isRunningGetterFunctionFirstTime = true;
 _getterFunctionFirstRunTask = Task.Run(() => runGetterFunction());
 }
 }
 }
 Task.WaitAll(_getterFunctionFirstRunTask);
 _isRunningGetterFunctionFirstTime = false;
 _hasRunGetterFunctionFirstTime = true;
 return;
 }
 }
 }
 }
 private void runGetterFunction()
 {
 var instances = _getterFunction();
 foreach (var instance in instances)
 {
 var id = _idFunction(instance);
 lock (_instancesLock)
 {
 _instances.AddOrUpdate(id, instance, (_, __) => instance);
 }
 }
 runGetterFunctionAfterWaitPeriod();
 }
 private void runGetterFunctionAfterWaitPeriod()
 {
 if (_getterFunctionWaitPeriod == null)
 {
 return;
 }
 if (!_isWaitingToRunGetterFunction)
 {
 lock (_isWaitingToRunGetterFunctionLock)
 {
 if (!_isWaitingToRunGetterFunction)
 {
 _isWaitingToRunGetterFunction = true;
 Task.Run(() =>
 {
 Thread.Sleep(_getterFunctionWaitPeriod);
 runGetterFunction();
 _isWaitingToRunGetterFunction = false;
 });
 }
 }
 }
 }
 }
}

Answer 1

I'm assuming InstanceType and IdType are custom classes? Or are they supposed to be generic type parameters -- because if they are, the naming rule expects them to start with T.

---

These can all be made readonly: _hasRunGetterFunctionFirstTimeLock, _isRunningGetterFunctionFirstTimeLock, _getterFunction, _getterFunctionWaitPeriod, _isWaitingToRunGetterFunctionLock, _idFunction, _instances, _instancesLock.

---

assertGetterFunctionHasRunFirstTime(), runGetterFunction(), runGetterFunctionAfterWaitPeriod() should be PascalCase.

---

I don't think _getterFunctionWaitPeriod can be null, since TimeSpan is a struct -- if (_getterFunctionWaitPeriod == null). Should it be a nullable instead?

---

The arrow anti pattern is also present in RunGetterFunctionAfterWaitPeriod(), which can be rewritten as:

private void RunGetterFunctionAfterWaitPeriod()
{
 if (_getterFunctionWaitPeriod == null)
 {
 return;
 }
 if (_isWaitingToRunGetterFunction)
 {
 return;
 }
 lock (_isWaitingToRunGetterFunctionLock)
 {
 if (_isWaitingToRunGetterFunction)
 {
 return;
 }
 _isWaitingToRunGetterFunction = true;
 Task.Run(() =>
 {
 Thread.Sleep(_getterFunctionWaitPeriod);
 runGetterFunction();
 _isWaitingToRunGetterFunction = false;
 });
 }
}

• user77309: So TIdType and TInstanceType? I myself am using only string and sometimes int for IdType, should it be constrained as a : struct perhaps?
• user77309: Or perhaps TId and TInstance it is.
• user77309: Regarding the TimeSpan, I found this stackoverflow.com/questions/1225949/… but I have changed it into a Nullable nevertheless.
• BCdotWEB: @BjörnAliGöransson Didn't know that about the implicit conversion. Odd that ReSharper "complained" about (_getterFunctionWaitPeriod == null) always being false.

Answer 2

This

private void assertGetterFunctionHasRunFirstTime()
{
 if (!_hasRunGetterFunctionFirstTime)
 {
 lock (_hasRunGetterFunctionFirstTimeLock)
 {
 if (!_hasRunGetterFunctionFirstTime)
 {
 if (!_isRunningGetterFunctionFirstTime)
 {
 lock (_isRunningGetterFunctionFirstTimeLock)
 {
 if (!_isRunningGetterFunctionFirstTime)
 {
 _isRunningGetterFunctionFirstTime = true;
 _getterFunctionFirstRunTask = Task.Run(() => runGetterFunction());
 }
 }
 }
 Task.WaitAll(_getterFunctionFirstRunTask);
 _isRunningGetterFunctionFirstTime = false;
 _hasRunGetterFunctionFirstTime = true;
 return;
 }
 }
 }
} 

is a good example of the arrow anti pattern. By inverting the conditions and returning early this can be refactored like so

private void assertGetterFunctionHasRunFirstTime()
{
 if (_hasRunGetterFunctionFirstTime) { return; }
 lock (_hasRunGetterFunctionFirstTimeLock)
 {
 if (_hasRunGetterFunctionFirstTime) { return; }
 if (_isRunningGetterFunctionFirstTime) { return; }
 lock (_isRunningGetterFunctionFirstTimeLock)
 {
 if (!_isRunningGetterFunctionFirstTime)
 {
 _isRunningGetterFunctionFirstTime = true;
 _getterFunctionFirstRunTask = Task.Run(() => runGetterFunction());
 Task.WaitAll(_getterFunctionFirstRunTask);
 _isRunningGetterFunctionFirstTime = false;
 _hasRunGetterFunctionFirstTime = true;
 return;
 }
 }
 }
} 

I have now placed the call to Task.WaitAll() inside the lock because you are waiting nevertheless.

• BCdotWEB: Shouldn't the break; be a return;?
• Heslacher: @BCdotWEB no, because in the original code after exiting the lock it will call Task.WaitAll(...) ...
• BCdotWEB: I don't think you can use break; in that place; AFAIK break is only used in the context of loops or switch statements. My VS complains of an "unresolved jump" when I copy-paste your code.
• Heslacher: You are right. I didn't test this (my bad). I will update my answer soon.
• Heslacher: Updated answer.