Here is an example of the general pattern that has me concerned in the FieldDataManager. I have seen code like this in CSLA (and have used the same pattern myself).
private static List GetConsolidatedList(Type type)
{
List result = null;
if (!_consolidatedLists.TryGetValue(type, out result))
{
lock (_consolidatedLists)
{
if (!_consolidatedLists.TryGetValue(type, out result))
{
result = CreateConsolidatedList(type);
_consolidatedLists.Add(type, result);
}
}
}
return result;
}
I've run into some threading problems in my implementation lately, and while I don't necessarily believe CSLA is at fault, I'm not sure I (or we :) should be writing code like this.
Is there any reason to be believe TryGetValue() would be thread-safe? Certainly concurrent TryGetValue() calls work, but might there be a problem is an Add() is executing concurrently with the TryGetValue()? Depending on the internal implementation, if no internal locking is used the I can see how it could return true while the out value is returned as null.
If this is indeed safe, I'd sure welcome a reference I could read to put me at ease.
This has been discussed before (at least twice). The short answer is that this is not 100% safe. The long answer is that, prior to .NET 4 with its new threading collection types, the only solution that is 100% safe requires a lock on every call. The performance implications of doing that are quite high.
The advantage CSLA is working with is that the initialization of these values is rare. In pretty much every case this occurs once per AppDomain, as the type is first accessed. So the practical result is that (to my knowledge) this has never caused any issues. Yes, it is theoretically an issue, but given the way it is applied in CSLA it doesn't appear to be a real issue.
Sadly, while .NET 4 added new threadsafe types, SL4 did not. So CSLA 4 now has some cases where this type of code still exists on the SL side, but where I've been able to leverage the new .NET 4 types for the .NET side. A step in the right direction at least.
OK, this is good to know (I did a precursory search before posting, but sorry for bringing up a duplicate topic). I know much of the CSLA code is executed just once -- I wasn't picking on this particular instance, just trying understand if the pattern itself had problems.
Now that I have been doing logging, it turns out that so far all of my own problems are occuring at initialization time and I have eliminated at least one race condition and a possible deadlock. Things work just fine, and then the application pool recycles while a bunch of incoming requests are trying to be processed while all the DLL's are loading and the BO classes get initialized.
I'll search for the prior discussions. From today's intensive reading, I was under the impression that the cost of a lock() was comparatively low as long as there was no contention. (And if there is contention, then it seems like you need the lock anyway).
As far as I know, lock() is a Mutex, which is a kernel mode lock - which at a minimum means a context switch out of user mode at the CPU/OS level on each invocation.
That's yet another .NET 4 feature - a set of cheaper user-mode locks (spinwait locks) that avoid that context switch.
It is true that lock() isn't ridiculously expensive - unless it is invoked in a tight loop, like during the process of loading your business objects with data from the DAL where you might touch thousands of properties (and thus rules) in what we hope is a fraction of a second. In that case I fear lock() might not be so cheap...
I think this was the original thread on this: http://forums.lhotka.net/forums/p/8474/40306.aspx#40306
Also, I found a good discussion (and code which actually breaks this pattern) here if anyone is interested:
Copyright (c) Marimer LLC