[benaadams]

May cause heap corruption re: #332 (comment)

Currently it Unpins, then a block is returned which throws an ODE exception aspnet/KestrelHttpServer#2486

Log Critical[0]: Unexpected exception in HttpConnection.ProcessRequestsAsync. System.ObjectDisposedException: Cannot access a disposed object.
[20:27:03]	Object name: 'MemoryPoolBlock'.
[20:27:03]	at System.Buffers.ThrowHelper.ThrowObjectDisposedException(ExceptionArgument argument) in D:\b\w\1e8cd6334ef22651\packages\microsoft.extensions.buffers.sources\2.1.0-preview3-32110\contentFiles\cs\netstandard1.0\Microsoft.Extensions.Buffers.Sources\PipelinesThrowHelper.cs:line 51
[20:27:03]	at System.Buffers.MemoryPoolBlock.Dispose() in D:\b\w\1e8cd6334ef22651\packages\microsoft.extensions.buffers.sources\2.1.0-preview3-32110\contentFiles\cs\netstandard1.0\Microsoft.Extensions.Buffers.Sources\MemoryPoolBlock.Release.cs:line 54
[20:27:03]	at System.IO.Pipelines.BufferSegment.ResetMemory()
[20:27:03]	at System.IO.Pipelines.Pipe.CompletePipe()
[20:27:03]	at System.IO.Pipelines.Pipe.CompleteWriter(Exception exception)
[20:27:03]	at Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal.AdaptedPipeline.ReadInputAsync(Stream stream)
[20:27:03]	at Microsoft.AspNetCore.Server.Kestrel.Core.Adapter.Internal.AdaptedPipeline.RunAsync(Stream stream) in D:\b\w\1e8cd6334ef22651\src\Kestrel.Core\Adapter\Internal\AdaptedPipeline.cs:line 45
[20:27:03]	at Microsoft.AspNetCore.Server.Kestrel.Core.Internal.HttpConnection.ProcessRequestsAsync[TContext](IHttpApplication`1 httpApplication) in D:\b\w\1e8cd6334ef22651\src\Kestrel.Core\Internal\HttpConnection.cs:line 183

Probably bad things also happen as the data block would now be using unpinned memory, with a long previously captured pointer... Like heap corruption? aspnet/KestrelHttpServer#2503 https://github.com/dotnet/coreclr/issues/17679

New spin on this; is the change #332 started throwing ODE exceptions, whereas previously it didn't do, so that may have been where the GC crash leading to hangs surfaced in the error handling/unwind?

Resolves aspnet/KestrelHttpServer#2486 (Test timeout: MemoryPoolBlock ObjectDisposedException)

/cc @pakrym @davidfowl @GrabYourPitchforks @mikeharder @jkotas

[benaadams]

Think it mostly arises in Kestrel tests; as a shutdown race

[pakrym]

I feel like we need to fix kestrel so it doesn't dispose while in use.

[pakrym]

Move this logic into Slab.Return.

[pakrym]

I'm concerned about leaking pinned arrays. Are you sure we would always end up unpinning them?

[benaadams]

Not if the block was never returned

[pakrym]

This case is double return. To check for actual dispose use _disposedValue. Can we throw only when the block we are turning is being double returned?

[pakrym]

We do with assert. Should we change assert to exception? What's easier to debug?

[pakrym]

Add blocks Leasers to exception message.

[pakrym]

Tests for things that are reliable testable.

[benaadams]

@pakrym I'm happy for you to take this on 😉

[mikeharder]

For the record, aspnet/KestrelHttpServer#2503 was caused by a bug in the GC (https://github.com/dotnet/coreclr/issues/17716) rather than managed heap corruption.

[davidfowl]

Closing this as there are tons of conflicts and we aren't particularly convinced this is required.