Background

This PR introduces a new stream ApmToTapStream, which can ensure that execution doesn’t unintentionally go down sync code paths when using some streams in .NET Framework 4.8.

Some streams in net48 do not override the modern TAP async methods (e.g. ReadAsync, WriteAsync) and thus fallback to the default implementation in the Stream base class when they are called. This default implementation uses the APM async methods (e.g. BeginRead, EndRead), which in turn calls the sync methods like Read and Write. This has the unintended consequence of causing a call to ReadAsync on an outer stream eventually calling Read on an inner stream, therefore making the execution path go from async to sync.

DeflateStream is one of these streams, and the usage of this type in Halibut’s MessageSerializer class means that many of our async calls in net48 were not truly async. By wrapping DeflateStream's inner stream in a ApmToTapStream, this problem can be mitigated.

Results

Fixes [sc-57049]

Before

async Task<T> ReadCompressedMessageAsync<T>(Stream stream, IRewindableBuffer rewindableBuffer, CancellationToken cancellationToken)
{
    // Do some stuff

    try
    {
        using var compressedByteCountingStream = new ByteCountingStream(stream, OnDispose.LeaveInputStreamOpen);
        using var zip = new DeflateStream(compressedByteCountingStream, CompressionMode.Decompress, true);
        using var decompressedByteCountingStream = new ByteCountingStream(zip, OnDispose.LeaveInputStreamOpen);
        using var deflatedInMemoryStream = new ReadIntoMemoryBufferStream(decompressedByteCountingStream, readIntoMemoryLimitBytes, OnDispose.LeaveInputStreamOpen);

        // Read the stream(s) here
        await deflatedInMemoryStream.BufferIntoMemoryFromSourceStreamUntilLimitReached(cancellationToken);

        // Do some more stuff
    }
}

ApmToTapStream Before

20230818-110137_rider64_wFR4nt2vWs

After

async Task<T> ReadCompressedMessageAsync<T>(Stream stream, IRewindableBuffer rewindableBuffer, CancellationToken cancellationToken)
{
    // Do some stuff

    try
    {
        using var compressedByteCountingStream = new ByteCountingStream(stream, OnDispose.LeaveInputStreamOpen);
        using var apmToTapStream = new ApmToTapStream(compressedByteCountingStream);
        using var zip = new DeflateStream(apmToTapStream, CompressionMode.Decompress, true);
        using var decompressedByteCountingStream = new ByteCountingStream(zip, OnDispose.LeaveInputStreamOpen);
        using var deflatedInMemoryStream = new ReadIntoMemoryBufferStream(decompressedByteCountingStream, readIntoMemoryLimitBytes, OnDispose.LeaveInputStreamOpen);

        // Read the stream(s) here
        await deflatedInMemoryStream.BufferIntoMemoryFromSourceStreamUntilLimitReached(cancellationToken);

        // Do some more stuff
    }
}

ApmToTapStream After

20230818-115809_rider64_B6WZHuiIEz

How to review this

Quality :heavy_check_mark:

Pre-requisites

[ ] I have read How we use GitHub Issues for help deciding when and where it's appropriate to make an issue.
[ ] I have considered informing or consulting the right people, according to the ownership map.
[ ] I have considered appropriate testing for my change.

OctopusDeploy / Halibut