From ea2dc8fef7860b09435076ef8ebb236fa5e783d6 Mon Sep 17 00:00:00 2001 From: GreySaturn <62218218+ddunham121@users.noreply.github.com> Date: Wed, 26 Aug 2026 09:22:17 +0000 Subject: [PATCH] Fix ServerCompositionDrawingSurface orphaning GPU snapshots when an update is processed after Dispose (#21876) * Add failing test: CompositionDrawingSurface update processed after Dispose orphans the GPU snapshot A commit batch is processed on the render thread in serialization order: the dispose list is written before queued server jobs. The legal user-code order `surface.UpdateAsync(image); surface.Dispose();` therefore executes on the render thread as Dispose() -> UpdateWithAutomaticSync(), and the update stores a fresh snapshot ref into the already-disposed ServerCompositionDrawingSurface. Nothing disposes that ref afterwards, so the GPU-backed snapshot is cleaned up by the RefCountable.Ref critical finalizer, which performs GPU work on the finalizer thread and races the compositor render loop (#21865). * Fix ServerCompositionDrawingSurface orphaning GPU snapshots when an update is processed after Dispose Add a disposed flag: an Update processed after Dispose now disposes the fresh snapshot immediately on the render thread (where the GPU context is current) instead of storing it into the disposed surface where nothing will ever release it. This removes the finalizer-thread GPU cleanup path that races the compositor render loop and crashes in sk_canvas_flush (#21865). --- .../Server/ServerCompositionDrawingSurface.cs | 14 ++ .../CompositionDrawingSurfaceTests.cs | 138 ++++++++++++++++++ 2 files changed, 152 insertions(+) create mode 100644 tests/Avalonia.Base.UnitTests/Composition/CompositionDrawingSurfaceTests.cs diff --git a/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositionDrawingSurface.cs b/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositionDrawingSurface.cs index 04350902a6..7430be040b 100644 --- a/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositionDrawingSurface.cs +++ b/src/Avalonia.Base/Rendering/Composition/Server/ServerCompositionDrawingSurface.cs @@ -9,6 +9,7 @@ internal class ServerCompositionDrawingSurface : ServerCompositionSurface, IDisp { private IRef? _bitmap; private IPlatformRenderInterfaceContext? _createdWithContext; + private bool _disposed; public override IRef? Bitmap { get @@ -41,6 +42,17 @@ internal class ServerCompositionDrawingSurface : ServerCompositionSurface, IDisp void Update(IBitmapImpl newImage, IPlatformRenderInterfaceContext context) { + if (_disposed) + { + // Batches are processed with disposals before server jobs, so an update job + // can be processed after this surface was disposed in the same batch. + // Dispose the snapshot here on the render thread (context is current) + // instead of orphaning it: an orphaned ref is only ever released by the + // Ref critical finalizer, which then performs GPU work on the finalizer + // thread and races the render loop (#21865). + newImage.Dispose(); + return; + } _bitmap?.Dispose(); _bitmap = RefCountable.Create(newImage); _createdWithContext = context; @@ -92,5 +104,7 @@ internal class ServerCompositionDrawingSurface : ServerCompositionSurface, IDisp public void Dispose() { _bitmap?.Dispose(); + _bitmap = null; + _disposed = true; } } diff --git a/tests/Avalonia.Base.UnitTests/Composition/CompositionDrawingSurfaceTests.cs b/tests/Avalonia.Base.UnitTests/Composition/CompositionDrawingSurfaceTests.cs new file mode 100644 index 0000000000..a3eaa8d08a --- /dev/null +++ b/tests/Avalonia.Base.UnitTests/Composition/CompositionDrawingSurfaceTests.cs @@ -0,0 +1,138 @@ +using System; +using System.Collections.Generic; +using System.IO; +using System.Threading.Tasks; +using Avalonia.Media.Imaging; +using Avalonia.Platform; +using Avalonia.Rendering.Composition; +using Avalonia.UnitTests; +using Xunit; + +namespace Avalonia.Base.UnitTests.Composition; + +public class CompositionDrawingSurfaceTests : ScopedTestBase +{ + private class TrackingBitmapImpl : IBitmapImpl + { + public bool IsDisposed { get; private set; } + public Vector Dpi => new(96, 96); + public PixelSize PixelSize => new(1, 1); + public int Version => 1; + public void Save(Stream stream, BitmapEncoderOptions options) => throw new NotSupportedException(); + public void Dispose() => IsDisposed = true; + } + + private class FakeImportedImage : IPlatformRenderInterfaceImportedImage + { + public TrackingBitmapImpl LastSnapshot { get; private set; } = null!; + + private IBitmapImpl Snapshot() => LastSnapshot = new TrackingBitmapImpl(); + + public IBitmapImpl SnapshotWithKeyedMutex(uint acquireIndex, uint releaseIndex) => Snapshot(); + public IBitmapImpl SnapshotWithSemaphores( + IPlatformRenderInterfaceImportedSemaphore waitForSemaphore, + IPlatformRenderInterfaceImportedSemaphore signalSemaphore) => Snapshot(); + public IBitmapImpl SnapshotWithTimelineSemaphores( + IPlatformRenderInterfaceImportedSemaphore waitForSemaphore, ulong waitForValue, + IPlatformRenderInterfaceImportedSemaphore signalSemaphore, ulong signalValue) => Snapshot(); + public IBitmapImpl SnapshotWithAutomaticSync() => Snapshot(); + public void Dispose() + { + } + } + + private class FakeExternalObjectsFeature : IExternalObjectsRenderInterfaceContextFeature + { + public FakeImportedImage Image { get; } = new(); + + public IReadOnlyList SupportedImageHandleTypes => new[] { "Fake" }; + public IReadOnlyList SupportedSemaphoreTypes => Array.Empty(); + public byte[]? DeviceUuid => null; + public byte[]? DeviceLuid => null; + + public IPlatformRenderInterfaceImportedImage ImportImage(IPlatformHandle handle, + PlatformGraphicsExternalImageProperties properties) => Image; + + public IPlatformRenderInterfaceImportedImage ImportImage(ICompositionImportableSharedGpuContextImage image) => + Image; + + public IPlatformRenderInterfaceImportedSemaphore ImportSemaphore(IPlatformHandle handle) => + throw new NotSupportedException(); + + public CompositionGpuImportedImageSynchronizationCapabilities GetSynchronizationCapabilities( + string imageHandleType) => CompositionGpuImportedImageSynchronizationCapabilities.Automatic; + } + + [Fact] + public async Task Update_Processed_After_Dispose_Should_Dispose_Snapshot_Instead_Of_Orphaning_It() + { + // A commit batch is processed on the render thread in serialization order: + // the dispose list is written (and therefore processed) BEFORE queued server + // jobs. This means the perfectly legal user-code order + // surface.UpdateAsync(image); surface.Dispose(); + // executes on the render thread as Dispose() -> UpdateWithAutomaticSync(). + // Without a disposed-guard, the update stores a fresh snapshot into the + // already-disposed surface. Nothing ever disposes that ref again, so the + // GPU-backed snapshot is left to the RefCountable.Ref critical finalizer, + // which performs GPU work (context MakeCurrent + native SKImage dispose) on + // the finalizer thread and races the compositor render loop. See #21865. + using var services = new CompositorTestServices(); + var compositor = services.Compositor; + + var feature = new FakeExternalObjectsFeature(); + var interop = new CompositionInterop(compositor, feature); + var imported = interop.ImportImage( + new PlatformHandle(new IntPtr(1), "Fake"), + new PlatformGraphicsExternalImageProperties { Width = 1, Height = 1 }); + + services.RunJobs(); + Assert.True(((CompositionImportedGpuImage)imported).ImportCompleted.IsCompletedSuccessfully); + + var surface = compositor.CreateDrawingSurface(); + + var update = surface.UpdateAsync(imported); + surface.Dispose(); + + services.RunJobs(); + await update; + + var snapshot = feature.Image.LastSnapshot; + Assert.NotNull(snapshot); + Assert.True(snapshot.IsDisposed, + "The snapshot taken by an update processed after the surface was disposed " + + "must be disposed on the render thread instead of being orphaned to the finalizer."); + } + + [Fact] + public async Task Update_Before_Dispose_In_Separate_Batches_Should_Dispose_Snapshot_With_The_Surface() + { + // Baseline: when the update is processed in an earlier batch than the dispose, + // the surface's Dispose() releases the stored snapshot. This already works and + // must keep working with the disposed-guard in place. + using var services = new CompositorTestServices(); + var compositor = services.Compositor; + + var feature = new FakeExternalObjectsFeature(); + var interop = new CompositionInterop(compositor, feature); + var imported = interop.ImportImage( + new PlatformHandle(new IntPtr(1), "Fake"), + new PlatformGraphicsExternalImageProperties { Width = 1, Height = 1 }); + + services.RunJobs(); + + var surface = compositor.CreateDrawingSurface(); + + var update = surface.UpdateAsync(imported); + services.RunJobs(); + await update; + + var snapshot = feature.Image.LastSnapshot; + Assert.NotNull(snapshot); + Assert.False(snapshot.IsDisposed); + + surface.Dispose(); + services.RunJobs(); + + Assert.True(snapshot.IsDisposed); + } +}