search

microsoft / microsoft-ui-xaml

Windows UI Library: the latest Windows 10 native controls and Fluent styles for your applications
MIT License
6.28k stars 674 forks source link

TreeView Memory Leak #8275

Open jschwizer99 opened 1 year ago

jschwizer99 commented 1 year ago

Describe the bug

When removing/adding items to a tree view memory is leaked. It seems to be caused by virtualization. This memory leak seems to be closely related to other stability issues observed when updating a treeview based on a template selector. Observed instabilities

All above listed instabilities and memory leaks are blockers for releasing our application to customers.

This ticket is focused on the memory leak as it is easy to transfer to a simple demo app.

Steps to reproduce the bug

Unzip the attached demo app based on an App SDK WinRT C++ project.

The failure modes reported are based on App SDK version 1.2.4. However, the same behavior can be observed on 1.3.0-preview1 as well. The application contains a very simple treeview with in total 7 items in 3 levels. Different modes to clear and repopulate the tree can be selected.

Clear Methods

Populate Methods

The memory leak can be easily observed with heap profiling.

Memory Leak when using Clear()

When using Clear() on the observablevector of the ItemsSource all items are leaked. In this example, this is 7 items. As Clear() is triggering a Reset() event it seems that the XAML code doesn't get any notification that the nodes were removed from the observable vector.

TreeViewLeakClear

Memory Leak when using per item RemoveAtEnd()

When removing items per element the XAML code is correctly notified and thus releases the WinRT object.

    void MainWindow::clearRecursive(TreeStability::TreeViewModel const& value) const
    {
        for (const auto& child : value.SubItems())
        {
            clearRecursive(child);
        }
        while (value.SubItems().Size() > 0)
        {
            value.SubItems().RemoveAtEnd();
        }
    }

But all items that are directly under the shown items do still leak. These items are managed by virtualization. When these items are expanded / collapsed the Unload event is triggered.

TreeViewRemoveAtEnd

If no level is expanded 3 items are leaking as a consequence. In total there are 4 hidden items but one is on the second level and thus not affected by the leak.

If the full review is expanded no memory leak is visible.

TreeViewLeakRemoveAtEndExpanded

Unfortunately, with the above per-item removal, another issue can be observed. If executed many times a crash can be triggered by the IsExpanded(true) property change.

IsExpandedCrash

Different other approaches have been examined without any success

For this specific case, the removal of the full treeview could solve the leak for that case. But this isn't a feasible solution for cases where only some items have to be removed or added to the tree.

As a clean fix may take longer any idea for a work-around for the memory leak and stability issue is highly welcome. In our application, we rely on the treeview with template selectors. But with the current stability issues, we face a blocker for any shipment of the application.

Are there any possibilities to disable virtualization and node recycling on the treeview on WinRT C++?

Expected behavior

No memory leak when adding or removing treeview items. The virtualization used in the tree view should not trigger instabilities by dynamic treeview updates.

Screenshots

TreeViewLeakApp

NuGet package version

WinUI 3 - Windows App SDK 1.2.4: 1.2.230217.4

Windows version

Windows 10 (21H2): Build 19044, Windows 10 (20H2): Build 19042

Additional context

TreeStability.zip

As I had to remove the package folder you may see a weird deploy error for the Debug version. Try Release instead or restore the package folder.

List of other issues reported for treeview and listview which may be very closely linked due to the virtualization issues.

TreeView Memory Leak https://github.com/microsoft/microsoft-ui-xaml/issues/7049 https://github.com/microsoft/microsoft-ui-xaml/issues/818

IsExpanded Crash https://github.com/microsoft/microsoft-ui-xaml/issues/7618

Crashes https://github.com/microsoft/microsoft-ui-xaml/issues/5683

Scrambling https://github.com/microsoft/microsoft-ui-xaml/issues/7044 https://github.com/microsoft/microsoft-ui-xaml/issues/2121

ListView Layout Cycle https://github.com/microsoft/microsoft-ui-xaml/issues/6218

jschwizer99 commented 1 year ago

Adding an additional UpdateLayout() between node creation and expansion changes the behavior.

Create Nodes Update Layout -> Memory leak, no crashes

Create Nodes Update Layout Expand Nodes Update Layout -> Memory leak, significantly lower crash risk

Create Nodes Expand Nodes Update Layout -> No memory leak, crash during expand

Thus, it looks like for the crash we need to expand newly added nodes before the first layout update. The memory leak seems to be triggered during the layout update when there are collapsed nodes (with virtualization).

But in general, the UpdateLayout() is not required to trigger the issue. But please keep in mind that the 10x repeat button won't perform as expected when entirely removed.

P.S. The weird deploy error of the example solution due to the deleted package folder can be resolved by restoring the NuGet packages and the close and reopening Visual Studio 2022.

KubaSzostak commented 1 year ago

Here you have another TreeViewNode crash code sample: #7089