Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion binding/SkiaSharp/SKImage.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,13 @@ public static SKImage Create (SKImageInfo info)
var pixels = Marshal.AllocCoTaskMem (info.BytesSize);
using (var pixmap = new SKPixmap (info, pixels)) {
// don't use the managed version as that is just extra overhead which isn't necessary
return GetObject (SkiaApi.sk_image_new_raster (pixmap.Handle, DelegateProxies.SKImageRasterReleaseProxyForCoTaskMem, null));
var image = GetObject (SkiaApi.sk_image_new_raster (pixmap.Handle, DelegateProxies.SKImageRasterReleaseProxyForCoTaskMem, null));
if (image == null) {
Comment thread
mattleibow marked this conversation as resolved.
// the native image was not created, so the release proc will never run to
// free the buffer - free it now to avoid leaking the allocation
Marshal.FreeCoTaskMem (pixels);
}
return image;
}
}

Expand Down
47 changes: 47 additions & 0 deletions tests/Tests/SkiaSharp/SKImageTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,53 @@ public void TestNotLazyImage()
Assert.False(image.IsLazyGenerated);
}

[Fact]
public void FailedRasterCreateDoesNotLeakPixelBuffer()
{
// The leak is measured via System.Diagnostics.Process.PrivateMemorySize64, which is
// not supported on WASM (no System.Diagnostics.Process), iOS or Mac Catalyst. The fix
// itself is platform-agnostic and still exercised by the desktop and Android legs.
SkipOnPlatform(IsBrowser || IsIOS || IsMacCatalyst,
"System.Diagnostics.Process.PrivateMemorySize64 is not supported on WASM, iOS or Mac Catalyst");

// An Rgba8888 info with an Unknown alpha type has a positive BytesSize but is
// rejected by Skia, so sk_image_new_raster returns null. SKImage.Create must free
// the CoTaskMem pixel buffer it allocated, otherwise every failed call leaks it.
var info = new SKImageInfo(1024, 1024, SKColorType.Rgba8888, SKAlphaType.Unknown);
Assert.True(info.BytesSize > 0);

// sanity: creation really does fail for this info
Assert.Null(SKImage.Create(info));

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
Comment thread
Copilot marked this conversation as resolved.
Outdated

var process = System.Diagnostics.Process.GetCurrentProcess();
Comment thread
mattleibow marked this conversation as resolved.
Outdated
process.Refresh();
var before = process.PrivateMemorySize64;

const int count = 200;
for (var i = 0; i < count; i++)
{
var image = SKImage.Create(info);
Assert.Null(image);
}

GC.Collect();
GC.WaitForPendingFinalizers();
GC.Collect();
Comment thread
mattleibow marked this conversation as resolved.
Outdated

process.Refresh();
var grownBytes = process.PrivateMemorySize64 - before;
var allocatedBytes = (long)count * info.BytesSize; // ~400 MB if leaked

// If the buffer leaks, growth is a large fraction of allocatedBytes. When freed
// correctly, growth stays near zero. Use a generous threshold to avoid flakiness.
Assert.True(grownBytes < allocatedBytes / 4,
$"Private memory grew {grownBytes / (1024 * 1024)} MB over {count} failed SKImage.Create calls (~{allocatedBytes / (1024 * 1024)} MB allocated) - the pixel buffer is leaking.");
}

[Fact]
public void ToRasterImageReturnsSameRaster()
{
Expand Down
Loading