Dirkster99 / AvalonDock

Our own development branch of the well known WPF document docking library
Microsoft Public License
1.4k stars 321 forks source link

InvalidOperationException when deserializing layout #59

Closed dazawa closed 4 years ago

dazawa commented 5 years ago

Hi, when attempting to deserialize a saved layout from XML, I get the following exception:

System.InvalidOperationException: 'Collection was modified; enumeration operation may not execute.'

It seems to be reproducable by moving or removing a LayoutAnchrablePane that is contained within the serialized layout and then deserializing the layout.

Here is the deserialization code:

var serializer = new XmlLayoutSerializer(dockingManager); serializer.LayoutSerializationCallback += SerializerLayoutSerializationCallback; [...] using (var stream = new StreamReader(defaultLayout)) { serializer.Deserialize(stream); }

(The exception is thrown somewhere inside serializer.Deserialize(stream);)

StackTrace:

at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.List1.Enumerator.MoveNextRare() at System.Collections.Generic.List1.Enumerator.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.d0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 32 at Xceed.Wpf.AvalonDock.Layout.Extensions.d__0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 35 at Xceed.Wpf.AvalonDock.Layout.Extensions.d0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 35 at Xceed.Wpf.AvalonDock.Layout.Extensions.d0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 35 at Xceed.Wpf.AvalonDock.Layout.Extensions.d__0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 35 at Xceed.Wpf.AvalonDock.Layout.Extensions.d0.MoveNext() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Extentions.cs:line 35 at System.Linq.Enumerable.d__951.MoveNext() at Xceed.Wpf.AvalonDock.DockingManager.Layout_ElementAdded(Object sender, LayoutElementEventArgs e) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\DockingManager.cs:line 2976 at Xceed.Wpf.AvalonDock.Layout.LayoutRoot.OnLayoutElementAdded(LayoutElement element) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutRoot.cs:line 763 at Xceed.Wpf.AvalonDock.Layout.LayoutElement.OnRootChanged(ILayoutRoot oldRoot, ILayoutRoot newRoot) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutElement.cs:line 136 at Xceed.Wpf.AvalonDock.Layout.LayoutElement.set_Parent(ILayoutContainer value) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutElement.cs:line 68 at Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable.UpdateParentVisibility() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutAnchorable.cs:line 769 at Xceed.Wpf.AvalonDock.Layout.LayoutAnchorable.OnParentChanged(ILayoutContainer oldValue, ILayoutContainer newValue) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutAnchorable.cs:line 270 at Xceed.Wpf.AvalonDock.Layout.LayoutElement.set_Parent(ILayoutContainer value) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutElement.cs:line 64 at Xceed.Wpf.AvalonDock.Layout.LayoutGroup1._children_CollectionChanged(Object sender, NotifyCollectionChangedEventArgs e) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutGroup.cs:line 253 at System.Collections.Specialized.NotifyCollectionChangedEventHandler.Invoke(Object sender, NotifyCollectionChangedEventArgs e) at System.Collections.ObjectModel.ObservableCollection1.OnCollectionChanged(NotifyCollectionChangedEventArgs e) at System.Collections.ObjectModel.ObservableCollection1.RemoveItem(Int32 index) at System.Collections.ObjectModel.Collection1.Remove(T item) at Xceed.Wpf.AvalonDock.Layout.LayoutGroup1.RemoveChild(ILayoutElement element) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutGroup.cs:line 137 at Xceed.Wpf.AvalonDock.DockingManager.DetachAnchorablesSource(LayoutRoot layout, IEnumerable anchorablesSource) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\DockingManager.cs:line 2917 at Xceed.Wpf.AvalonDock.DockingManager.OnLayoutChanged(LayoutRoot oldLayout, LayoutRoot newLayout) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\DockingManager.cs:line 147 at Xceed.Wpf.AvalonDock.DockingManager.OnLayoutChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\DockingManager.cs:line 127 at System.Windows.DependencyObject.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args) at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType) at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal) at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value) at Xceed.Wpf.AvalonDock.Layout.Serialization.XmlLayoutSerializer.Deserialize(TextReader reader) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Serialization\XmlLayoutSerializer.cs:line 82 at Alpine.Gui.TestSuite.TestSuiteWindow.LoadDefaultLayout(Object sender, RoutedEventArgs e) in D:#ATAT3\Alpine.Atat\src\Core\Alpine.Gui\TestSuite\TestSuiteWindow.xaml.cs:line 389 at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs) at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised) at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args) at System.Windows.UIElement.RaiseEvent(RoutedEventArgs e) at System.Windows.Controls.MenuItem.InvokeClickAfterRender(Object arg) at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs) at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

