Skip to content

Commit c17df3d

Browse files
Fix arena double-release on ordered flush failure and harden disposal
An exception during an ordered flush skipped copying the executor's replacement arenas back to the caller, so the finally returned already-disposed arenas to the reuse caches; a later drain then released their buffers a second time, corrupting native memory. Arenas now copy back unconditionally, and both arena types dispose behind an interlocked flag, nulling each buffer so a stale reference fails CanReuse instead of touching freed memory. Disposal is hardened across both projects: guard flags set before releases, exception-safe multi-buffer creation, per-entry isolation in the deferred status harvest, retained-scene item release on the empty and exception paths of scene creation, root/region canvases sharing one deferred-image list, and decoration ink buffers owned by the glyph builder base with a chained virtual dispose. Software adapters are rejected at adapter selection via wgpuAdapterGetInfo; GPU-less environments are served by the CPU backend. The staged submission, dispatch, and scene-render probes and the ForceFallbackAdapter option are removed along with the CI diagnostic capture.
1 parent 1e262e3 commit c17df3d

24 files changed

Lines changed: 831 additions & 840 deletions

.github/workflows/build-and-test.yml

Lines changed: 0 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -208,32 +208,6 @@ jobs:
208208
SIXLABORS_TESTING_PREVIEW: True
209209
XUNIT_PATH: .\tests\ImageSharp.Drawing.Tests # Required for xunit
210210

211-
# Diagnostic capture for the WARP-only native fault: reruns the WebGPU suite with the
212-
# support probe bypassed so the fault can occur, collecting crash dumps as artifacts.
213-
# Never affects job status; remove once the fault is root-caused upstream.
214-
- name: WebGPU WARP Diagnostic
215-
if: ${{ runner.os == 'Windows' && matrix.options.sdk-preview != true }}
216-
continue-on-error: true
217-
timeout-minutes: 12
218-
shell: pwsh
219-
run: |
220-
New-Item -ItemType Directory -Force warp-dumps | Out-Null
221-
dotnet test tests/ImageSharp.Drawing.Tests -c Release -f ${{matrix.options.framework}} --filter "FullyQualifiedName~WebGPUDrawingBackendTests"
222-
env:
223-
SIXLABORS_TESTING: True
224-
SIXLABORS_WEBGPU_PROBE_BYPASS: 1
225-
DOTNET_DbgEnableMiniDump: 1
226-
DOTNET_DbgMiniDumpType: 2
227-
DOTNET_DbgMiniDumpName: ${{ github.workspace }}\warp-dumps\webgpu-crash-%p.dmp
228-
229-
- name: Export WebGPU Diagnostic Dumps
230-
uses: actions/upload-artifact@v7
231-
if: ${{ always() && runner.os == 'Windows' && matrix.options.sdk-preview != true }}
232-
with:
233-
name: webgpu_warp_dumps_${{ matrix.options.framework }}
234-
path: warp-dumps/
235-
if-no-files-found: ignore
236-
237211
- name: Export Failed Output
238212
uses: actions/upload-artifact@v7
239213
if: failure()

src/ImageSharp.Drawing.WebGPU/WebGPU.cs

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,22 @@ public void AdapterRequestDevice(
9292
public WGPUStatus AdapterGetLimits(WGPUAdapterImpl* adapter, WGPULimits* limits)
9393
=> WebGPUNative.wgpuAdapterGetLimits(adapter, limits);
9494

95+
/// <summary>
96+
/// Queries an adapter's identity and classification.
97+
/// </summary>
98+
/// <param name="adapter">The adapter to query.</param>
99+
/// <param name="info">Receives the adapter info; release with <see cref="AdapterInfoFreeMembers"/>.</param>
100+
/// <returns>The query status.</returns>
101+
public WGPUStatus AdapterGetInfo(WGPUAdapterImpl* adapter, WGPUAdapterInfo* info)
102+
=> WebGPUNative.wgpuAdapterGetInfo(adapter, info);
103+
104+
/// <summary>
105+
/// Releases the allocated string members of an adapter info value.
106+
/// </summary>
107+
/// <param name="info">The adapter info whose members are released.</param>
108+
public void AdapterInfoFreeMembers(WGPUAdapterInfo info)
109+
=> WebGPUNative.wgpuAdapterInfoFreeMembers(info);
110+
95111
/// <summary>
96112
/// Releases an adapter reference.
97113
/// </summary>

src/ImageSharp.Drawing.WebGPU/WebGPUDrawingBackend.Ordered.cs

Lines changed: 32 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -59,10 +59,21 @@ private void RenderOrderedScene<TPixel>(
5959
resourceArena,
6060
schedulingArena);
6161

62-
executor.Execute();
63-
currentBumpSizes = executor.BumpSizes;
64-
resourceArena = executor.ResourceArena;
65-
schedulingArena = executor.SchedulingArena;
62+
try
63+
{
64+
executor.Execute();
65+
currentBumpSizes = executor.BumpSizes;
66+
}
67+
finally
68+
{
69+
// The executor replaces its arenas in place when a range outgrows them, disposing
70+
// the instances these ref slots still name. The copy-back must run on the failure
71+
// path too: otherwise the caller's finally returns the disposed originals to the
72+
// reuse caches (a later release then frees their buffers twice) and the live
73+
// replacements held only by the executor leak.
74+
resourceArena = executor.ResourceArena;
75+
schedulingArena = executor.SchedulingArena;
76+
}
6677
}
6778

