Well, I recently placed a post (http://forums.lhotka.net/forums/post/16373.aspx) regarding an issue with business objects and editing the same business object in a grid and in a separate child form. As someone else pointed out, they are two separate use cases... However, I believe in the process of figuring out THAT issue, we stumbled onto a bug in CSLA 3.0.
The problem, was that occasionally, when canceling the changes to a child object who has a list of child objects, the edit levels on the grand-children would get screwed up. In our particular instance, the child would get correctly canceled back down to edit level 1, and then the children would start to cancel down, and some how right in the middle of canceling the child list's objects, a BeginEdit would be called by the UndoChangesComplete method. This didn't make a whole lot of sense to me, and was very hard to replicate because it was VERY random. There would be times where I step through the CSLA undo code, and the problem would occur and the child objects (and grand-child objects) would get set back to edit level 1 correctly, and then there would be other times where the grand child object would be set to edit level 3, and the others to edit level 1.
With all of that being taken into consideration, I thought that these "random" type of bugs normally crop up because of threading issues. So, what we decided to do was look through all of the CSLA code to find what code was locked for thread-safety. What I found, was that there was a lock placed in some code in the UndoChangesComplete (of BusinessBase.cs) method for adding the rules back into the object. I looked at little bit more at the method and saw that OnUnknownPropertyChanged was called almost immediately after the lock was released. I remembered that OnUnknownPropertyChanged can eventually lead down to the DeserializedHandlers which can get Invoked. Since any thread can wire to the PropertyChanged class, I realized that what was most likely happening was that OnUnkownPropertyChanged was getting called in the middle of my cancel edit, and a PropertyChanged (somewhere) was calling BeginEdit (dont yet know why, though).
All-in-all, I managed to fix the inconsistent issue by increasing the lock within the UndoChangesComplete method to include the OnUnknownPropertyChanged and base.UndoChangesComplete method. The code now looks like this instead:
private object _undoChangesCompleteLock = new object();
protected override void UndoChangesComplete()
{
_bindingEdit = false;
ValidationRules.SetTarget(this);
AddInstanceBusinessRules();
lock (_undoChangesCompleteLock)
{
if (!Validation.SharedValidationRules.RulesExistFor(this.GetType()))
{
//lock (this.GetType())
if (!Validation.SharedValidationRules.RulesExistFor(this.GetType()))
AddBusinessRules();
}
OnUnknownPropertyChanged();
base.UndoChangesComplete();
}
}
That's interesting, though you've reintroduced the bug that the lock was there to prevent in the first place... ![Smile [:)]](/emoticons/emotion-1.gif)
The lock is there to resolve an issue in ASP.NET that exists because the SharedValidationRules are static - they exist across all instances of the type. So the lock itself must also be static. It actually must lock across all instances of that specific type - hence the use of this.GetType() as the lock source.
You could change your _undoChangesCompleteLock to be static, but then you'd be locking across all instances of all types, which is too broad a scope. But by using an instance level object your scope is too narrow and the original race condition has been reintroduced.
So to make this work as you expect, you'll need two locks - one at the type level (like existed before) and your new instance level lock.
But it is important to realize that, in general, CSLA is not threadsafe. In your scenario you must, by definition, be interacting with the object across multiple threads, and that's not supported in general.
So while it might be a "bug" that you are seeing this issue, it is probably one of many bugs that you'll encounter if you allow multiple threads to interact with your object concurrently.
Good point about asp.net. Our application is not asp.net based - it's a windows forms app... So, I failed to think about that particular case. That now makes sense to me why I saw you using this.GetType() for all of your locks.
I am curious though... You say that CSLA.net is not thread-safe, but in any complex application (whether it be ASP.Net or Windows Forms), it is a probability that you will be dealing with multiple threads... For example, in our particular instance, we are not purposely interacting with our object from multiple threads, this is being done somewhere in the .net framework. This is especially the case when dealing with events, such as the PropertyChanged event. I may be wrong, but I can see it being a very common use case to have multiple threads accessing the business objects, so why do you say that CSLA is not thread-safe?
The .NET BCL is not (for the most part) threadsafe either. Certainly
Windows Forms isn’t threadsafe for example. Neither is most of ASP.NET,
because it is designed with the assumption that each page request runs on a
single thread. WPF isn’t threadsafe either, looking into the future –
all the UI components are assumed to run on a single thread.
So CSLA .NET is merely following .NET’s lead in this case.
The single-threaded assumption is a double-edged sword. On one
hand, it is much faster, because it minimizes locking (which is a perf killer).
On the other hand, it restricts the use of threading in user code. Microsoft
has generally opted for performance rather than the flexibility of using free
threading models, and so have I.
The only methods, in general terms, that are threadsafe are
static/Shared methods. This is true for the .NET BCL, and for CSLA .NET. If an
instance method or type is threadsafe, there’s a special note to that
effect in the docs. You’ll be hard-pressed to find many of them though –
it just isn’t the model followed by .NET.
Rocky
I was getting this same exact issue, using Windows Forms. I created some extension methods so I could see the edit level for debugging purposes(http://forums.lhotka.net/forums/thread/22498.aspx), and discovered that my child that was getting the edit level increased on a OnUnknownPropertyChanged() call. I figured that my child must still be databinded someone where. After searching more, I realized that the child bindingsource.datasource must be set to null to clear it. After I added this call, everything worked fine.
Is there any chance you can modify the Business Base Class to catch this error?
protected override void UndoChangesComplete()
{
BindingEdit = false;
ValidationRules.SetTarget(this);
AddInstanceBusinessRules();
if (!Validation.SharedValidationRules.RulesExistFor(this.GetType()))
{
lock (this.GetType())
{
if (!Validation.SharedValidationRules.RulesExistFor(this.GetType()))
AddBusinessRules();
}
}
int currentEditLevel = this.EditLevel;
OnUnknownPropertyChanged();
if(this.EditLevel != currentEditLevel)
Throw new Exception("Edit level has been updated during Undo. Object is still databinded and must be unbinded before Cancel Edit is called");
base.UndoChangesComplete();
}
I'm not sure what your issue is. For testing purposes, I found it helped to use these Extension Methods http://forums.lhotka.net/forums/thread/22498.aspx to see what each call was doing to the edit levels.
Then setup a function that does something like this.
root = cslaRoot.GetRoot()
Console.Writeline("Initial Edit Levels")
Console.Writeline(root.GetChildEditLevelHierarchy())
dataBindingSource.DataSource = root
Console.Writeline("After Data Binding")
Console.Writeline(root.GetChildEditLevelHierarchy())
dataBindingSource.DataSource.CancelEdit
Console.Writeline("After Cancel Edit")
Console.Writeline(root.GetChildEditLevelHierarchy())
If you're binding Children and grand children, make sure to put that in there as well. It really helped me to narrow down what I was working on.
Copyright (c) Marimer LLC