PNG decoder: chunk buffer is still not disposed when cancellation is observed just after the chunk-data read (follow-up to #3202) #3215

Prerequisites

• I have written a descriptive issue title
• I have verified that I am running the latest version of ImageSharp — 4.1.3, checked on 2026-10-10
• I have verified if the problem exist in both DEBUG and RELEASE mode — both builds of the repro were run; the output is identical apart from the build= line
• I have searched open and closed issues — searches for ReadChunkData, TryReadChunk, ValidateChunk, ReadChunkCrc and for cancellation / undisposed-allocation reports found only PNG decoder: buffer allocated in ReadChunkData is not disposed when the read is cancelled #3202, which is closed.

ImageSharp version

4.1.3 (4.1.3+751037d0c6384c97e77ee1abd86e2203ae8dbd95)

Other ImageSharp packages and versions

None.

Environment (Operating system, version and so on)

Windows x64 (Microsoft Windows 10.0.26300).

.NET Framework version

.NET 10.0.12.

Description

This is a follow-up to #3202.

#3203 (backported to 4.1.x as #3209 and released in 4.1.3) put a try/catch
around the stream read in PngDecoderCore.ReadChunkData. That path is fixed
in 4.1.3, and the repro below confirms it (case A).

The rented buffer is still unowned for a few more statements after
ReadChunkData returns. TryReadChunk builds the chunk from it, validates the
CRC, and for IDAT/fdAT moves the stream back to the start of the data —
all before it hands the chunk to the loop whose finally disposes
chunk.Data. Two of those statements check the cancellation token:

1. ValidateChunk → ReadChunkCrc → BufferedReadStream.Read, which begins
   with CancellationToken.ThrowIfCancellationRequested().
2. this.currentStream.Position = position for IDAT/fdAT: the
   BufferedReadStream.Position setter makes the same check.

If a cancellation is first observed at either of them, the exception leaves
TryReadChunk and nothing disposes the buffer. ValidateChunk disposes it
only when the CRC does not match.

The effect is the one #3202 described:
MemoryDiagnostics.TotalUndisposedAllocationCount stays one above its
baseline after the cancelled operation. As there, this demonstrates missing
explicit disposal, not that the memory necessarily stays allocated after
finalization.

Source at the v4.1.3 commit:

• TryReadChunk: the buffer is rented (L2375), the chunk is validated (L2377),
  the position is restored for IDAT/fdAT (L2383), and only then does it
  return:
  

  ImageSharp/src/ImageSharp/Formats/Png/PngDecoderCore.cs

  Lines 2371 to 2386 in 751037d

  	position = this.currentStream.Position;
  	chunk = new PngChunk(
  	length: (int)Math.Min(length, this.currentStream.Length - position),
  	type: type,
  	data: this.ReadChunkData(length));
  	this.ValidateChunk(chunk, buffer);
  	// Restore the stream position for IDAT and fdAT chunks, because it will be decoded later and
  	// was only read to verifying the CRC is correct.
  	if (type is PngChunkType.Data or PngChunkType.FrameData)
  	{
  	this.currentStream.Position = position;
  	}
  	return true;

• ReadChunkData, with the guard from fix: Dispose chunk data buffer in ReadChunkData when read is cancelled #3203 around the read only:
  

  ImageSharp/src/ImageSharp/Formats/Png/PngDecoderCore.cs

  Lines 2475 to 2501 in 751037d

  	private IMemoryOwner<byte> ReadChunkData(int length)
  	{
  	if (length == 0)
  	{
  	return new BasicArrayBuffer<byte>([]);
  	}
  	// We rent the buffer here to return it afterwards in Decode()
  	// We don't want to throw a degenerated memory exception here as we want to allow partial decoding
  	// so limit the length.
  	length = (int)Math.Min(length, this.currentStream.Length - this.currentStream.Position);
  	IMemoryOwner<byte> buffer = this.configuration.MemoryAllocator.Allocate<byte>(length, AllocationOptions.Clean);
  	try
  	{
  	this.currentStream.Read(buffer.GetSpan(), 0, length);
  	}
  	catch
  	{
  	// The read can throw before the caller takes ownership of the buffer (e.g. the stream
  	// observes a cancellation request), so dispose it here to avoid leaking the rented memory.
  	buffer.Dispose();
  	throw;
  	}
  	return buffer;
  	}

• ValidateChunk reads the CRC first (L2422) and disposes only on a mismatch
  (L2437); ReadChunkCrc is the read (L2451):
  

  ImageSharp/src/ImageSharp/Formats/Png/PngDecoderCore.cs

  Lines 2420 to 2457 in 751037d

  	private void ValidateChunk(in PngChunk chunk, Span<byte> buffer)
  	{
  	uint inputCrc = this.ReadChunkCrc(buffer);
  	if (chunk.IsCritical(this.segmentIntegrityHandling))
  	{
  	Span<byte> chunkType = stackalloc byte[4];
  	BinaryPrimitives.WriteUInt32BigEndian(chunkType, (uint)chunk.Type);
  	this.crc32.Reset();
  	this.crc32.Append(chunkType);
  	this.crc32.Append(chunk.Data.GetSpan());
  	if (this.crc32.GetCurrentHashAsUInt32() != inputCrc)
  	{
  	string chunkTypeName = Encoding.ASCII.GetString(chunkType);
  	// ensure when throwing we dispose the data back to the memory allocator
  	chunk.Data?.Dispose();
  	PngThrowHelper.ThrowInvalidChunkCrc(chunkTypeName);
  	}
  	}
  	}
  	/// <summary>
  	/// Reads the cycle redundancy chunk from the data.
  	/// </summary>
  	/// <param name="buffer">Temporary buffer.</param>
  	[MethodImpl(InliningOptions.ShortMethod)]
  	private uint ReadChunkCrc(Span<byte> buffer)
  	{
  	uint crc = 0;
  	if (this.currentStream.Read(buffer, 0, 4) == 4)
  	{
  	crc = BinaryPrimitives.ReadUInt32BigEndian(buffer);
  	}
  	return crc;
  	}

• The Decode loop and its dispose, L191 and L325; the Identify loop and its
  dispose, L366 and L537 (same file).
• BufferedReadStream.Read(Span<byte>) checks the token first (L178), and so
  does the Position setter (L98):
  

  ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

  Lines 95 to 98 in 751037d

  	set
  	{
  	Guard.MustBeGreaterThanOrEqualTo(value, 0, nameof(this.Position));
  	this.cancellationToken.ThrowIfCancellationRequested();

  
  

  ImageSharp/src/ImageSharp/IO/BufferedReadStream.cs

  Lines 176 to 178 in 751037d

  	public override int Read(Span<byte> buffer)
  	{
  	this.cancellationToken.ThrowIfCancellationRequested();

From the source, not exercised by the repro: the first window (the CRC read)
does not depend on the chunk type, and TryReadChunk has other callers
(L2192, L2225, L2250). The window is inside TryReadChunk itself, so it should
not depend on the caller. The repro cancels at the first IDAT chunk only.

How this was noticed: while verifying the 4.1.3 fix. An application-level
harness that cancels PNG processing at random points still ended with the
count one above its baseline in a small share of runs on 4.1.3. That harness is
not part of this report, and those runs are mentioned only as the reason for
looking. The repro below makes the window deterministic. It shows the
mechanism; it does not show that every such run took one of these two paths.

A guard in TryReadChunk covering the statements between ReadChunkData and
the return would close both windows. Note that ValidateChunk already disposes
the buffer on a CRC mismatch, so such a guard has to allow for that. I have not
tested a patch.

Expected: the buffer is released on every path, including cancellation, so
the undisposed count returns to its baseline.

Steps to Reproduce

The inline repro below is a console app that references only ImageSharp 4.1.3
and generates its own 256×256 PNG. It runs with the default configuration, with
no timer and no repetition. Only public ImageSharp APIs are used. Reflection is
limited to diagnostic version and stack information; it is not used to access
ImageSharp internals. Save the two files below into a repro directory.
Building requires the .NET 10 SDK and the normal ImageSharp 4 license
configuration; no license key is included.

cd repro
dotnet run -c Release

To compare with the previous release:

dotnet run -c Release -p:ImageSharpVersion=4.1.2

How the timing is made deterministic: the input is a MemoryStream subclass,
which the decoder reads in place. It cancels the token once, from inside one
chosen call, and lets that call complete. The decoder then observes the
cancellation at its next token check. The first IDAT chunk (65,535 bytes) is
larger than the decoder's 8,096-byte stream buffer, so its data is read in a
single call to the input stream and the stream buffer is refilled for the CRC;
the repro checks that precondition and refuses to run without it.

Result, one process per ImageSharp version. Each case runs once under
Image.IdentifyAsync and once under Image.LoadAsync<Rgba32>; both gave the
same count in every case.

The "observes it at" column is the throw site the repro records from the
first-chance exception; the full lines are in the recorded output.

Images

None needed: the repro generates its input.

Complete reproduction

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net10.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <!-- To compare with the previous release: -p:ImageSharpVersion=4.1.2 -->
    <ImageSharpVersion Condition="'$(ImageSharpVersion)' == ''">4.1.3</ImageSharpVersion>
  </PropertyGroup>

  <ItemGroup>
    <PackageReference Include="SixLabors.ImageSharp" Version="$(ImageSharpVersion)" />
  </ItemGroup>

</Project>

// Reproduction for SixLabors.ImageSharp 4.1.3: a PNG chunk buffer is still left
// undisposed when a cancellation is first observed just AFTER the chunk-data
// read that #3203 / #3209 guarded.
//
// No timer and no repetition. The input stream cancels the token itself, at one
// chosen call, and the decoder observes it at its next check:
//
//   A  during the read of the first IDAT chunk's data
//        -> observed inside that read                     (the path #3203 fixed)
//   B  in the seek that follows that read
//        -> observed at the entry of the CRC read         (ValidateChunk)
//   C  during the read that fetches that chunk's CRC
//        -> observed when TryReadChunk moves the stream back to the IDAT data
//
// Each case runs once under Image.IdentifyAsync and once under Image.LoadAsync,
// with the default configuration.
//
// Only public ImageSharp APIs are used. Reflection is limited to diagnostic
// version and stack information; it is not used to access ImageSharp internals.
//
//   dotnet run -c Release

using System.Buffers.Binary;
using System.Diagnostics;
using System.Reflection;
using System.Runtime.InteropServices;
using SixLabors.ImageSharp;
using SixLabors.ImageSharp.Diagnostics;
using SixLabors.ImageSharp.PixelFormats;

Console.WriteLine($"imagesharp={typeof(Image).Assembly.GetCustomAttribute<AssemblyInformationalVersionAttribute>()?.InformationalVersion}");
Console.WriteLine($"runtime={RuntimeInformation.FrameworkDescription}");
Console.WriteLine($"os={RuntimeInformation.OSDescription} {RuntimeInformation.ProcessArchitecture}");
#if DEBUG
Console.WriteLine("build=Debug");
#else
Console.WriteLine("build=Release");
#endif

byte[] png = CreatePng(256, 256);
(int dataOffset, int dataLength) = FirstIdat(png);
int streamBuffer = Configuration.Default.StreamProcessingBufferSize;
Console.WriteLine($"input=generated PNG 256x256, {png.Length} bytes; first IDAT data at offset {dataOffset}, {dataLength} bytes; decoder stream buffer {streamBuffer} bytes");

if (dataLength <= streamBuffer)
{
    // The decoder reads a chunk in one call to the input stream only when the
    // chunk is larger than its own buffer; the cases below rely on that.
    Console.WriteLine("result=cannot run: the first IDAT chunk is not larger than the decoder's stream buffer");
    return 2;
}

// Where the cancellation is thrown. An awaited exception no longer carries it.
string? observedAt = null;
AppDomain.CurrentDomain.FirstChanceException += (_, e) =>
{
    if (e.Exception is OperationCanceledException && observedAt is null)
    {
        observedAt = string.Join(
            " < ",
            new StackTrace(fNeedFileInfo: false).GetFrames()
                .Select(frame => frame.GetMethod())
                .Where(method => method?.DeclaringType?.FullName?.StartsWith("SixLabors.ImageSharp", StringComparison.Ordinal) == true)
                .Take(6)
                .Select(method => $"{method!.DeclaringType!.Name}.{method.Name}"));
    }
};

int keptAfterTheGuardedRead = 0;

foreach (CancelAt cancelAt in Enum.GetValues<CancelAt>())
{
    foreach (bool decode in new[] { false, true })
    {
        using CancellationTokenSource cancellation = new();
        using CancellingStream stream = new(png, dataOffset, dataLength, cancelAt, cancellation);
        observedAt = null;

        int before = MemoryDiagnostics.TotalUndisposedAllocationCount;
        string outcome;

        try
        {
            if (decode)
            {
                using Image<Rgba32> image = await Image.LoadAsync<Rgba32>(stream, cancellation.Token);
            }
            else
            {
                await Image.IdentifyAsync(stream, cancellation.Token);
            }

            outcome = "completed";
        }
        catch (OperationCanceledException e)
        {
            outcome = e.GetType().Name;
        }

        int kept = MemoryDiagnostics.TotalUndisposedAllocationCount - before;

        if (cancelAt != CancelAt.DataRead)
        {
            keptAfterTheGuardedRead += kept;
        }

        Console.WriteLine();
        Console.WriteLine($"case={Label(cancelAt)} api={(decode ? "Image.LoadAsync<Rgba32>" : "Image.IdentifyAsync")}");
        Console.WriteLine($"  cancelled in     : {stream.CancelledIn ?? "NOT CANCELLED - this case shows nothing"}");
        Console.WriteLine($"  outcome          : {outcome}");
        Console.WriteLine($"  observed at      : {observedAt ?? "none"}");
        Console.WriteLine($"  undisposed count : {kept:+0;-0;+0}");
    }
}

Console.WriteLine();
Console.WriteLine(keptAfterTheGuardedRead > 0
    ? $"result=REPRODUCED: {keptAfterTheGuardedRead} buffer(s) left undisposed in cases B and C"
    : "result=not reproduced: cases B and C returned every buffer");

return keptAfterTheGuardedRead > 0 ? 1 : 0;

static string Label(CancelAt cancelAt) => cancelAt switch
{
    CancelAt.DataRead => "A (during the chunk-data read)",
    CancelAt.SeekAfterDataRead => "B (in the seek after the chunk-data read)",
    _ => "C (during the CRC read)",
};

// Noise, so the image data does not fit in one small IDAT chunk.
static byte[] CreatePng(int width, int height)
{
    Random random = new(7);
    using Image<Rgba32> image = new(width, height);
    image.ProcessPixelRows(accessor =>
    {
        for (int y = 0; y < accessor.Height; y++)
        {
            foreach (ref Rgba32 pixel in accessor.GetRowSpan(y))
            {
                pixel = new Rgba32((byte)random.Next(256), (byte)random.Next(256), (byte)random.Next(256), 255);
            }
        }
    });

    using MemoryStream output = new();
    image.SaveAsPng(output);
    return output.ToArray();
}

// Where the first IDAT chunk's data starts, and how long it is.
static (int Offset, int Length) FirstIdat(byte[] png)
{
    for (int at = 8; at + 8 <= png.Length;)
    {
        int length = (int)BinaryPrimitives.ReadUInt32BigEndian(png.AsSpan(at));

        if (png.AsSpan(at + 4, 4).SequenceEqual("IDAT"u8))
        {
            return (at + 8, length);
        }

        at += 4 + 4 + length + 4; // length, type, data, CRC
    }

    throw new InvalidOperationException("The PNG holds no IDAT chunk.");
}

enum CancelAt
{
    DataRead,
    SeekAfterDataRead,
    CrcRead,
}

// A MemoryStream, because that is what the decoder reads in place. It cancels
// the token once, inside the chosen call, and then lets the call complete.
sealed class CancellingStream(
    byte[] content,
    int dataOffset,
    int dataLength,
    CancelAt cancelAt,
    CancellationTokenSource cancellation)
    : MemoryStream(content, writable: false)
{
    private bool dataReadSeen;

    public string? CancelledIn { get; private set; }

    // Every Read overload of a type derived from MemoryStream arrives here.
    public override int Read(byte[] buffer, int offset, int count)
    {
        long position = this.Position;
        bool isDataRead = position == dataOffset && count == dataLength;

        if (isDataRead && cancelAt == CancelAt.DataRead)
        {
            this.Cancel($"Read at {position}, {count} bytes (the chunk's data)");
        }
        else if (this.dataReadSeen && position == dataOffset + dataLength && cancelAt == CancelAt.CrcRead)
        {
            this.Cancel($"Read at {position}, {count} bytes (starts with the chunk's CRC)");
        }

        this.dataReadSeen |= isDataRead;
        return base.Read(buffer, offset, count);
    }

    public override long Seek(long offset, SeekOrigin loc)
    {
        if (this.dataReadSeen && cancelAt == CancelAt.SeekAfterDataRead
            && loc == SeekOrigin.Begin && offset == dataOffset + dataLength)
        {
            this.Cancel($"Seek to {offset} (the end of the chunk's data)");
        }

        return base.Seek(offset, loc);
    }

    private void Cancel(string where)
    {
        if (!cancellation.IsCancellationRequested)
        {
            this.CancelledIn = where;
            cancellation.Cancel();
        }
    }
}

Recorded output

Windows, one fresh process per run. The Debug run on 4.1.3 differs from the
Release run only in its build= line.

imagesharp=4.1.3+751037d0c6384c97e77ee1abd86e2203ae8dbd95
runtime=.NET 10.0.12
os=Microsoft Windows 10.0.26300 X64
build=Release
input=generated PNG 256x256, 225226 bytes; first IDAT data at offset 62, 65535 bytes; decoder stream buffer 8096 bytes

case=A (during the chunk-data read) api=Image.IdentifyAsync
  cancelled in     : Read at 62, 65535 bytes (the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < BufferedReadStream.ReadToBufferDirectSlow < BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkData < PngDecoderCore.TryReadChunk
  undisposed count : +0

case=A (during the chunk-data read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Read at 62, 65535 bytes (the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < BufferedReadStream.ReadToBufferDirectSlow < BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkData < PngDecoderCore.TryReadChunk
  undisposed count : +0

case=B (in the seek after the chunk-data read) api=Image.IdentifyAsync
  cancelled in     : Seek to 65597 (the end of the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkCrc < PngDecoderCore.ValidateChunk < PngDecoderCore.TryReadChunk < PngDecoderCore.Identify
  undisposed count : +1

case=B (in the seek after the chunk-data read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Seek to 65597 (the end of the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkCrc < PngDecoderCore.ValidateChunk < PngDecoderCore.TryReadChunk < PngDecoderCore.Decode
  undisposed count : +1

case=C (during the CRC read) api=Image.IdentifyAsync
  cancelled in     : Read at 65597, 8096 bytes (starts with the chunk's CRC)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < PngDecoderCore.TryReadChunk < PngDecoderCore.Identify < ImageDecoderCore.Identify < PngDecoder.Identify < <>c__DisplayClass5_0.<IdentifyAsync>b__0
  undisposed count : +1

case=C (during the CRC read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Read at 65597, 8096 bytes (starts with the chunk's CRC)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < PngDecoderCore.TryReadChunk < PngDecoderCore.Decode < ImageDecoderCore.Decode < PngDecoder.Decode < SpecializedImageDecoder`1.Decode
  undisposed count : +1

result=REPRODUCED: 4 buffer(s) left undisposed in cases B and C

imagesharp=4.1.2+dee414097e4be7bb35ef0209cf96bd0b61a7133a
runtime=.NET 10.0.12
os=Microsoft Windows 10.0.26300 X64
build=Release
input=generated PNG 256x256, 225226 bytes; first IDAT data at offset 62, 65535 bytes; decoder stream buffer 8096 bytes

case=A (during the chunk-data read) api=Image.IdentifyAsync
  cancelled in     : Read at 62, 65535 bytes (the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < BufferedReadStream.ReadToBufferDirectSlow < BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkData < PngDecoderCore.TryReadChunk
  undisposed count : +1

case=A (during the chunk-data read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Read at 62, 65535 bytes (the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < BufferedReadStream.ReadToBufferDirectSlow < BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkData < PngDecoderCore.TryReadChunk
  undisposed count : +1

case=B (in the seek after the chunk-data read) api=Image.IdentifyAsync
  cancelled in     : Seek to 65597 (the end of the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkCrc < PngDecoderCore.ValidateChunk < PngDecoderCore.TryReadChunk < PngDecoderCore.Identify
  undisposed count : +1

case=B (in the seek after the chunk-data read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Seek to 65597 (the end of the chunk's data)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.Read < BufferedReadStreamExtensions.Read < PngDecoderCore.ReadChunkCrc < PngDecoderCore.ValidateChunk < PngDecoderCore.TryReadChunk < PngDecoderCore.Decode
  undisposed count : +1

case=C (during the CRC read) api=Image.IdentifyAsync
  cancelled in     : Read at 65597, 8096 bytes (starts with the chunk's CRC)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < PngDecoderCore.TryReadChunk < PngDecoderCore.Identify < ImageDecoderCore.Identify < PngDecoder.Identify < <>c__DisplayClass5_0.<IdentifyAsync>b__0
  undisposed count : +1

case=C (during the CRC read) api=Image.LoadAsync<Rgba32>
  cancelled in     : Read at 65597, 8096 bytes (starts with the chunk's CRC)
  outcome          : TaskCanceledException
  observed at      : BufferedReadStream.set_Position < PngDecoderCore.TryReadChunk < PngDecoderCore.Decode < ImageDecoderCore.Decode < PngDecoder.Decode < SpecializedImageDecoder`1.Decode
  undisposed count : +1

result=REPRODUCED: 4 buffer(s) left undisposed in cases B and C