I have run into a problem in an application that runs numerous (25+) threads concurrently (the code runs in a BizTalk transmit adapter), using CSLA BO's. At startup, the following exception occurs for the first creation of a specific BO.
Exception DataPortal.Fetch failed (System.Reflection.TargetInvocationException: Exception has been thrown by the target of an invocation. ---> System.ArgumentException: An item with the same key has already been added.
at System.ThrowHelper.ThrowArgumentException(ExceptionResource resource)
at System.Collections.Generic.Dictionary`2.Insert(TKey key, TValue value, Boolean add)
at Csla.Security.SharedAuthorizationRules.GetManager(Type objectType, Boolean create) in C:\c\Common\csla2.1.4\csla20cs\Csla\Security\SharedAuthorizationRules.cs:line 36
at Csla.Security.AuthorizationRules.HasReadAllowedRoles(String propertyName) in C:\c\Common\csla2.1.4\csla20cs\Csla\Security\AuthorizationRules.cs:line 226
at Csla.Core.BusinessBase.CanReadProperty(String propertyName) in C:\c\Common\csla2.1.4\csla20cs\Csla\Core\BusinessBase.cs:line 460
at Csla.Core.BusinessBase.CanReadProperty(String propertyName, Boolean throwOnFalse) in C:\c\Common\csla2.1.4\csla20cs\Csla\Core\BusinessBase.cs:line 406
The CSLA code looks like this:
internal static AuthorizationRulesManager GetManager(Type objectType, bool create)
{
AuthorizationRulesManager result = null;
if (!_managers.TryGetValue(objectType, out result) && create)
{
lock (_managers)
{
result = new AuthorizationRulesManager();
_managers.Add(objectType, result);
}
}
return result;
}
What is happening seems to be that two threads passes the if (TryGetValue()) at the same time (it is a dual core processor, so we do have two threads running at exactly the same time), one of them reaches the lock first, and adds the new result object. Meanwhile the other thread is waiting for the lock, and when the lock is released, tries to add the very same thing, resulting in the above exception.
I see two possible solutions:
1. Move the lock so that in encapsulates the TryGetValue() call. This would induce the overhead of always aquiring a lock.
2. Encapsulate the _managers.Add() call in a try-catch clause that catches and ignores any exceptions thrown. This would not induce any overhead, but looks like the ugly kludge it is.
Comments?
Regards;
/jb
A quick search in the CSLA project reveals three different ways locking implemented in the framework:
1. With the risk for a potential exception
if (!_managers.TryGetValue(objectType, out result) && create)
{
lock (_managers)
{
result = new ValidationRulesManager();
_managers.Add(objectType, result);
}
}
2. With locking overhead
lock (_syncClientContext)
{
HybridDictionary ctx = GetClientContext();
if (ctx == null)
{
ctx = new HybridDictionary();
SetClientContext(ctx);
}
return ctx;
}
3. With an extra safeguard
if (applySort && !_sorted)
{
lock (_list)
{
if (applySort && !_sorted)
{
_list.Sort();
_sorted = true;
}
}
}
Seems to me that the last one is the optimal solution, as it only uses a lock when necessary, but still avoids duplicate work by rechecking the criteria.
Regards;
/jb
We're having similar issues with this method as well. We get intermitent exceptions since it's running in a multi-threaded environment with a high volume of requests.
Personally, I'm thinking that moving the lock statement before the
if (!_managers.TryGetValue(objectType, out result) && create)
line would suffice. Locking has less of an overhead than one might think and it makes things much simpler.
Rocky, have you had a chance to look at this issue yet? I know you've made changes to this class in the past to better support multi-threading but perhaps overlooked this edge case?
~pete
JBergdahl:
Seems to me that the last one is the optimal solution, as it only uses a lock when necessary, but still avoids duplicate work by rechecking the criteria.
There are good reasons for using 2 and 3 in different locations. Number 1 is clearly a bug, and where that occurs it should be changed (probably) to use technique 3.
The two cases where the race condition existed have been changed now in version 3.0.4 and 3.5. Both sets of code are in svn at the moment, so if you could let me know if this addresses the issue that'd be really helpful.
Copyright (c) Marimer LLC