Make LimitArrayPoolWriteStream.ReturnAllPooledBuffers idempotent

[EgorBo]

Makes LimitArrayPoolWriteStream.ReturnAllPooledBuffers idempotent under concurrent Dispose() so that user code cannot accidentally double-return the same buffer to ArrayPool<byte>.Shared. Two Interlocked.Exchange calls on the existing fields are enough — no new allocations or fast-path overhead in the single-threaded case.

It's not a security issue, just makes code more robust to buggy implementations

using var content = new RacingContent();
await content.LoadIntoBufferAsync(int.MaxValue);

internal sealed class RacingContent : HttpContent
{
    private static readonly byte[] s_payload = new byte[16 * 1024];

    protected override bool TryComputeLength(out long length) { length = 0; return false; }

    protected override async Task SerializeToStreamAsync(Stream stream, TransportContext? context)
    {
        await stream.WriteAsync(s_payload);   // populates _pooledBuffers[0]
        await stream.WriteAsync(s_payload);   // _lastBuffer is now pooled

        // if stream.Dispose is called in parallel here before we exit - it will lead to double-free in ArrayPool
    }
}

Note

This PR was authored with the assistance of GitHub Copilot.

[dotnet-policy-service]

Tagging subscribers to this area: @karelz, @dotnet/ncl
See info in area-owners.md if you want to be subscribed.

[Copilot]

Pull request overview

This PR makes HttpContent.LimitArrayPoolWriteStream.ReturnAllPooledBuffers() safe to call multiple times concurrently by making buffer-return operations idempotent, preventing accidental double-return of the same byte[] to ArrayPool<byte>.Shared (which can corrupt the shared pool).

Changes:

• Atomically claims and clears _pooledBuffers via Interlocked.Exchange(ref _pooledBuffers, null) so only one caller returns the pooled buffer list.
• Atomically claims and clears _lastBufferIsPooled via Interlocked.Exchange(ref _lastBufferIsPooled, false) so only one caller returns the final pooled buffer.
• Updates internal comments to document the rationale and the concurrency/idempotency intent.

[MihaZupan]

We could consider having Dispose unconditionally throw on this type, and have all internal logic call ReturnAllPooledBuffers directly instead.
There's no reason for user code to call dispose on this type, they never have ownership of the lifetime, and calling dispose would just break other logic.

In your example, if you call Dispose, you can see that other functionality on the HttpContent will be broken, e.g.

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
   at System.Net.Http.HttpContent.<>c.<ReadAsByteArrayAsync>b__31_0(HttpContent s)
   at System.Net.Http.HttpContent.WaitAndReturnAsync[TState,TResult](Task waitTask, TState state, Func`2 returnFunc)

System.ArgumentOutOfRangeException: Specified argument was out of the range of valid values.
   at System.Net.Http.HttpContent.LimitArrayPoolWriteStream.GetFirstBuffer()
   at System.Net.Http.HttpContent.ReadBufferAsString(LimitArrayPoolWriteStream stream, HttpContentHeaders headers)
   at System.Net.Http.HttpContent.WaitAndReturnAsync[TState,TResult](Task waitTask, TState state, Func`2 returnFunc)

System.ArgumentNullException: Value cannot be null. (Parameter 'buffer')
   at System.ArgumentNullException.Throw(String paramName)
   at System.IO.MemoryStream..ctor(Byte[] buffer, Int32 index, Int32 count, Boolean writable, Boolean publiclyVisible)
   at System.IO.MemoryStream..ctor(Byte[] buffer, Int32 index, Int32 count, Boolean writable)
   at System.Net.Http.HttpContent.CreateMemoryStreamFromBufferedContent()
   at System.Net.Http.HttpContent.ReadAsStreamAsync(CancellationToken cancellationToken)

[EgorBo]

I'm not familiar with this logic so I'm leaving it to you. The general idea is that even if users do something stupid (as long as it's not unsafe code) we shouldn't corrupt the global state which ArrayPool is

[MihaZupan]

#128328