6879
/// <summary>
@@ -193,12 +204,23 @@ public WebGPUOrderedExecutor(
193204
}
194205
}
195206

196-
this.effectRenderer = new WebGPUEffectRenderer(
197-
flushContext,
198-
encodedScene.MaximumShaderEffectWidth,
199-
encodedScene.MaximumShaderEffectHeight,
200-
encodedScene.MaximumShaderUniformByteLength,
201-
maximumPassCount);
207+
try
208+
{
209+
this.effectRenderer = new WebGPUEffectRenderer(
210+
flushContext,
211+
encodedScene.MaximumShaderEffectWidth,
212+
encodedScene.MaximumShaderEffectHeight,
213+
encodedScene.MaximumShaderUniformByteLength,
214+
maximumPassCount);
215+
}
216+
catch
217+
{
218+
// A ctor throw means the caller's using never binds, so Dispose cannot retire
219+
// the callback owner and event created above.
220+
this.mapCallback?.Dispose();
221+
this.mapReady?.Dispose();
222+
throw;
223+
}
202224
}
203225
}
204226

src/ImageSharp.Drawing.WebGPU/WebGPUDrawingBackend.cs

Lines changed: 35 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
// Licensed under the Six Labors Split License.
33

44
using System.Runtime.CompilerServices;
5+
using System.Runtime.ExceptionServices;
56
using SixLabors.ImageSharp.Drawing.Processing.Backends.Native;
67
using SixLabors.ImageSharp.PixelFormats;
78

@@ -547,6 +548,14 @@ internal void ReturnArenas(
547548
// must be released immediately instead of being left for final backend disposal.
548549
WebGPUSceneResourceArena.Dispose(Interlocked.Exchange(ref this.cachedResourceArena, resourceArena));
549550
}
551+
552+
// Dispose can drain the cache between the guard above and these stores; sweep again
553+
// so an arena parked after that drain cannot outlive the backend.
554+
if (this.isDisposed)
555+
{
556+
WebGPUSceneSchedulingArena.Dispose(Interlocked.Exchange(ref this.cachedSchedulingArena, null));
557+
WebGPUSceneResourceArena.Dispose(Interlocked.Exchange(ref this.cachedResourceArena, null));
558+
}
550559
}
551560

552561
/// <summary>
@@ -860,11 +869,17 @@ public void Dispose()
860869

861870
// Retire deferred readbacks before their buffers. Each map is resolved or canceled here;
862871
// a callback that native WebGPU still owes remains self-rooted after a timeout and is
863-
// prevented from entering the disposed readback owner.
864-
this.HarvestPendingSchedulingStatuses(scene: null, drainAll: true);
865-
866-
WebGPUSceneSchedulingArena.Dispose(Interlocked.Exchange(ref this.cachedSchedulingArena, null));
867-
WebGPUSceneResourceArena.Dispose(Interlocked.Exchange(ref this.cachedResourceArena, null));
872+
// prevented from entering the disposed readback owner. The arena drain must run even
873+
// when the harvest throws, or both cached arenas leak with no retry possible.
874+
try
875+
{
876+
this.HarvestPendingSchedulingStatuses(null, true);
877+
}
878+
finally
879+
{
880+
WebGPUSceneSchedulingArena.Dispose(Interlocked.Exchange(ref this.cachedSchedulingArena, null));
881+
WebGPUSceneResourceArena.Dispose(Interlocked.Exchange(ref this.cachedResourceArena, null));
882+
}
868883
}
869884

870885
/// <summary>
@@ -942,6 +957,11 @@ private void HarvestPendingSchedulingStatuses(WebGPUDrawingBackendScene? scene,
942957
return;
943958
}
944959

960+
// Entries were already removed from the queue above, so a failure on one entry must not
961+
// abandon the rest: every remaining status would leak its readback buffer and its scene
962+
// reference, leaving the scene's deferred teardown permanently blocked. The first
963+
// failure is rethrown after every entry has been retired.
964+
Exception? firstFailure = null;
945965
foreach (PendingSchedulingStatusEntry entry in due)
946966
{
947967
try
@@ -956,11 +976,21 @@ private void HarvestPendingSchedulingStatuses(WebGPUDrawingBackendScene? scene,
956976
entry.CorrectiveRender?.Invoke();
957977
}
958978
}
979+
catch (Exception failure)
980+
{
981+
firstFailure ??= failure;
982+
entry.Status.Dispose();
983+
}
959984
finally
960985
{
961986
entry.Scene?.ReleaseDeferredRenderReference();
962987
}
963988
}
989+
990+
if (firstFailure is not null)
991+
{
992+
ExceptionDispatchInfo.Capture(firstFailure).Throw();
993+
}
964994
}
965995

