HiveBrain v1.2.0
Get Started
← Back to all entries
patterncsharpMinor

Static ConcurrentDictionary to maintain static objects

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
concurrentdictionaryobjectsmaintainstatic

Problem

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> m_InstancesCached = new ConcurrentDictionary>();
    private static readonly Object _entriesLock = new object();

    public List GeKeysFromCache(string cacheKey)
    {
        return m_InstancesCached.GetOrAdd(cacheKey, (key) =>
        {
            return new List();
        });
    }

    public void AddKeysToCache(string cacheKey, List 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();

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();

for (int i = 0; i
{
//lock (_entriesLock)
//{
var instanceFields2 = (new CacheHelper()).GeKeysFromCache(instanceName);
instanceFields2 = instanceFields2 ?? new List();

for (int i = 10000; i

Solution

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

    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()).ToList();

Code Snippets

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

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

Context

StackExchange Code Review Q#158493, answer score: 2

Revisions (0)

No revisions yet.