TypeModel threading issues

There are a few threading issues with the TypeModel class. In specific the method static GetTypeModel has some concurrency issues.

The problem occurs when two threads try to query using the same TypeModel at the same time. There are two exceptions that can occur when this happens.

Problem 1:
Path component [Property] not found in type [Entity]

This occurs because TypeModels are added to _typeModels before they are fully initialized.

Problem 2:
Collection was modified; enumeration operation may not execute.

This occurs because you are accessing the dictionary outside of the lock. Unfortunately dictionaries are not thread safe.

From MSDN (http://msdn.microsoft.com/en-us/library/xfhwa508.aspx):

A Dictionary<(Of <(TKey, TValue>)>) can support multiple readers concurrently, as long as the collection is not modified. Even so, enumerating through a collection is intrinsically not a thread-safe procedure. In the rare case where an enumeration contends with write accesses, the collection must be locked during the entire enumeration. To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.

The easiest solution that fixes both of these problems is to remove the first TryGetValue so that you are locking every time you access the dictionary. I also have a very simple test project that reproduces both of these problems. If there is a place that I can email or upload it to let me know.

Is this a known issue? Is there a known work around for these issues? We are currently using LightSpeed 2.2.

Hello Todd,

It's not an issue we've seen before, so we don't have a proven workaround.  However I believe you should be able to work around it by forcing all types to initialise on a single thread.

For example, during application startup, within a guarded section (e.g. using lock), create an instance of each entity type.  You can throw the instance away again immediately -- the idea is just to get LightSpeed to initialise all of the type models before multiple threads can kick in.  To reduce maintenance risk, you could use reflection to automatically locate and instantiate all Entity-derived types.

We will investigate a fix -- if you could provide us with your repro case then that would speed up resolution.  You can attach a zip file via the Options tab or email it via the contact link (please remove all binaries first).  Thanks!

Here is the simple repro project. As is true of most concurency issues it is not guaranteed to happen on every run. So you will probably need to run the project a few times to see the crash. Also, you will most likely only run into Problem 1. I only get problem 2 about once in every 100 runs.

Unfortunately loading all of the models is not a great option, because we have a number of LightSpeed databases in our app.

If you have any questions let me know.

Thanks Todd.  A candidate fix has been committed and should be available in current nightly builds.  Please let us know if you still see problems. 

The TypeModel.GetTypeModel static method still appears to be unsafe for simultaneous use by multiple threads. Consider threads A & B calling the method at once. Thread A calls _typeModelLock.WaitOne() then _typeModelLock.ReleaseMutex(). Thread B does the same. Now Thread A calls TryGetValue which returns false, so Thread A proceeds through the lock and the if statements. Thread A takes the mutex again and eventually calls LoadType which calls WalkHierarchy which calls _typeModels.Add. While Thread A is in Add(), Thread B calls TryGetValue (in the if statement); it has already acquired and released the mutex, so there's nothing stopping it from executing this code. This is a synchronization problem, because Dictionary<> is not safe for simultaneous access on two threads. (“Any instance members are not guaranteed to be thread safe. … A Dictionary(TKey, TValue) can support multiple readers concurrently, as long as the collection is not modified. … To allow the collection to be accessed by multiple threads for reading and writing, you must implement your own synchronization.” http://msdn.microsoft.com/en-us/library/xfhwa508.aspx). The simplest solution is to remove the mutex, remove the outermost if statement in GetTypeModel (i.e., the “double-checked locking” pattern), and always lock on _typeModels before calling any method on it. (We can send a suggested patch if desired.) There will be some contention between concurrent readers, but lock (i.e., Monitor.Enter) is very lightweight (especially compared to the cost of always taking a Mutex, which incurs a kernel mode transition) and Dictionary lookups are generally very fast. This would allow TypeModel to follow the standard .NET pattern of having all static methods being thread-safe. Thanks, Bradley

Thanks for the precise feedback Bradley - most appreciated  You must be reading our minds *grin* - the update with the Mutex was a temporary fix to solve that specific problem while we were investigating some other scenarios. After those investigation the simplest approach is as you have suggested, to simply lock on _typeModels at the outer scope so we have refactored to this today and this will be in the subsequent builds.

 

Jeremy

Thanks for the update; we look forward to the new drop.

(Sorry about the formatting in the last post; it was nicely formatted on my screen when I clicked the "Post" button, but somehow got lost.)

Thanks,
Bradley