Fixed memory leak when DataGrid is attached to INotifyCollectionChanged

AvaloniaUI · Apr 8, 2022 · bbf3099 · bbf3099
1 parent 6e35970
commit bbf3099
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 0 deletions.
diff --git a/src/Avalonia.Controls.DataGrid/DataGrid.cs b/src/Avalonia.Controls.DataGrid/DataGrid.cs
@@ -2060,6 +2060,25 @@ public void ScrollIntoView(object item, DataGridColumn column)
                     forceHorizontalScroll: true);
             }
         }
+
+        protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnAttachedToVisualTree(e);
+            if (DataConnection.DataSource != null && !DataConnection.EventsWired)
+            {
+                DataConnection.WireEvents(DataConnection.DataSource);
+            }
+        }
+
+        protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnDetachedFromVisualTree(e);
+            // When wired to INotifyCollectionChanged, the DataGrid will be cleaned up by GC
+            if (DataConnection.DataSource != null && DataConnection.EventsWired)
+            {
+                DataConnection.UnWireEvents(DataConnection.DataSource);
+            }
+        }
 
         /// <summary>
         /// Arranges the content of the <see cref="T:Avalonia.Controls.DataGridRow" />.

diff --git a/tests/Avalonia.LeakTests/Avalonia.LeakTests.csproj b/tests/Avalonia.LeakTests/Avalonia.LeakTests.csproj
@@ -9,6 +9,7 @@
   <Import Project="..\..\build\NetFX.props" />
   <Import Project="..\..\build\SharedVersion.props" />
   <ItemGroup>
+    <ProjectReference Include="..\..\src\Avalonia.Controls.DataGrid\Avalonia.Controls.DataGrid.csproj" />
     <ProjectReference Include="..\..\src\Markup\Avalonia.Markup.Xaml\Avalonia.Markup.Xaml.csproj" />
     <ProjectReference Include="..\..\src\Markup\Avalonia.Markup\Avalonia.Markup.csproj" />
     <ProjectReference Include="..\..\src\Avalonia.Animation\Avalonia.Animation.csproj" />

diff --git a/tests/Avalonia.LeakTests/ControlTests.cs b/tests/Avalonia.LeakTests/ControlTests.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Collections.Generic;
+using System.Collections.ObjectModel;
 using System.Linq;
 using System.Runtime.Remoting.Contexts;
 using Avalonia.Controls;
@@ -24,11 +25,52 @@ namespace Avalonia.LeakTests
     [DotMemoryUnit(FailIfRunWithoutSupport = false)]
     public class ControlTests
     {
+        // Need to have the collection as field, so GC will not free it
+        private readonly ObservableCollection<string> _observableCollection = new();
+
         public ControlTests(ITestOutputHelper atr)
         {
             DotMemoryUnitTestOutput.SetOutputMethod(atr.WriteLine);
         }
 
+
+        [Fact]
+        public void DataGrid_Is_Freed()
+        {
+            using (Start())
+            {
+                // When attached to INotifyCollectionChanged, DataGrid will subscribe to it's events, potentially causing leak
+                Func<Window> run = () =>
+                {
+                    var window = new Window
+                    {
+                        Content = new DataGrid
+                        {
+                            Items = _observableCollection
+                        }
+                    };
+
+                    window.Show();
+
+                    // Do a layout and make sure that DataGrid gets added to visual tree.
+                    window.LayoutManager.ExecuteInitialLayoutPass();
+                    Assert.IsType<DataGrid>(window.Presenter.Child);
+
+                    // Clear the content and ensure the DataGrid is removed.
+                    window.Content = null;
+                    window.LayoutManager.ExecuteLayoutPass();
+                    Assert.Null(window.Presenter.Child);
+
+                    return window;
+                };
+
+                var result = run();
+
+                dotMemory.Check(memory =>
+                    Assert.Equal(0, memory.GetObjects(where => where.Type.Is<DataGrid>()).ObjectsCount));
+            }
+        }
+
         [Fact]
         public void Canvas_Is_Freed()
         {