Will code produce a race condition?

Question

I have a file with a base path to all my resources. For example:

build/scripts/script1.js  
build/scripts/script2.js

I of course need a base path for example:

https://example.org/SuperDuperSite/build/scripts/script1.js

What i was hoping to do was load the file into a global dictionary at startup with the path. The dictionary would only have to be loaded once. Unfortunately, as far as i can tell, the base path is not available until the first request. So in asp.net i have to use application_beginrequest rather than application_start. What is bad about this is now I have to deal with multithreading issues.

which forces me to write the following type of code:

  lock(_lock) {
    if (_dictionary == null) {
        LoadDictionary();
    }
  }

This would be called for every single request when I only really need to load it once. I really don't like this of course. I don't want to have to lock on every single request for performance reasons. One solution, after talking to colleges that we came up with:

if (_dictionary == null)
{
   lock(_lock) {
      if (_dictionary == null) {
         LoadDictionary();
      }
    }
}

So with this solution I would not be required to lock on every single request but if multiple threads ended up getting this section on startup I would then protect it by checking if the object is null again inside the lock. Will this code work or am I going to run into a race condition?

Answer 1

Be careful when using double-checked locks.

Yes, it's thread-safe, but in your particular code you might run into a subtle bug where _dictionary has been instantiated by the other thread (passing the null check), but not fully populated yet, and you may end up attempting to access a partially populated dictionary. And end up with one of these two:

1. You're missing results when reading the dictionary, or worse
2. Unless you're using a ConcurrentDictionary (which also comes with its performance implications), you may be reading and writing simultaneously to the dictionary, and causing a deadlock (yes, the Dictionary class is known to cause deadlocks in many a code).

A bool _dictionaryLoaded flag (double-checked), flipped to true at the end of LoadDictionary(), is probably better.

Or use Lazy<> if you're on .NET 4. It's much cleaner, all you have to do is pass in LoadDictionary to be used as the init function.

Edit: Lazy<> is internally implemented with a double-checked lock.