Calling Pipe.Writer.WriteAsync after Pipe.Writer.Advance flush truncated data to the PipeReader.

[Kuinox]

Description

(I edited this issue when I noticed the bug did not affected only ReadAtLeastAsync, original title was 'Pipe.ReadAtLeastAsync return with less bytes than asked.')

Calling Pipe.Writer.WriteAsync after Pipe.Writer.Advance flush truncated data to the PipeReader.
I initially found this bug because ReadAtLeastAsync returned me a 12 bytes buffer, when I asked for 26.

Reproduction Steps

Edit:

[Test]
public async Task ReadAtLeastAsync_works()
{
    Pipe pipe = new Pipe(new PipeOptions(minimumSegmentSize: 1));
    Task<ReadResult> resTask = pipe.Reader.ReadAtLeastAsync( 26 ).AsTask();

    pipe.Writer.GetSpan( 14 );
    pipe.Writer.Advance( 14 );
    await pipe.Writer.WriteAsync( new byte[12] );
    ReadResult res = await resTask;
    res.Buffer.Length.Should().Be( 26 ); //fails
}

Expected behavior

Data written is not overwritten.

Actual behavior

Only 12 bytes are returned. The first write is partially overwritten.

Regression?

The reproduction does not show the bug on version 4.5.4.

Known Workarounds

Call FlushAsync before calling WriteAsync if you did Advance() before.

Configuration

N/A.

[dotnet-issue-labeler]

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

[halter73]

It should not be possible for PipeReader.ReadAtLeastAsyncCore(...) to return at all unless result.Buffer.Length >= minimumSize || result.IsCompleted || result.IsCanceled.

runtime/src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/PipeReader.cs

Lines 61 to 64 in 7a62468

	if (buffer.Length >= minimumSize || result.IsCompleted || result.IsCanceled)
	{
	return result;
	}

Is it possible that you're using a PipeReader with a custom ReadAtLeastAsyncCore(...) implementation? Or could buffer.Length have been a smaller value than 26 when ReadAtLeastAsync(buffer.Length, cts.Token) was initially called?

[Kuinox]

Is it possible that you're using a PipeReader with a custom ReadAtLeastAsyncCore(...) implementation?

I'm using the duplex "Pipe" implementation.
I just managed to make a minimal reproduction:

[Test]
public async Task ReadAtLeastAsync_works()
{
    Pipe pipe = new Pipe(new PipeOptions(minimumSegmentSize: 1));
    Task<ReadResult> resTask = pipe.Reader.ReadAtLeastAsync( 26 ).AsTask();

    pipe.Writer.GetSpan( 14 );
    pipe.Writer.Advance( 14 );
    await pipe.Writer.WriteAsync( new byte[12] );
    ReadResult res = await resTask;
    res.Buffer.Length.Should().Be( 26 ); //fails
}

Can I also do the PR to fixes this bug, if possible ?

[Kuinox]

Looks like the bug is worst than I thought;
I tried to understand what happened to the missing data.

 [Test]
  public async Task ReadAtLeastAsync_works()
  {
      Memory<byte> buffer = new byte[] { 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13 };
      Pipe pipe = new( new PipeOptions( minimumSegmentSize: 1 ) );
      Task<ReadResult> resTask = pipe.Reader.ReadAtLeastAsync( 26 ).AsTask();

      var mem = pipe.Writer.GetMemory( 14 )[..14];
      buffer.CopyTo( mem );
      pipe.Writer.Advance( 14 );
      await pipe.Writer.WriteAsync( new byte[12] { 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25 } );
      ReadResult res = await resTask;
      Console.WriteLine( string.Join( ", ", res.Buffer.ToArray() ) );
      res.Buffer.Length.Should().Be( 26 );
  }

Output: 0, 1, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25

It looks like it's a regression from 4.5.4(pass test) from to 5.0.0(fail test).

I'm feeling that I should not WriteAsync after Advance without calling FlushAsync, but nothing indicate in the docs that you shouldn't do that.

This bug (if this is indeed a bug and not missing docs) is not exclusive to ReadAtLeastAsync and and also drop bytes if you try to use ReadAsync.

[Kuinox]

I went ahead and made a PR because the fix is one line.