npolyak / NP.Ava.UniDock

New (Avalonia 11) UniDock repository
MIT License
141 stars 10 forks source link

Memory Leak with MVVM? #10

Closed StefanKoell closed 7 months ago

StefanKoell commented 7 months ago

Hi,

I did some profiling and testing lately and found out that view models are not cleaned up/garbage collected once the DockItem has been removed from the TabbedDockGroup. I only tested with TabbedDockGroup maybe there's also an issue with the StackDockGroup. This happens when I just close the tab or explicitly remove the ViewModel from the IUniDockService.DockItemsViewModels observable collection.

From the memory profiling it seems that the LogicalTreeattachmentEventArgs is in the path of one of the Dominators as well as DockItem.

I wasn't able to further track down where a reference is still kept to the view model. Any idea why this happens? Do I have to do something specific to make sure my VMs are picked up for garbage collection?

StefanKoell commented 7 months ago

Did some further debugging but couldn't get far as it's hard to get familiar with the quite complex code base. What I see in the profiler is the following: CleanShot 2024-01-31 at 16 55 20@2x Looks like the DockItem instance keeps lingering around after the tab has been closed. The DockItem still holds a reference to the view model which keeps it from garbage collection.

npolyak commented 7 months ago

Thanks Stefan, I'll take a look on Sunday.

StefanKoell commented 7 months ago

Hi Nick!

I think I could locate the memory leak and I wanted to submit a PR but it seems I can't build from source anymore. Looks like the submodules are out of place or something. I cloned it several times and also colleagues tried it. No dice :(

Spend a couple of hours with that and had to give up. I think you really should consider a simpler structure in a single repo. Can you check?

As for the leak: The DockItem.Remove() method is invoked via the DataItemsViewModelBehavior => OnGroupViewModelRemoved. The issue is, as far as I can see, the DockItem is never removed from memory because in the OnDockParentChanged the AttachedToLogicalTree event is subscribed but never invoked and therefore never unsubscribed if the tab is just closed.

Not sure if that's all there is. Since I can't build from source anymore, it's hard to verify.

Thanks! Stefan

npolyak commented 7 months ago

Hey Stefan,

Sorry, the prototype project I recommended to you NP.DataContextSample was missing a reference project. I added it and it should compile - please pull the latest. You can also use any other prototype to build NP.Ava.UniDock - I tested about half of them and none failed aside from NP.DataContextSample.

If you still have a problem - let us sync up on Sunday - I sent you my whatsapp number via linkedin.

StefanKoell commented 7 months ago

Hi Nick,

check my PR. Not sure I'm doing it "right" - hopefully there are no side effects ;)

npolyak commented 7 months ago

Closing this issue, after Stefan fixed it.