I added the layout I am trying to deserialize: layout.zip

Kind Regards, David

Dirkster99 commented 5 years ago

Hi I cannot verify your problem since you do not show your code

.LayoutSerializationCallback += (s, e) =>
{
...
}

handler to handle the case in which you load a layout item that cannot be matched to a registered viewmodel/view.

Your description sounds like this is what you are missing?

If thats not the problem please either attach your code or try to explain your problem based on any of the demo projects in this repository, thanx

dazawa commented 5 years ago

Thanks for your response.

We already have a handler: serializer.LayoutSerializationCallback += SerializerLayoutSerializationCallback;

Unfortunately, I cannot show any more of the internal code than what I have already posted, but I will try to reproduce with one of the demo projects today as you suggested and get back to you with the result.

Dirkster99 commented 5 years ago

The error message 'Collection was modified; enumeration operation may not execute.' does not appear to be a XML serializer specific message. It seems more likely that you are using a foreach loop somewhere in your code to iterate through all toolwindows/documents and the collection gets changed while your foreach iterateration is still on-going.

I would follow the suggestion on StackOverflow (change to foreach(var i in ....ToList())) just to confirm if this fixes the error message. If it does you are probably doing two things in a bad timing and you have to review that problem more carefully.

This could of course also be a bug in AvalonDock but I'd be realy surprised if it was since nobody else has ever mentioned this specific exception...

dazawa commented 5 years ago

I was able to reproduce the exception only once using the TestApp demo project by randomly closing and reloading toolboxes. The exception was thrown somewhere within Manager.Layout = layout; in the Deserialize(System.IO.TextReader reader) method in DockingManager... which is weird since all that does is call GetValue/SetValue from DependencyObject. Unfortunately, I haven't been able to reproduce again....

I also tried downgrading our AvalonDock version from 3.5.10 to 3.5.4.1 which solved the issue.

Dirkster99 commented 5 years ago

Do you mean using version 3.5.4 solved your issue? Thats really strange because I don't think we've changed anything in particular about the layout serializer. Can you determine the version where it works for you and where it stops working (eg. version 3.5.4 works but version 3.5.5 dows not work)?

dazawa commented 5 years ago

Yes, 3.5.4 works fine for me but 3.5.5 does not.

Dirkster99 commented 5 years ago

Thats interesting. The 2 versions are different by a small number of commits shown below in the commit history spanning from 2. May to 13. May. The actual number of commits relevant to us is smaller than whats shown in the history because we only need to look at commits that resulted in a change in the source/Components/Xceed.Wpf.AvalonDock/ library.

Would you be able to setup a test project with the sources from 2. May (verify your problem does not exist) and move forward in the commit history (until 13 May) to see when your problem appears for the first time? This should enable us to find the actual commit that causes your problem and maybe even determine a fix for it :-)

Commit history for version 3.5.4 to 3.5.5

Dirkster99 commented 5 years ago

Question on the side: Does your exception also occur when you initialize the layout within the ui thread?

I am only asking because I had this other issue with two other users and it turned out that one possible solution was to initialize the layout in the context of the ui thread (you may load the string within a background task but do the deserialization in the ui thread as shown in the link above).

dazawa commented 5 years ago

Hi, sorry for the late reply. I did what you suggested and went through each commit between 3.5.4 and 3.5.5. The issue appears for the first time after the merge you did on 13.05. (ID: f818c88f). The commits before (09967524 and earlier) that all do not have the issue.

