From 7f456057cdad1c64253d208091112f63bca7456f Mon Sep 17 00:00:00 2001 From: Nikita Tsukanov Date: Mon, 24 Aug 2026 13:41:38 +0000 Subject: [PATCH] Raise TopLevel.Closed from managed Dispose() paths (#22045) TopLevel teardown (HandleClosed) was only ever triggered by the backend invoking ITopLevelImpl.Closed. Browser, iOS, Android, macOS and the offscreen (designer previewer) impls never raise it from Dispose(), so EmbeddableControlRoot.Closed was never raised there and StopRendering() was never reached - the top level stayed registered in MediaContext forever. Make teardown idempotent behind EnsureClosed() and call it from the managed Dispose() paths (EmbeddableControlRoot, OffscreenTopLevel, PopupRoot). The guard lives in a new non-virtual entry point because WindowBase and Window override HandleClosed and run side effects before calling base. Also drop ChoreographerTimer's view-visibility gate on Android. It predates the render timer rewrite and vetoed ticks that DefaultRenderLoop had explicitly asked for, so the synchronous compositor round-trip in HandleClosed (Renderer.Dispose -> MediaContext.SyncDisposeCompositionTarget) could never complete once the view had unsubscribed - deadlocking every activity destroy. DefaultRenderLoop already owns the sleep/wake state machine, driven by StartRendering/StopRendering, which makes the extra gate redundant. Verified at runtime on X11, Wayland, Win32, macOS, Headless, Browser (WASM), Android and iOS: Closed fires exactly once per teardown, MediaContext returns to baseline, and platform-initiated closes still fire exactly once. Co-authored-by: Claude Opus 5 (1M context) --- src/Android/Avalonia.Android/AvaloniaView.cs | 15 ++-- .../Avalonia.Android/ChoreographerTimer.cs | 22 +----- .../Embedding/EmbeddableControlRoot.cs | 2 +- .../Embedding/Offscreen/OffscreenTopLevel.cs | 1 + src/Avalonia.Controls/Primitives/PopupRoot.cs | 1 + src/Avalonia.Controls/TopLevel.cs | 15 +++- .../TopLevelTests.cs | 76 +++++++++++++++++++ 7 files changed, 99 insertions(+), 33 deletions(-) diff --git a/src/Android/Avalonia.Android/AvaloniaView.cs b/src/Android/Avalonia.Android/AvaloniaView.cs index b2decd5fa2..96a95b8a12 100644 --- a/src/Android/Avalonia.Android/AvaloniaView.cs +++ b/src/Android/Avalonia.Android/AvaloniaView.cs @@ -23,7 +23,7 @@ namespace Avalonia.Android private readonly ViewImpl _view; private readonly ExploreByTouchHelper _accessHelper; - private IDisposable? _timerSubscription; + private bool _isRendering; private bool _surfaceCreated; public AvaloniaView(Context context) : base(context) @@ -106,13 +106,9 @@ namespace Avalonia.Android if (_root == null || !_surfaceCreated) return; - if (isVisible && _timerSubscription == null) + if (isVisible && !_isRendering) { - if (AndroidPlatform.Timer is { } timer) - { - _timerSubscription = timer.SubscribeView(this); - } - + _isRendering = true; _root.StartRendering(); if (_view.TryGetFeature(out var insetsManager) == true) @@ -120,11 +116,10 @@ namespace Avalonia.Android (insetsManager as AndroidInsetsManager)?.ApplyStatusBarState(); } } - else if (!isVisible && _timerSubscription != null) + else if (!isVisible && _isRendering) { + _isRendering = false; _root.StopRendering(); - _timerSubscription?.Dispose(); - _timerSubscription = null; } } diff --git a/src/Android/Avalonia.Android/ChoreographerTimer.cs b/src/Android/Avalonia.Android/ChoreographerTimer.cs index 9bc8e78a52..d31a0aa915 100644 --- a/src/Android/Avalonia.Android/ChoreographerTimer.cs +++ b/src/Android/Avalonia.Android/ChoreographerTimer.cs @@ -1,9 +1,7 @@ using System; -using System.Collections.Generic; using System.Runtime.InteropServices; using System.Threading; using System.Threading.Tasks; -using Avalonia.Reactive; using Avalonia.Rendering; using static Avalonia.Android.Platform.SkiaPlatform.AndroidFramebuffer; @@ -17,7 +15,6 @@ namespace Avalonia.Android private readonly TaskCompletionSource _choreographer = new(); private readonly AutoResetEvent _event = new(false); private readonly GCHandle _timerHandle; - private readonly HashSet _views = new(); private Action? _tick; private bool _pendingCallback; private long _lastTime; @@ -49,23 +46,6 @@ namespace Avalonia.Android } } - internal IDisposable SubscribeView(AvaloniaView view) - { - lock (_lock) - { - _views.Add(view); - PostFrameCallbackIfNeeded(); - } - - return Disposable.Create( - () => - { - lock (_lock) - _views.Remove(view); - } - ); - } - private void Loop() { Looper.Prepare(); @@ -94,7 +74,7 @@ namespace Avalonia.Android if(_pendingCallback) return; - if (_tick == null || _views.Count == 0) + if (_tick == null) return; _pendingCallback = true; diff --git a/src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs b/src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs index 59718c3e3f..b65db54a93 100644 --- a/src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs +++ b/src/Avalonia.Controls/Embedding/EmbeddableControlRoot.cs @@ -69,7 +69,7 @@ namespace Avalonia.Controls.Embedding public void Dispose() { PlatformImpl?.Dispose(); - LayoutManager?.Dispose(); + EnsureClosed(); } } } diff --git a/src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs b/src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs index a87e36d00d..65ca492f1f 100644 --- a/src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs +++ b/src/Avalonia.Controls/Embedding/Offscreen/OffscreenTopLevel.cs @@ -35,6 +35,7 @@ namespace Avalonia.Controls.Embedding.Offscreen public void Dispose() { PlatformImpl?.Dispose(); + EnsureClosed(); } } } diff --git a/src/Avalonia.Controls/Primitives/PopupRoot.cs b/src/Avalonia.Controls/Primitives/PopupRoot.cs index 93a30b06a3..234f47258d 100644 --- a/src/Avalonia.Controls/Primitives/PopupRoot.cs +++ b/src/Avalonia.Controls/Primitives/PopupRoot.cs @@ -126,6 +126,7 @@ namespace Avalonia.Controls.Primitives public void Dispose() { PlatformImpl?.Dispose(); + EnsureClosed(); } private void UpdatePosition() diff --git a/src/Avalonia.Controls/TopLevel.cs b/src/Avalonia.Controls/TopLevel.cs index ac5f6693e8..2fd1524cef 100644 --- a/src/Avalonia.Controls/TopLevel.cs +++ b/src/Avalonia.Controls/TopLevel.cs @@ -123,6 +123,7 @@ namespace Avalonia.Controls private readonly IDisposable? _backGestureSubscription; private readonly Dictionary _platformImplBindings = new(); private double _scaling; + private bool _isClosed; private Size _clientSize; private Size? _frameSize; private WindowTransparencyLevel _actualTransparencyLevel; @@ -233,7 +234,7 @@ namespace Avalonia.Controls - impl.Closed = HandleClosed; + impl.Closed = EnsureClosed; impl.Paint = HandlePaint; impl.Resized = HandleResized; impl.ScalingChanged += HandleScalingChanged; @@ -661,6 +662,18 @@ namespace Avalonia.Controls private protected void StopRendering() => MediaContext.Instance.RemoveTopLevel(this); + /// + /// Runs the top level teardown exactly once, no matter whether it was initiated by the + /// platform via or by a managed Dispose(). + /// + private protected void EnsureClosed() + { + if (_isClosed) + return; + _isClosed = true; + HandleClosed(); + } + /// /// Handles a closed notification from . /// diff --git a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs index 15c1f650da..d48f69e710 100644 --- a/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs +++ b/tests/Avalonia.Controls.UnitTests/TopLevelTests.cs @@ -1,4 +1,5 @@ using System; +using Avalonia.Controls.Embedding; using Avalonia.Controls.Presenters; using Avalonia.Controls.Templates; using Avalonia.Input; @@ -166,6 +167,81 @@ namespace Avalonia.Controls.UnitTests } } + [Fact] + public void Impl_Close_Should_Raise_Closed_Event_Only_Once() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = CreateMockTopLevelImpl(true); + + var raised = 0; + var target = new TestTopLevel(impl.Object); + target.Closed += (s, e) => raised++; + + impl.Object.Closed!(); + impl.Object.Closed!(); + + Assert.Equal(1, raised); + } + } + + [Fact] + public void EmbeddableControlRoot_Dispose_Should_Raise_Closed_Event() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = CreateMockTopLevelImpl(true); + + var raised = 0; + var target = new EmbeddableControlRoot(impl.Object); + target.Closed += (s, e) => raised++; + + target.Dispose(); + + Assert.Equal(1, raised); + } + } + + [Fact] + public void EmbeddableControlRoot_Dispose_After_Impl_Close_Should_Raise_Closed_Event_Only_Once() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = CreateMockTopLevelImpl(true); + + var raised = 0; + var target = new EmbeddableControlRoot(impl.Object); + target.Closed += (s, e) => raised++; + + impl.Object.Closed!(); + target.Dispose(); + + Assert.Equal(1, raised); + } + } + + [Fact] + public void EmbeddableControlRoot_Dispose_Should_Dispose_Impl_Before_Teardown() + { + using (UnitTestApplication.Start(TestServices.StyledWindow)) + { + var impl = CreateMockTopLevelImpl(true); + + var closedRaised = false; + var implDisposedBeforeClosed = false; + impl.Setup(x => x.Dispose()).Callback(() => implDisposedBeforeClosed = !closedRaised); + + var target = new EmbeddableControlRoot(impl.Object); + target.Closed += (s, e) => closedRaised = true; + + target.Dispose(); + + impl.Verify(x => x.Dispose(), Times.Once); + Assert.True(implDisposedBeforeClosed); + Assert.True(closedRaised); + } + } + [Fact] public void Impl_Close_Should_Raise_DetachedFromLogicalTree_Event() {