Merge pull request #3951 from MarchingCube/fix-path-geometry-leak

Make sure that Path does not stay subscribed to Data indefinitely.

src/Avalonia.Controls/Shapes/Path.cs

@@ -1,5 +1,4 @@
 using System;
-using Avalonia.Data;
 using Avalonia.Media;
 
 namespace Avalonia.Controls.Shapes
@@ -9,6 +8,8 @@ namespace Avalonia.Controls.Shapes
         public static readonly StyledProperty<Geometry> DataProperty =
             AvaloniaProperty.Register<Path, Geometry>(nameof(Data));
 
+        private EventHandler _geometryChangedHandler;
+
         static Path()
         {
             AffectsGeometry<Path>(DataProperty);
@@ -21,21 +22,48 @@ namespace Avalonia.Controls.Shapes
             set { SetValue(DataProperty, value); }
         }
 
+        private EventHandler GeometryChangedHandler => _geometryChangedHandler ??= GeometryChanged;
+
         protected override Geometry CreateDefiningGeometry() => Data;
 
+        protected override void OnAttachedToVisualTree(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnAttachedToVisualTree(e);
+
+            if (Data is object)
+            {
+                Data.Changed += GeometryChangedHandler;
+            }
+        }
+
+        protected override void OnDetachedFromVisualTree(VisualTreeAttachmentEventArgs e)
+        {
+            base.OnDetachedFromVisualTree(e);
+
+            if (Data is object)
+            {
+                Data.Changed -= GeometryChangedHandler;
+            }
+        }
+
         private void DataChanged(AvaloniaPropertyChangedEventArgs e)
         {
+            if (VisualRoot is null)
+            {
+                return;
+            }
+
             var oldGeometry = (Geometry)e.OldValue;
             var newGeometry = (Geometry)e.NewValue;
 
             if (oldGeometry is object)
             {
-                oldGeometry.Changed -= GeometryChanged;
+                oldGeometry.Changed -= GeometryChangedHandler;
             }
 
             if (newGeometry is object)
             {
-                newGeometry.Changed += GeometryChanged;
+                newGeometry.Changed += GeometryChangedHandler;
             }
         }
 

tests/Avalonia.Controls.UnitTests/Shapes/PathTests.cs

@@ -23,12 +23,16 @@ namespace Avalonia.Controls.UnitTests.Shapes
             var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) };
             var target = new Path { Data = geometry };
 
+            var root = new TestRoot(target);
+
             target.Measure(Size.Infinity);
             Assert.True(target.IsMeasureValid);
 
             geometry.Rect = new Rect(0, 0, 20, 20);
 
             Assert.False(target.IsMeasureValid);
+
+            root.Child = null;
         }
     }
 }

tests/Avalonia.LeakTests/ControlTests.cs

@@ -3,6 +3,7 @@ using System.Collections.Generic;
 using System.Linq;
 using System.Runtime.Remoting.Contexts;
 using Avalonia.Controls;
+using Avalonia.Controls.Shapes;
 using Avalonia.Controls.Templates;
 using Avalonia.Diagnostics;
 using Avalonia.Input;
@@ -492,6 +493,45 @@ namespace Avalonia.LeakTests
             }
         }
 
+        [Fact]
+        public void Path_Is_Freed()
+        {
+            using (Start())
+            {
+                var geometry = new EllipseGeometry { Rect = new Rect(0, 0, 10, 10) };
+
+                Func<Window> run = () =>
+                {
+                    var window = new Window
+                    {
+                        Content = new Path
+                        {
+                            Data = geometry
+                        }
+                    };
+
+                    window.Show();
+
+                    window.LayoutManager.ExecuteInitialLayoutPass(window);
+                    Assert.IsType<Path>(window.Presenter.Child);
+
+                    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<Path>()).ObjectsCount));
+
+                // We are keeping geometry alive to simulate a resource that outlives the control.
+                GC.KeepAlive(geometry);
+            }
+        }
+
         private IDisposable Start()
         {
             return UnitTestApplication.Start(TestServices.StyledWindow.With(