Global thread-safe multi-value custom Dictionary with single instantiation

Question

I would like to have a global object similar to a multi-value Dictionary that is shared among different Threads.

I would like the object to be created only once (for example getting the data from a Database) and then used by the different Threads.

The Object should be easily extendable with additional properties (currently have only JobName and URL).

If possible, I would prefer to avoid locking.

I am facing the following issues:

• The current version displayed below is not Thread safe;
• I cannot use a ConcurrentDictionary since I have extended the Dictionary object to allow multiple values for each key;

This is the object structure that should be modified easily:

    public struct JobData
    {
        public string JobName;
        public string URL;
    }

I have extended the Dictionary object to allow multiple values for each key:

    public class JobsDictionary : Dictionary<string, JobData>
    {
        public void Add(string key, string jobName, string url)
        {
            JobData data;
            data.JobName = jobName;
            data.URL = url;
            this.Add(key, data);
        }
    }

Static class that is shared among Threads. As you can see it creates a Dictionary entry for the specific Job the first time it is called for that Job.

For instance, the first time it is called for "earnings" it will create the "earnings" dictionary entry. This creates issues with Thread safety:

public static class GlobalVar
{
    private static JobsDictionary jobsDictionary = new JobsDictionary();
    public static JobData Job(string jobCat)
    {   
        if (jobsDictionary.ContainsKey(jobCat))
            return jobsDictionary[jobCat];
        else
        {
            String jobName;
            String url = null;

            //TODO: get the Data from the Database
            switch (jobCat)
            {
                case "earnings":
                    jobName="EarningsWhispers";
                    url = "http://www.earningswhispers.com/stocks.asp?symbol={0}";
                    break;
                case "stock":
                    jobName="YahooStock";
                    url = "http://finance.yahoo.com/q?s={0}";
                    break;
                case "functions":
                    jobName = "Functions";
                    url = null;
                    break;
                default:
                    jobName = null;
                    url = null;
                    break;
            }
            jobsDictionary.Add(jobCat, jobName, url);
            return jobsDictionary[jobCat];
        }
    }

In each Thread I get the specific Job property in this way:

//Get the Name
string JobName= GlobalVar.Job(jobName).JobName;

//Get the URL
string URL = string.Format((GlobalVar.Job(jobName).URL), sym);

How can I create a custom Dictionary that is "instantiated" once (I know it is not the right term since it is static...) and it is Thread-safe ?

Thanks

UPDATE

Ok, here is the new version.

I have simplified the code by removing the switch statement and loading all dictionary items at once (I need all of them anyway).

The advantage of this solution is that it is locked only once: when the dictionary data is added (the first Thread entering the lock will add data to the dictionary). When the Threads access the dictionary for reading, it is not locked.

It should be Thread-Safe and it should not incur in deadlocks since jobsDictionary is private.

public static class GlobalVar
{
    private static JobsDictionary jobsDictionary = new JobsDictionary();   
    public static JobData Job(string jobCat)
    {
        JobData result;
        if (jobsDictionary.TryGetValue(jobCat, out result))
            return result;

        //if the jobsDictionary is not initialized yet...
        lock (jobsDictionary)
        {
            if (jobsDictionary.Count == 0)
            {
                //TODO: get the Data from the Database
                jobsDictionary.Add("earnings", "EarningsWhispers", "http://www.earningswhispers.com/stocks.asp?symbol={0}");
                jobsDictionary.Add("stock", "YahooStock", "http://finance.yahoo.com/q?s={0}");   
                jobsDictionary.Add("functions", "Functions", null);  
            }
            return jobsDictionary[jobCat];
        }
    }
}

Answer 1

If you are populating the collection once, you don't need any locking at all, since a Dictionary is thread-safe when it is only read from. If you want prevent multiple threads from initializing multiple times you can use a double-checked lock during initalization, like this:

static readonly object syncRoot = new object();
static Dictionary<string, JobData> cache;

static void Initialize()
{
    if (cache == null)
    {
        lock (syncRoot)
        {
            if (cache == null)
            {
                cache = LoadFromDatabase();
            }
        }
    }
}

Instead of allowing every thread to access the dictionary, hide it behind a facade that only exposes the operations you really need. This makes it much easier to reason about thread-safety. For instance:

public class JobDataCache : IJobData
{
    readonly object syncRoot = new object();
    Dictionary<string, JobData> cache;

    public void AddJob(string key, JobData data)
    {
        lock (this.syncRoot)
        {
            cache[key] = data;
        }
    }
}

Trying to prevent locking without having measured that locking actually has a too big impact on performance is bad. Prevent doing that. Often using a simple lock statement is much simpler than writing lock-free code. There is a nasty problem with concurrency bugs compared to normal software bugs. They are very hard to reproduce and very hard to track down. If you can, prevent writing concurrency bugs. You can do this by writing the simplest code you can, even if it is slower. If it proves to be too slow, you can always optimize.

If you want to write lock-free code anyway, try using immutable data structures, or prevent changing existing data. This is one trick I used when writing the Simple Injector (a reusable library). In this framework, I never update the internal dictionary, but always completely replace it with a new one. The dictionary itself is therefore never changed, the reference to that instance is just replaced with a completely new dictionary. This prevents you from having to do locks completely. However, you must realize that it is possible to loose updates. In other words, when multiple threads are updating that dictionary, one can loose its changes, simply because each thread creates a new copy of that dictionary and adds its own value too its own copy, before making that reference public to other threads.

In other words, you can only use this method when external callers only read (and you can recover from lost changes, for instance by querying the database again).

UPDATE

Your updated version is still not thread-safe, because of the reasons I explained on @ili's answer. The following will do the trick:

public static class GlobalVar
{
    private static readonly object syncRoot = new object();
    private static JobsDictionary jobsDictionary = null;

    public static JobData Job(string jobCat)
    {
        Initialize();

        return jobsDictionary[jobCat];
    }

    private void Initialize()
    {
        // Double-checked lock.
        if (jobsDictionary == null)
        {
            lock (syncRoot)
            {
                if (jobsDictionary == null)
                {
                    jobsDictionary = CreateJobsDictionary();
                }
            }
        }
    }

    private static JobsDictionary CreateJobsDictionary()
    {
        var jobs = new JobsDictionary();

        //TODO: get the Data from the Database
        jobs.Add("earnings", "EarningsWhispers", "http://...");
        jobs.Add("stock", "YahooStock", "http://...");
        jobs.Add("functions", "Functions", null);

        return jobs;
    }
}

You can also use the static constructor, which would prevent you from having to write the double checked lock yourself. However, it is dangarous to call the database inside a static constructor, because a static constructor will only run once and when it fails, the complete type will be unusable for as long as the AppDomain lives. In other words your application must be restarted when this happens.

UPDATE 2:

You can also use .NET 4.0's Lazy<T>, which is safer than a double checked lock, since it is easier to implement (and easier to implement correctly) and is is also thread-safe on processor architectures with weak memory models (weaker than x86 such as ARM):

static Lazy<Dictionary<string, JobData>> cache =
    new Lazy<Dictionary<string, JobData>>(() => LoadFromDatabase());

Answer 2

1) Use singleton patern to have one instance (one of the ways is to use static class as you have done)

2) To make anything thread safe you should use lock or it's analog. If you are afraids of unnessessary locks do like this:

public object GetValue(object key)
{
    object result;
    if(_dictionary.TryGetValue(key, out result) 
        return result;

     lock(_dictionary)
     {
        if(_dictionary.TryGetValue(key, out result) 
            return result;
        //some get data code
        _dictionary[key]=result;
        return result;
     }
}