initializing the layout inside or outside of the ui thread does not affect the issue.

Dirkster99 commented 5 years ago

Hey, thanks for testing this but your result looks really impossible because you can see in the change log that the merge did not change anything in the source/Components/Xceed.Wpf.AvalonDock/ library.

All that was changed was the Readme.md file, and some files in MLibTest which is a demo project (unless I am missing something here).

So, the problem must at least either be one (or more) commit(s) before or after this commit, but not the linked commit (would you agree?)

dazawa commented 5 years ago

Yes, I fully agree, it does look strange, I'm just saying what results I came to. I will double check. In the meantime, I've narrowed the source of the issue down to the following foreach located in /Layout/Extensions.cs by analyzing the stack-trace a bit closer: foreach( var childElement in container.Children ) { yield return childElement; foreach( var childChildElement in childElement.Descendents() ) yield return childChildElement; }

This is where the exception is thrown. Does this help you in any way?

I will check the commits again.

Dirkster99 commented 5 years ago

The reference is relevant as it shows what I initially suggested. So, you could go ahead and replace the Foreach loops with the ToList() statements using for loops instead to test whether this results in a crash somewhere else - since that crash could indicate the real source of your problem.

The code you are refering to is used in 81 places to recursively visit all Children of all Children in the Hierarchy of containers that can be configured in AvalonDock

public static IEnumerable<ILayoutElement> Descendents( this ILayoutElement element )
{
  var container = element as ILayoutContainer;
  if( container != null )
  {
    foreach( var childElement in container.Children )
    {
      yield return childElement;
      foreach( var childChildElement in childElement.Descendents() )
        yield return childChildElement;
    }
  }
}

The originating call seems to be coming from the DockingManager's.InternalSetActiveContent(...) method (if I read the stack trace correctly):

private void InternalSetActiveContent( object contentObject )
{
  var layoutContent = Layout.Descendents().OfType<LayoutContent>().FirstOrDefault( lc => lc == contentObject || lc.Content == contentObject );
  _insideInternalSetActiveContent = true;
  Layout.ActiveContent = layoutContent;
  _insideInternalSetActiveContent = false;
}

So, this makes me wonder, if the controls that you use within the AvalonDock containers are: 1) aquiring a focus or 2) try to activate themeselves when being loaded - or 3) try to add a type of layoutanchorable while deserialization takes place?

this may even be initiated through a bound viewmodel. So, another way of isolating this problem could be to test whether deactivating code attached to OnLoadEvents of each control (and other such events) would lead to an absence of the error - if yes - we would know at last how the problem was triggered but not why, yet...

Either way, it should be possible to isolate the problem either in the test appliactions in this repository (by adjusting the test application such that your problem is visible) or adjusting a test version of you own application such that the problem is no longer verifyable...

The result should be that we find: 1) its connected to a certain control being used in your AvalonDock app or 2) a certain layout that cause the problem no matter was control is used.

I take it the layout is not the problem as you were able to check with the test apps? Maybe its time to take a hard look at your app then?

Martin5630 commented 5 years ago

@Dirkster99 I also get an error with the MFLibTest application. I used the application to find out why the AutoHide not working. I get the error in the DockingManager Class in the method DockingManager_Loaded

AvalonDock.Layout.txt

image

And I also get the error in the Extension method in my Application when I load different Layouts at runtime.I have only Tool-Windows in my Application. So what I do actually is, before I load a new Layout I set by all the Toolwindows the property isvisible = false. I hope this helps to find the error in AvalonDock. It must be a race condition when AvalonDock add and removes anchorables.

Dirkster99 commented 5 years ago

OK, I can verify the exception with the Layout from @Martin5630 thanks for that - I can now look deeper into it :-)