966996
/// <summary>

src/ImageSharp.Drawing.WebGPU/WebGPUEnvironmentError.cs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,12 @@ public enum WebGPUEnvironmentError
3737
/// </summary>
3838
AdapterRequestFailed,
3939

40+
/// <summary>
41+
/// The only available adapter is a software rasterizer. Software adapters are not supported
42+
/// by this backend; the CPU drawing backend serves GPU-less environments instead.
43+
/// </summary>
44+
SoftwareAdapterUnsupported,
45+
4046
/// <summary>
4147
/// The device request callback did not complete before the timeout expired.
4248
/// </summary>

src/ImageSharp.Drawing.WebGPU/WebGPUEnvironmentOptions.cs

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,4 @@ public sealed class WebGPUEnvironmentOptions
2121
/// Gets or sets the adapter power preference requested for the library-managed device.
2222
/// </summary>
2323
public WebGPUPowerPreference PowerPreference { get; set; } = WebGPUPowerPreference.HighPerformance;
24-
25-
/// <summary>
26-
/// Gets or sets a value indicating whether adapter requests are restricted to the platform's
27-
/// software fallback adapter, such as WARP on Windows. Software adapters match the rendering
28-
/// environment of GPU-less continuous-integration machines.
29-
/// </summary>
30-
public bool ForceFallbackAdapter { get; set; }
3124
}

src/ImageSharp.Drawing.WebGPU/WebGPUFlushContext.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -655,6 +655,11 @@ public void Dispose()
655655
return;
656656
}
657657

658+
// The flag is set before any release so a throw mid-teardown cannot leave a
659+
// re-runnable context: a second Dispose over the still-populated lists would
660+
// release every tracked object twice and double-return pooled rentals.
661+
this.disposed = true;
662+
658663
// Ordering: end any open passes before releasing the encoder they were recorded on,
659664
// then release the encoder before the transient objects it may still reference.
660665
this.EndComputePassIfOpen();
@@ -732,8 +737,6 @@ public void Dispose()
732737

733738
this.pooledTextures.Clear();
734739
this.pooledUniformBuffers.Clear();
735-
736-
this.disposed = true;
737740
}
738741

739742
/// <summary>

src/ImageSharp.Drawing.WebGPU/WebGPUPendingSchedulingStatus.cs

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -400,17 +400,25 @@ public void Dispose()
400400
// readback resources twice. The wrapper retains its own root if native still owes a call.
401401
this.disposed = true;
402402

403-
if (!this.resolved)
403+
try
404404
{
405-
// Give the accepted map a bounded opportunity to finish before releasing its buffer.
406-
// A timeout is still safe because retiring the wrapper below suppresses a later call
407-
// into this owner while its self-root preserves the native function target.
408-
_ = this.TryResolve(out _);
405+
if (!this.resolved)
406+
{
407+
// Give the accepted map a bounded opportunity to finish before releasing its buffer.
408+
// A timeout is still safe because retiring the wrapper below suppresses a later call
409+
// into this owner while its self-root preserves the native function target.
410+
_ = this.TryResolve(out _);
411+
}
412+
}
413+
finally
414+
{
415+
// A resolve throw (device poll or map-range failure) must not skip the retirement:
416+
// the flag above already made this the only disposal pass, so the buffer, callback
417+
// owner, and event would otherwise leak for the process lifetime.
418+
this.ReleaseBuffer(unmap: false);
419+
this.callback.Dispose();
420+
this.mapReady.Dispose();
409421
}
410-
411-
this.ReleaseBuffer(unmap: false);
412-
this.callback.Dispose();
413-
this.mapReady.Dispose();
414422
}
415423

416424
/// <summary>

src/ImageSharp.Drawing.WebGPU/WebGPURuntime.DeviceSharedState.cs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -314,7 +314,7 @@ public DeviceSharedState(WebGPU api, WebGPUDeviceHandle deviceHandle)
314314
/// <summary>
315315
/// Gets the linear filtering sampler shared by layer-effect shader passes on this device.
316316
/// </summary>
317-
public WGPUSamplerImpl* EffectFilteringSampler { get; }
317+
public WGPUSamplerImpl* EffectFilteringSampler { get; private set; }
318318

319319
/// <summary>
320320
/// Gets a value indicating whether the native runtime has reported this device as lost.
@@ -991,6 +991,7 @@ public void Dispose()
991991
this.effectPipelines.Clear();
992992

993993
this.Api.SamplerRelease(this.EffectFilteringSampler);
994+
this.EffectFilteringSampler = null;
994995

995996
lock (this.statusReadbackSync)
996997
{

0 commit comments

Comments
 (0)