Merge pull request #880 from kekekeks/disposal

Make sure that Renderer is disposed when TopLevel is closed

src/Avalonia.Controls/TopLevel.cs

@@ -184,7 +184,7 @@ namespace Avalonia.Controls
         /// <summary>
         /// Gets the renderer for the window.
         /// </summary>
-        public IRenderer Renderer { get; }
+        public IRenderer Renderer { get; private set; }
 
         /// <summary>
         /// Gets the access key handler for the window.
@@ -381,6 +381,8 @@ namespace Avalonia.Controls
         private void HandleClosed()
         {
             Closed?.Invoke(this, EventArgs.Empty);
+            Renderer?.Dispose();
+            Renderer = null;
             _applicationLifecycle.OnExit -= OnApplicationExiting;
         }
 

src/Gtk/Avalonia.Gtk3/TopLevelImpl.cs

@@ -75,7 +75,7 @@ namespace Avalonia.Gtk3
 
         private bool OnDestroy(IntPtr gtkwidget, IntPtr userdata)
         {
-            Closed?.Invoke();
+            Dispose();
             return false;
         }
 
@@ -210,7 +210,9 @@ namespace Avalonia.Gtk3
 
         public void Dispose()
         {
-            Closed?.Invoke();
+            //We are calling it here, since signal handler will be detached
+            if (!GtkWidget.IsClosed)
+                Closed?.Invoke();
             foreach(var d in Disposables.AsEnumerable().Reverse())
                 d.Dispose();
             Disposables.Clear();

src/Windows/Avalonia.Win32/WindowImpl.cs

@@ -182,9 +182,18 @@ namespace Avalonia.Win32
 
         public void Dispose()
         {
-            s_instances.Remove(this);
+            _framebuffer?.Dispose();
-            _framebuffer.Dispose();
+            _framebuffer = null;
-            UnmanagedMethods.DestroyWindow(_hwnd);
+            if (_hwnd != IntPtr.Zero)
+            {
+                UnmanagedMethods.DestroyWindow(_hwnd);
+                _hwnd = IntPtr.Zero;
+            }
+            if (_className != null)
+            {
+                UnmanagedMethods.UnregisterClass(_className, UnmanagedMethods.GetModuleHandle(null));
+                _className = null;
+            }
         }
 
         public void Hide()
@@ -418,12 +427,13 @@ namespace Avalonia.Win32
                     return IntPtr.Zero;
 
                 case UnmanagedMethods.WindowsMessage.WM_DESTROY:
-                    if (Closed != null)
+                    //Window doesn't exist anymore
-                    {
+                    _hwnd = IntPtr.Zero;
-                        UnmanagedMethods.UnregisterClass(_className, UnmanagedMethods.GetModuleHandle(null));
+                    //Remove root reference to this class, so unmanaged delegate can be collected
-                        Closed();
+                    s_instances.Remove(this);
-                    }
+                    Closed?.Invoke();
-
+                    //Free other resources
+                    Dispose();
                     return IntPtr.Zero;
 
                 case UnmanagedMethods.WindowsMessage.WM_DPICHANGED:

tests/Avalonia.LeakTests/ControlTests.cs

@@ -11,6 +11,7 @@ using Avalonia.Controls.Primitives;
 using Avalonia.Controls.Templates;
 using Avalonia.Diagnostics;
 using Avalonia.Layout;
+using Avalonia.Platform;
 using Avalonia.Rendering;
 using Avalonia.Styling;
 using Avalonia.UnitTests;
@@ -301,6 +302,33 @@ namespace Avalonia.LeakTests
             }
         }
 
+
+        [Fact]
+        public void RendererIsDisposed()
+        {
+            using (UnitTestApplication.Start(TestServices.StyledWindow))
+            {
+                var renderer = new Mock<IRenderer>();
+                renderer.Setup(x => x.Dispose());
+                var impl = new Mock<IWindowImpl>();
+                impl.SetupGet(x => x.Scaling).Returns(1);
+                impl.SetupProperty(x => x.Closed);
+                impl.Setup(x => x.Dispose()).Callback(() => impl.Object.Closed());
+
+                AvaloniaLocator.CurrentMutable.Bind<IWindowingPlatform>()
+                    .ToConstant(new MockWindowingPlatform(() => impl.Object));
+                AvaloniaLocator.CurrentMutable.Bind<IRendererFactory>()
+                    .ToConstant(new MockRendererFactory(renderer.Object));
+                var window = new Window()
+                {
+                    Content = new Button()
+                };
+                window.Show();
+                window.Close();
+                renderer.Verify(r => r.Dispose());
+            }
+        }
+
         private static void PurgeMoqReferences()
         {
             // Moq holds onto references in its mock of IRenderer in case we want to check if a method has been called;

tests/Avalonia.UnitTests/Avalonia.UnitTests.csproj

@@ -55,6 +55,7 @@
   </ItemGroup>
   <ItemGroup>
     <Compile Include="InvariantCultureFixture.cs" />
+    <Compile Include="MockRendererFactory.cs" />
     <Compile Include="NotifyingBase.cs" />
     <Compile Include="TestLogSink.cs" />
     <Compile Include="TestTemplatedRoot.cs" />

tests/Avalonia.UnitTests/MockRendererFactory.cs

@@ -0,0 +1,29 @@
+using System;
+using System.Collections.Generic;
+using System.Linq;
+using System.Text;
+using System.Threading.Tasks;
+using Avalonia.Rendering;
+
+namespace Avalonia.UnitTests
+{
+    public class MockRendererFactory : IRendererFactory
+    {
+        private readonly Func<IRenderRoot, IRenderLoop, IRenderer> _cb;
+
+        public MockRendererFactory(Func<IRenderRoot, IRenderLoop, IRenderer> cb = null)
+        {
+            _cb = cb;
+        }
+
+        public MockRendererFactory(IRenderer renderer) : this((_, __) => renderer)
+        {
+
+        }
+
+        public IRenderer CreateRenderer(IRenderRoot root, IRenderLoop renderLoop)
+        {
+            return _cb?.Invoke(root, renderLoop);
+        }
+    }
+}