The source of the problem in the layout from @Martin5630 seems to be the LayoutDocumentFloatingWindowtag - I can replace it with <LayoutDocumentFloatingWindow/> and the exception is no longer thrown. Although, the problem is similar in its nature it seems to be different to the original exception which was raised in the Extensions.cs method and the original layout from @dazawa does not contain a LayoutDocumentFloatingWindow item - so, both problems are similar in their nature but probably not the same :-(

    <LayoutDocumentFloatingWindow>
      <LayoutDocument Title="Noname" IsLastFocusedDocument="True" FloatingLeft="1394.28571428571" FloatingTop="321.714285714286" FloatingWidth="770.857142857143" FloatingHeight="433.142857142857" LastActivationTimeStamp="08/13/2019 12:47:24" PreviousContainerId="6f73d732-d2a8-47da-ae59-d6ba0fdeaf15" PreviousContainerIndex="0" />
    </LayoutDocumentFloatingWindow>

More precisly, I can remove the PreviousContainerId="6f73d732-d2a8-47da-ae59-d6ba0fdeaf15" item from the above tag and the exception is no longer happening for me.

A possible fix for the exception from @Martin5630 seems to be to finish the query before adding the new FloatingWindowControls in the DockingManager_Loaded method in line 2396 of DockingManager.cs:

        List<LayoutFloatingWindow> items = new List<LayoutFloatingWindow>(Layout.FloatingWindows.Where(fw => !_fwList.Any(fwc => fwc.Model == fw)));
        foreach ( var fw in items)
          _fwList.Add( CreateUIElementForModel( fw ) as LayoutFloatingWindowControl );

I have to think about this - it looks like there are too many linq queries based on events and events of events that will eventually create a race condition as we see here, so I am not sure if the above is really a solution or just a hack :-(

Dirkster99 commented 5 years ago

Hi @dazawa and @Martin5630

I have created a fix candidate for the deserializing issues discussed here:

1) issue 1 2) issue 2

This candidate is in a seperate branch to make sure it does fix the issue and breaks nothing else - would you guys please download, compile and test the branch to see if this clears your issues that you mentioned?

It would also be good if you could do some general testing to amke sure nothing new creps up:

Branch for testing is here: EnumInvalidOperationException

dazawa commented 4 years ago

Hi @Dirkster99 sorry for the late reply, I've been away.

Thanks for the effort! I will test as soon as I get back, which will be in a couple of days and will post an update here.

Dirkster99 commented 4 years ago

This issue should be fixed with version 3.6.1 (just released it now)

dazawa commented 4 years ago

Hey @Dirkster99 We updated to the latest version (3.6.1) but this issue still occurs. Same error path and exception.

Dirkster99 commented 4 years ago

That's really strange, I expected this to resolved now :-( can you please post the current stack trace one more time? - at least some line numbers should be different now … I am really at a lost end since I cannot debug your problem so I am not able to find a fix - do you have any suggestion towards a fix?

dazawa commented 4 years ago

Here's the full StackTrace

