Static ConcurrentDictionary to maintain static objects

Question

I am using following code to maintain some static information. The problem I see with it is that, if the information retrieved using GeKeysFromCache is modified without using lock keyword it may lead to exceptions in a multithreaded environment. Is there a way to improve this implementation?

public class CacheHelper
{
 private static ConcurrentDictionary<string, List<InstanceField>> m_InstancesCached = new ConcurrentDictionary<string, List<InstanceField>>();
 private static readonly Object _entriesLock = new object();
 public List<InstanceField> GeKeysFromCache(string cacheKey)
 {
 return m_InstancesCached.GetOrAdd(cacheKey, (key) =>
 {
 return new List<InstanceField>();
 });
 }
 public void AddKeysToCache(string cacheKey, List<InstanceField> inputs)
 {
 m_InstancesCached.AddOrUpdate(cacheKey, inputs, (key, oldValue) =>
 {
 lock (_entriesLock)
 {
 oldValue.AddRange(inputs);
 return oldValue;
 }
 });
 }
}

Test Code: Without lock keyword it fails

public class ThreadSafeCachingTest
{
 private static readonly Object _entriesLock = new object();
 public ThreadSafeCachingTest()
 {
 }
 public void Invoke()
 {
 var thread1Completed = false;
 var thread2Completed = false;
 var thread3Completed = false;
 var threadList = new List<Thread>();
 var instanceName = "MyInstance";
 var beforeCachingData = (new CacheHelper()).GeKeysFromCache(instanceName);
 threadList.Add(new Thread(() =>
 {
 //lock (_entriesLock)
 //{
 var instanceFields1 = (new CacheHelper()).GeKeysFromCache(instanceName);
 instanceFields1 = instanceFields1 ?? new List<InstanceField>();
 for (int i = 0; i < 10000; i++)
 {
 instanceFields1.Add(new InstanceField() { FieldName = "FieldName-" + i.ToString() });
 }
 thread1Completed = true;
 //}
 }));
 threadList.Add(new Thread(() =>
 {
 //lock (_entriesLock)
 //{
 var instanceFields2 = (new CacheHelper()).GeKeysFromCache(instanceName);
 instanceFields2 = instanceFields2 ?? new List<InstanceField>();
 for (int i = 10000; i < 20000; i++)
 {
 instanceFields2.Add(new InstanceField() { FieldName = "FieldName-" + i.ToString() });
 }
 thread2Completed = true;
 //}
 }));
 threadList.Add(new Thread(() =>
 {
 //lock (_entriesLock)
 //{
 while (!(thread1Completed && thread2Completed && thread3Completed))
 {
 var instanceFields3 = (new CacheHelper()).GeKeysFromCache(instanceName);
 if (instanceFields3.Count > 0)
 {
 instanceFields3.RemoveAt(0);
 }
 else
 {
 thread3Completed = true;
 }
 //}
 }
 }));
 foreach (Thread t in threadList) t.Start();
 foreach (Thread t in threadList) t.Join();
 var afterCachingData = (new CacheHelper()).GeKeysFromCache(instanceName);
 }
}
public class InstanceField
{
 public string FieldName { get; set; }
}

• 404: I've never used a ConcurrentDictionary, but presumably all its methods are thread safe. So are you actually asking about making modifications to a List<InstanceField> in a thread-safe way?
• Aashish Upadhyay: @eurotrash Yes.

Answer 1

public List<InstanceField> GeKeysFromCache(string cacheKey)
{
 var resultSet = m_InstancesCached.GetOrAdd(cacheKey, (key) =>
 {
 return new List<InstanceField>();
 });
 return resultSet.ToList();
}

Why so many returns and when you can simply do this with only one return and without the lambda:

return m_InstancesCached.GetOrAdd(cacheKey, new List<string>()).ToList();

Answer 2

What I was missing was something very trivial, adding a ToList() to GeKeysFromCache makes it ThreadSafe

public class CacheHelper
{
 private static ConcurrentDictionary<string, List<InstanceField>> m_InstancesCached = new ConcurrentDictionary<string, List<InstanceField>>();
 private static readonly Object _entriesLock = new object();
 public List<InstanceField> GeKeysFromCache(string cacheKey)
 {
 var resultSet = m_InstancesCached.GetOrAdd(cacheKey, (key) =>
 {
 return new List<InstanceField>();
 });
 lock (_entriesLock)
 {
 return resultSet.ToList();
 }
 }
 public void AddKeysToCache(string cacheKey, List<InstanceField> inputs)
 {
 m_InstancesCached.AddOrUpdate(cacheKey, inputs, (key, oldValue) =>
 {
 lock (_entriesLock)
 {
 oldValue.AddRange(inputs);
 return oldValue;
 }
 });
 }
}