Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NullReferenceException while restoring layout with not existing document #38

Closed
Muhahe opened this issue Jun 12, 2019 · 9 comments
Closed

Comments

@Muhahe
Copy link
Contributor

Muhahe commented Jun 12, 2019

Hi, im using your fork of AD with restore layout as described in this article

While restoring layout, there is possibility that XML layout will contains reference to document which isnt open (while restoring) and in previous version AD ignored it and opened rest of layout.
But in 3.5.6 version it throws NullReferenceException (shown below).

In LayoutContent.cs when it comes to restore not existing document Root.Manager is null (as shown on screenshot)

image

Is there a possibility how to skip restoring documents which are not opened? For restoring layout im using code you wrote on codeproject.com (link in top of issue). Full code example described bellow. I thought, that when i set e.Cancel to true in LayoutSerializationCallback, then layout restoring for this document will be canceled. Or am i missing something?

private void LoadDockingManagerLayout(DockingManager docManager)
        {
            String layoutFileName = Path.Combine(LayoutDir, LayoutFileName);
            layoutFileName = layoutFileName+ ".config";

            if (!File.Exists(layoutFileName))
            {
                return;
            }

            var layoutSerializer = new XmlLayoutSerializer(docManager);

            layoutSerializer.LayoutSerializationCallback += LayoutSerializer_LayoutSerializationCallback;

            try
            {
                layoutSerializer.Deserialize(layoutFileName);
            }
            catch (Exception ex)
            {

                Console.WriteLine("AD Layout deserialization failed: " + ex.ToString());
                //if loading layout fails, restore default layout
                ResetLayout();
            }
            layoutSerializer.LayoutSerializationCallback -= LayoutSerializer_LayoutSerializationCallback;
        }

        private void LayoutSerializer_LayoutSerializationCallback(object sender, LayoutSerializationCallbackEventArgs e)
        {
            // This can happen if the previous session was loading a file
            // but was unable to initialize the view ...
            if (e.Model.ContentId == null)
            {
                e.Cancel = true;
                return;
            }

            ReloadContentOnStartUp(e);
        }

        private void ReloadContentOnStartUp(LayoutSerializationCallbackEventArgs args)
        {
            string sId = args.Model.ContentId;

            // Empty Ids are invalid but possible if aaplication is closed with File>New without edits.
            if (string.IsNullOrWhiteSpace(sId) == true)
            {
             
```   args.Cancel = true;
                return;
            }

            args.Content = ReloadItem(args.Model);

            if (args.Content == null) { 
                args.Cancel = true;                
            }
        }

        private object ReloadItem(object item)
        {
            object ret = null;

            switch (item)
            {
                case LayoutAnchorable anchorable:    
                    //list of tools windows
                    ret = Manager.Tools.FirstOrDefault(i => i.ContentId == anchorable.ContentId);
                    break;
                case LayoutDocument document:
                    // list of restored documents
                    ret = Manager.Documents.FirstOrDefault(i => i.ContentId == document.ContentId);
                    break;
                default:
                    throw new NotImplementedException("Not implemented type of AD item ");
            }

            return ret;
        }


`System.NullReferenceException: Object reference not set to an instance of an object.
   at Xceed.Wpf.AvalonDock.Layout.LayoutContent.CloseInternal() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutContent.cs:line 872
   at Xceed.Wpf.AvalonDock.Layout.LayoutDocument.CloseDocument() in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\LayoutDocument.cs:line 181
   at Xceed.Wpf.AvalonDock.Layout.Serialization.LayoutSerializer.FixupLayout(LayoutRoot layout) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Serialization\LayoutSerializer.cs:line 125
   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 Xceed.Wpf.AvalonDock.Layout.Serialization.XmlLayoutSerializer.Deserialize(String filepath) in F:\MyFiles\CSharp\00_GitHub\Avalondock\source\Components\Xceed.Wpf.AvalonDock\Layout\Serialization\XmlLayoutSerializer.cs:line 110
   at Skald.MVVM.QuestSystem.Controls.Docking.LayoutSaveLoadUtil.LoadDockingManagerLayout(DockingManager docManager) in C:\Workspaces\Skald_questSystemUI\Skald\MVVM\QuestSystem\Controls\Docking\LayoutSaveLoadUtil.cs:line 72`
@Dirkster99
Copy link
Owner

Hi, thanks for you detailled application but I am not sure about the problem you want to raise or whether this realy is an issue in AvalonDock. I have tried this workflow in my Edi application:

Test Work Flow

  • Create a new file
  • Type some content into it
  • Save As 'Issue.txt' in a location of your choice
  • Close Edi
  • Rename file to 'NewIssue.txt'
  • Re-Start Edi (Layout is reloaded on start-up)

and it worked without an exception. Normally, AvalonDock should not be able to differenciate between a document that exists or does not exist. It should be the viewmodel that ensures that non-existing documents are handled as such (non-existing documents should be closed in reload layout). I am using the args.Cancel = true mechanism you describe but maybe I am not using the right workflow to trigger the problem?

Can you either give me a test workflow based on Edi or even better a test application (attach a zip file) and a test workflow to better understand what you are looking at?

Thanks Dirk

@Muhahe
Copy link
Contributor Author

Muhahe commented Jun 18, 2019

@Dirkster99 i will try to provide it ASAP

@Muhahe
Copy link
Contributor Author

Muhahe commented Jun 18, 2019

@Dirkster99 so im not able to reproduce it on testapp/EDI yet, but i saw that you call layout deserialization in EDI in dispatcher with DispatcherPriority.Background and now it seems its okay (example below). I will test it more and let you know if i figure something else

Thanks for your time

private void LoadDockingManagerLayout(DockingManager docManager)
        {
            String layoutFileName = Path.Combine(LayoutDir, LayoutFileName);
            layoutFileName = layoutFileName + ".config";

            if (!File.Exists(layoutFileName))
            {
                return;
            }

            var layoutSerializer = new XmlLayoutSerializer(docManager);

            Dispatcher.CurrentDispatcher.Invoke(new Action(() =>
            {
                layoutSerializer.LayoutSerializationCallback += LayoutSerializer_LayoutSerializationCallback;

                try
                {
                    layoutSerializer.Deserialize(layoutFileName);
                }
                catch (Exception ex)
                {

                    Logger.TraceError("AD Layout deserialization failed. Restoring default layout. : " + ex.ToString());
                    //if loading layout fails, restore default layout
                    ResetLayout();
                }
                layoutSerializer.LayoutSerializationCallback -= LayoutSerializer_LayoutSerializationCallback;
            }), DispatcherPriority.Background);
        }

@Dirkster99
Copy link
Owner

Hi,

since a few people
@mennowo (#47)

seem to run into trouble loading the Layout I am trying to come up with a solution that loads the layout async but ensures proper funtioning. So, I've created a branch with a drafted solution. Would you be able to test this solution (with you problem about the CallBack reported above)?

https://github.com/Dirkster99/AvalonDock/tree/LoadLayoutAsync

This solution loads the layout in a background task and loads the resulting string with Dispatcher.Background priority. This either happens event based or via the LayoutLoaderResult LayoutLoaded property in the App class (if the background task is faster then MainWindow construction).

What do you think about this? Does it work for you?

@Muhahe
Copy link
Contributor Author

Muhahe commented Jun 27, 2019

@Dirkster99 thanks for attempt, but on first look its not working for me. But im trying to provide you some enviroment when you can see this issue on your own.

But when i added fix #47 it seems fixed to me
image

@Dirkster99
Copy link
Owner

Hm, thats interesting. I wonder if there really is a condition where CloseInternal() can be called on a LayoutContent without a Root.Manager being available? I mean, if 2 people report the same solution on a similar case it does strongly suggest that the solution is useful. I just do not understand it, yet...

So, just to be clear - you already stated that using DispatcherPriority.Background like in Edi did work for you but the solution with the LoadAsync branch did not work for you - thats odd because they are both equivalent as far as populating the Layout goes.

It would be really helpful if I could look at a test sample to understand the problem? Is it possible to create a sample application to see if I can verify the same problem using my PC?

@Dirkster99
Copy link
Owner

Dirkster99 commented Jun 29, 2019

Hi, I found a major bug in the implementation of the
https://github.com/Dirkster99/AvalonDock/tree/LoadLayoutAsync

branch. To understand the bug you must understand that the application saves and reloads its layout in the bin folder at application start-up/end (eg. bin\Debug\AvalonDock.Layout.config). The bug was that the AvalonDockContainer is invisible at start-up because I noticed that it loads a lot faster without all the redraws being rendered visibly. But the application has mad the Avalondock container visible only if it actally loaded a layout from file.

So, in the default case, where you download the branch, build it for the first time, and start the app, you do not get any visible docking manager content :-(

... but for me, it was difficult to understand this from a feedback like 'on first look its not working for me' because my folder still had the layout file (even after Clean Solution) - so, I only ran into this condition when I changed branches...

So, to make this more deterministic, could you please download the branch one more time and follow the test steps below to see wether this works for you or not? If, not please tell me where you fail and show some evidence like stacktrace and so forth (I'll be happy to explain the solution next)

Download Branch LoadLayoutAsync
DownLoadBranch
DownLoadBranch_1

  • Extract file AvalonDock-LoadLayoutAsync.zip into a temp folder

  • Open the Solution in Visual Studio

    • Windows>Exception Settings
      Check > Common Language Runtime Exceptions
      (Enabling Exceptions should show some Xml Exceptions in mscorlib but there should be no exceptions beyond this).

    • Righ Click MLibTest (Project) -> Select: Set as Startup Project

    • Clean Solution

    • Rebuild Solution

    • Click Start (Button) in Visual Studio

    • MLibTest Starts with default Layout [OK]
      00 Untitled

  • Go to Windows Explorer and Create a Test Folder containing 3 Text Files
    I just took 3 files from this project and copied them into the test folder:
    (.gitignore, LICENSE, README.md)

    • Go back into MLibTest and Load all 3 files via File>Open
      MLibTest shows the documents and their content [OK]
      02 Untitled

      Close application via MainWindow Close [X] button

    • Restart MLibTest via Start (Button) in Visual Studio

    • The documents are shown as previously opened [OK]
      02 Untitled

      Close application via MainWindow Close [X] button again
      and rename any one of the 3 documents in Windows Explorer
      (eg: Rename '.gitignore' into '_.gitignore')

    • Restart MLibTest via Start (Button) in Visual Studio
      All documents except for the renamed document are loaded and shown [OK]
      03 Untitled

@Dirkster99
Copy link
Owner

I've also tested the test branch without the additional UI thread synchronization. Just comment out these 2 lines:

Application.Current.Dispatcher.Invoke(() => {
...
},System.Windows.Threading.DispatcherPriority.Background);

What about you? Does this work for you? The surprissing find I now have is that AvalonDock does not seem to require UI thread synchronization for deserialization even if the document loading has to be canceled with the e.Cancel = true; statement. So, this realy leaves me wondering how this test branch works for you and whether we are looking at a problem that might be specific to your App?

Dirkster99 added a commit that referenced this issue Jul 27, 2019
@Dirkster99
Copy link
Owner

Version 3.5.10 and later will include the suggested solution

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants