Which locking primitive to use?

Question

Yesterday i discovered we had a multithreading issue with a simple caching object we use:

 If Dictionary.Contains(lsKey.ToLower) Then 'if rate cached, then return value
      lvResult = Dictionary.Item(lsKey.ToLower)

  Else 'else retrieve from database, store, and return value
      lvResult = GetRateFromDB(voADO,
                               veRateType,
                               vdEffDate)
      Dictionary.Add(lsKey.ToLower, lvResult)

  End If

We discovered this problem on our asp.net website. The error messaged read something like "your trying to add a value to a hashtable that already exists. As you can tell from the code above the potential definitely exits for this to happen. I was somewhat familiar with waithandles and thought they would solve the problem. So i declared my waithandle at the class level:

private Shared _waitHandle as new AutoResetEvent(True)

Then in the specific section of code with the problem:

_waitHandle.Wait()
If Dictionary.Contains(lsKey.ToLower) Then 'if rate cached, then return value
    lvResult = Dictionary.Item(lsKey.ToLower)

Else 'else retrieve from database, store, and return value
    lvResult = GetRateFromDB(voADO,
                             veRateType,
                             vdEffDate)
     Dictionary.Add(lsKey.ToLower, lvResult)
End If
_waitHandle.Set()

for some reason the following code above was ALWAYS blocked. Even the very first thread the accessed the code. I played with things for awhile and even tried to set the waithandle to signaled in the constructor but i never could get it to work.

I end up using the following instead which works fine:

SyncLock loLock
    If Dictionary.Contains(lsKey.ToLower) Then 'if rate cached, then return value
        lvResult = Dictionary.Item(lsKey.ToLower)

    Else 'else retrieve from database, store, and return value
        lvResult = GetRateFromDB(voADO,
                                 veRateType,
                                 vdEffDate)
        Dictionary.Add(lsKey.ToLower, lvResult)

    End If
End SyncLock

So I have two questions:

1. Why didn't the waithandle solution work?
2. Is SynLock the correct / optimized lock type to use in this case?

Answer 1

1 waithandles block until signaled. You would need something somewhere to signal that waithandle for it to not block the first thread to get access. I believe it would work if you had signaled the handle in the constructor where you created the waithandle. Consider it like there is a slot for a signal inside the waithandle any thread that calls wait will wait until it can consume a signal before leaving the wait call.

2 In this case it probably isn't the best lock to use, if two threads try to read a value that is already in the cache then one will be blocked until the other is finished. I would use a readerwriter lock instead. This way multiple threads can read the cache at the same time and you can upgrade to write when necessary.

If you don't mind multiple loads of the same value to the cache you could use a concurrentdictionary. Any thread that needed a value not yet loaded would load it then call tryadd. In the case where multiple threads try to access the same unloaded value at the same time all of them will do the work of calling GetRateFromDB.

Answer 2

Windows Terms:

• An "Event" allows one thread (or process) signal another thread (or process).
• A "Semaphore" is similar to an Event but it can be used to signal (wake) a specific number of threads.
• A "Critical Section" is used to prevent simultaneous access to a block of code.
• A "Slim Lock" is similar to a critical section but typically doesn't allow the thread which owns the lock to enter it multiple times (which is allowed using a critical section).
• A "Reader Writer" lock gives you the flexibility to lock an object in exclusive mode (similar to a critical section) or in a shared mode which would allow multiple threads to execute the same block.
• For completeness, there is also a "Mutex" which is very similar to a critical section, but can be shared across processes.

So using an AutoResetEvent above is definitely not what you want. When a thread calls one of the Wait methods it will block until it receives a signal from another thread. Nobody ever signals you, so you wait forever.

The SyncLock statement uses a critical section under the covers and prevents any thread from entering the same chunk of code at the same time. This will give you the protection you need. But because you need to protect all accesses to the Dictionary object to avoid corruption, you need to use the lock everywhere you use the Dictionary object.

As already stated, the ConcurrentDictionary is a good thing to use in this case as it essentially has a highly tuned Reader Writer lock built into it. So then you don't have to go and add a bunch of locks throughout your code base. But as noted in my comment, you can still have race conditions when using a ConcurrentDictionary.