Error Message: Collection was modified; enumeration operation may not execute. Top-level Exception Type: System.InvalidOperationException Message: Collection was modified; enumeration operation may not execute. Source: mscorlib Stack Trace: at System.ThrowHelper.ThrowInvalidOperationException(ExceptionResource resource) at System.Collections.Generic.List1.Enumerator.MoveNextRare() at System.Collections.Generic.List1.Enumerator.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at Xceed.Wpf.AvalonDock.Layout.Extensions.<Descendents>d0.MoveNext() at System.Linq.Enumerable.<OfTypeIterator>d__951.MoveNext() at Xceed.Wpf.AvalonDock.DockingManager.Layout_ElementAdded(Object sender, LayoutElementEventArgs e) at Xceed.Wpf.AvalonDock.Layout.LayoutRoot.OnLayoutElementAdded(LayoutElement element) at Xceed.Wpf.AvalonDock.Layout.LayoutElement.OnRootChanged(ILayoutRoot oldRoot, ILayoutRoot newRoot) at Xceed.Wpf.AvalonDock.Layout.LayoutElement.set_Parent(ILayoutContainer value) at Xceed.Wpf.AvalonDock.Layout.LayoutGroup1._children_CollectionChanged(Object sender, NotifyCollectionChangedEventArgs e) at System.Collections.Specialized.NotifyCollectionChangedEventHandler.Invoke(Object sender, NotifyCollectionChangedEventArgs e) at System.Collections.ObjectModel.ObservableCollection1.OnCollectionChanged(NotifyCollectionChangedEventArgs e) at System.Collections.ObjectModel.ObservableCollection1.RemoveItem(Int32 index) at System.Collections.ObjectModel.Collection1.Remove(T item) at Xceed.Wpf.AvalonDock.Layout.LayoutGroup1.RemoveChild(ILayoutElement element) at Xceed.Wpf.AvalonDock.DockingManager.DetachAnchorablesSource(LayoutRoot layout, IEnumerable anchorablesSource) at Xceed.Wpf.AvalonDock.DockingManager.OnLayoutChanged(LayoutRoot oldLayout, LayoutRoot newLayout) at Xceed.Wpf.AvalonDock.DockingManager.OnLayoutChanged(DependencyObject d, DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.FrameworkElement.OnPropertyChanged(DependencyPropertyChangedEventArgs e) at System.Windows.DependencyObject.NotifyPropertyChange(DependencyPropertyChangedEventArgs args) at System.Windows.DependencyObject.UpdateEffectiveValue(EntryIndex entryIndex, DependencyProperty dp, PropertyMetadata metadata, EffectiveValueEntry oldEntry, EffectiveValueEntry& newEntry, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType) at System.Windows.DependencyObject.SetValueCommon(DependencyProperty dp, Object value, PropertyMetadata metadata, Boolean coerceWithDeferredReference, Boolean coerceWithCurrentValue, OperationType operationType, Boolean isInternal) at System.Windows.DependencyObject.SetValue(DependencyProperty dp, Object value) at Xceed.Wpf.AvalonDock.Layout.Serialization.XmlLayoutSerializer.Deserialize(TextReader reader) at Alpine.Gui.TestSuite.TestSuiteWindow.LoadDefaultLayout(Object sender, RoutedEventArgs e) in D:#ATAT3\ATAT3\src\Core\Alpine.Gui\TestSuite\TestSuiteWindow.xaml.cs:line 383 at System.Windows.RoutedEventHandlerInfo.InvokeHandler(Object target, RoutedEventArgs routedEventArgs) at System.Windows.EventRoute.InvokeHandlersImpl(Object source, RoutedEventArgs args, Boolean reRaised) at System.Windows.UIElement.RaiseEventImpl(DependencyObject sender, RoutedEventArgs args) at System.Windows.UIElement.RaiseEvent(RoutedEventArgs e) at System.Windows.Controls.MenuItem.InvokeClickAfterRender(Object arg) at System.Windows.Threading.ExceptionWrapper.InternalRealCall(Delegate callback, Object args, Int32 numArgs) at System.Windows.Threading.ExceptionWrapper.TryCatchWhen(Object source, Delegate callback, Object args, Int32 numArgs, Delegate catchHandler)

Dirkster99 commented 4 years ago

OK, since this nut seems to be a little harder to crack, please do also think about additional evidence like screenshot of the case in VS or (even better) a separate test client showing the problem with a certain workflow.

jacobavenkraft commented 4 years ago

Hello Dirkster99, I have just started investigating using AvalonDock and was writing some test applications. I ran across this exact same issue -- I am using the 4.10 version available on NuGet. I downloaded your code from GitHub so that I could debug the issue.

In my test app, it appears that the underlying issue is caused by the following section of code within DockingManager.cs: image

The "foreach" loop is calling Layout.Descendants() which as you know uses yield returns to return the descendants. I believe the calls to CreateDocumentLayoutItem and CreateAnchorableLayoutItem have the potential to modify the contents of the Layout and thus a "Collection was modified" exception gets thrown.

Changing the code as follows ensured that the exception did not occur (at least with my test app):

image

Dirkster99 commented 4 years ago

@jacobavenkraft Thanks for the useful feedback - I've implemented a fix as you suggested above and hope this will finally fix this issue. I am going to do another release soon ... Thanx Drk