We have a class that inherits from ReadOnlyBase<T> (CSLA.NET version 2.1.4). In our ASP.NET web application only a single instance of this class exists, which is being cached. Therefore, it's a Singleton instance.
Under heavy load conditions, we're noticing an exception that gets thrown periodically when the CanReadProperty method is called on certain properties. The exception is:
System.Collections.Generic.KeyNotFoundException: The given key was not present in the dictionary
And it occurs at the red line shown below within this CanReadProperty method overload in the ReadOnlyBase<T> class:
public virtual bool CanReadProperty(string propertyName)
{
bool result = true;VerifyAuthorizationCache();
if (_readResultCache.ContainsKey(propertyName))
{
// cache contains value - get cached value
result = _readResultCache[propertyName];
}
else
{
This seems like a threading issue and after doing some digging into the CSLA.NET source code I see that the _readResultCache private field (of type Dictionary<string,bool>) is used by ReadOnlyBase<T> to cache authorization rule results by property name. However, that dictionary object is being cleared by the VerifyAuthorizationCache method whenever the current user (IPrincipal) changes. In a multi-threaded web environment where each request potentially has a different IPrincipal instance, this clearing of the readResultCache object is happenning quite frequently. And if one thread clears it just before another thread executes the above line of code, I can see where the KeyNotFoundException would occur.
I know Rocky did some bug-fixing in the SharedAuthorizationRules class (which gets called if a property authorization result is not in the _readResultCache) which fixed some race conditions related to multi-threaded environments. We've actually patched our CLSA.NET 2.1.4 codebase with this fix, which fixed a past issue we had.
However, this problem is a different issue. Is there a known work-around or perhaps another bug fix? I looked into the SVN repository for the ReadOnlyBase<T> and didn't see anything too promising in the revisions after the 2.1.4 release. If you go past the CSLA.NET 3.0 revisions, Rocky has completely rewritten the property infrastructure altogether.
If I can't effectively patch our version of CSLA.NET based on existing code in the SVN repository, I'm considering the following options:
Thoughts?
CSLA objects are not threadsafe. That's not a design goal either, so it isn't likely to change.
Like with .NET itself I try to make sure the static/Shared methods are threadsafe, but instance methods/properties are absolutely not threadsafe.
I'll offer another alternative though:
4. Since this is a singleton, make sure all threads access it through a singleton accessor, and use locking in that accessor to ensure that only one thread can use the singleton object at a time - thus meeting the threading constraints of CSLA. This should make it work - but I suppose will still eliminate the benefit of the authorization results caching that would otherwise occur.
You have a good point and I think the alternative you've suggested is what I will do.
Hold on. I take that back. Putting lock code around the Singleton accessor will not solve the problem since that will only ensure that the retrieval of the object is thread-safe. But once the caller has the object instance, calling read-only properties on that object will still not be thread-safe.
Putting lock statements around each property getter in your business object doesn't do the trick either since thread 1 could be calling PropertyA and thread 2 could be calling PropretyB and clear the authorization results cache just before thread 1 hits the line shown in red in the original post and we'll get the KeyNotFoundException again.
Locking needs to be done around the instance itself and all interactions with that instance. The problem is the only place that could be done would be in the caller's code. Providing the caller with a nice Singleton read-only business object, but then requiring them to put locking code around it whenever it's used is a bit pointless.
So this takes me back to my option #3 where a mechanism needs to be provided by the ReadOnlyBase<T> class that makes it thread-safe when calls to the CanReadProperty are made in a multi-threaded environment. My idea was to provide a virtual property called RequireThreadSafety that defaults to false and, when overriden by a subclass and returned true, would bypass the authorization results caching altogether, eliminating the multi-threading issue. The authorization results cache doesn't give you much value anyway in a multi-threaded environment where the current principal is potentially different with each thread.
If what I'm saying is correct (and I'm not saying it is), this might represent a design issue with the ReadOnlyBase<T> class since it currently provides no way for the business object developer to make his/her Singleton business object class thread-safe.
You're absolutely right. For some reason I was thinking that this approach would only provide thread safety if another thread was accessing the same property. However, a lock object (similar to the SyncRoot object found on most collections) would do the trick. We could just have a private field of type object like this:
private readonly object instanceLock = new object();
And then lock on it like this:
public string PropertyA
{
lock (this.instanceLock)
{
this.CanReadProperty(true);
return this.propertyA;
}
}
Copyright (c) Marimer LLC