Repository navigation
Fix ServerCompositionDrawingSurface orphaning GPU snapshots when an update is processed after Dispose - #21876
Conversation
…spose 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<T> critical finalizer, which performs GPU work on the finalizer thread and races the compositor render loop (AvaloniaUI#21865).
…pdate 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 (AvaloniaUI#21865).
MrJul
added
bug
backport-candidate-12.1.x
|
You can test this PR using the following package version. |
|
|
@cla-avalonia agree |
|
While the fix looks solid and protects against the update occurring after disposal, I'd argue that the following claim isn't valid:
The This comment was true when it was written, but it no longer holds now that I'm inclined to accept this PR, as the hardening is good, but we should fix the GL swapchains. The Vulkan one is correctly awaiting: Avalonia/samples/GpuInterop/VulkanDemo/VulkanSwapchain.cs Lines 67 to 68 in 4357a8d cc @kekekeks for additional thoughts. |
|
I think it's a valid change just for defensiveness. Dispose should generally avoid breaking the application. E. g. when you dispose Socket with pending SendAsync, the it doesn't suddenly leak memory, so Dispose effectively cancelling pending operations is an established pattern in .NET |
…pdate is processed after Dispose (AvaloniaUI#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<T> critical finalizer, which performs GPU work on the finalizer thread and races the compositor render loop (AvaloniaUI#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 (AvaloniaUI#21865).
MrJul
added
backported-12.1.x
and removed
backport-candidate-12.1.x
What does the pull request do?
Fixes
ServerCompositionDrawingSurfaceorphaning GPU-backed snapshots when an update job is processed after the surface's disposal, which is the root cause of the native AV insidesk_canvas_flushreported in #21865.What is the root cause?
A commit batch is serialized (and therefore processed on the render thread) with the dispose list before queued server jobs. So the perfectly legal user-code order:
executes on the render thread as
Dispose()→UpdateWithKeyedMutex/…().Update()has no disposed state, so it stores a fresh snapshot ref into the already-disposed surface. Nothing ever disposes that ref again. The GPU-backedImmutableBitmapinside it is then released by theRefCountable.Ref<T>critical finalizer, whose dispose callback performs GPU work (EglContext.MakeCurrent()+ nativeSKImagedispose) on the .NET finalizer thread, racing the compositor render loop mid-Blit/sk_canvas_flush.This is not theoretical: in a production crash dump attached to #21865 (details in a follow-up comment there), the finalizer thread was captured exactly in
RefCountable.Ref<T>.Finalize() → ImmutableBitmap.Dispose() → GlSkiaImportedImage.<TakeSnapshot>b__0 → EglContext.MakeCurrent()while the render thread died inSurfaceRenderTarget.Blit → sk_canvas_flush.What is the fix?
ServerCompositionDrawingSurfacegets a_disposedflag. AnUpdateprocessed afterDisposenow disposes the fresh snapshot immediately on the render thread — where the GPU context is current — instead of orphaning it to the finalizer.Per the contributing guide, the first commit adds the failing test (fails on current
main: the snapshot is orphaned undisposed), the second commit fixes it. The test exercises the real commit/batch pipeline viaCompositorTestServiceswith a fake external-objects feature, so the dispose-before-jobs ordering is demonstrated end-to-end, headlessly.Field validation
We ship a desktop app whose users hit this crash across a wide range of GPUs (RTX 4070 12GB through 128MB-VRAM Intel iGPUs — dumps for two distinct libSkiaSharp fault sites are attached to #21865). We have been running this exact fix (backported to 11.3.18) in our production build since 2026-07-28.
Checklist