PipeReader.ReadAsync can return empty segments

[Trolldemorted]

I just noticed that segments in a ReadResult can be empty:

I assumed that the first segment was never empty, so I often use result.Buffer.First.Span[0]; to access the first byte.

Is this intended behaviour? Can this be caused by me writing to the pipe in a wrong way? This is how I fill my pipe from a System.IO.Compression.DeflateStream:

private async Task Decompress(PipeWriter writer, ReadOnlyMemory<byte> data, int count)
{
    InflateInputStream.Position = 0;
    InflateInputStream.Write(data.Span.Slice(0, count));
    InflateInputStream.Position = 0;
    InflateInputStream.SetLength(count);
    int read;

    do
    {
        Memory<byte> memory = writer.GetMemory(data.Length * 2);
        read = InflateOutputStream.Read(memory.Span);
        writer.Advance(read);
    }
    while (read != 0);
    await writer.FlushAsync();
}

[jkotalik]

@davidfowl @pakrym IIRC I believe this behavior can happen based on how the pipe advances internally. I remember you two had a conversation about it. Is it possible to check if the first segment is empty before returning the ReadResult?

[Trolldemorted]

Just FYI, this behaviour does not occur in my setup if I read the inflate stream's output into a dedicated buffer, and WriteAsync the buffer into the pipe.

[davidfowl]

Ugh yes we need to try to solve the empty segment problem. It’s a real PITA to deal with that today. Honestly I’d rather try to fix it with ReadOnlySequence itself but we’d need to measure the impact.

[jkotalik]

but we’d need to measure the impact.

What impact would it have?

[pakrym]

ROS is very performance sensitive and we would have to add empty segment skipping code to multiple very hot paths (construction, First, enumeration etc)

[davidfowl]

I think we should at the very least try to avoid empty segments while building up the structure internally.

[pakrym]

Wonder if we should make it a rule, don't produce ROS<> with empty segments. We won't be able to assert it unfortunately.

[jkotalik]

I think that rule works except when the ROS<> is empty.

@Trolldemorted From what I can tell, this potentially was fixed in 3.0. Could you try the latest 3.0-preview9 release and see if you hit this behavior again? https://dotnet.microsoft.com/download/dotnet-core/3.0.

edit: We believe dotnet/corefx@34765c5 should have fixed this issue. Please let us know if you are continuing to see this behavior.

[davidfowl]

@Trolldemorted can you show how you created the Pipe? In particular I want to see the PipeOptions

[Trolldemorted]

@jkotalik just making sure, you want me to upgrade to sdk preview9, but keep System.IO.Pipelines on the latest stable release (4.5.3)? Was 34765c5 landed in the sdk? I upgraded the SDK to 3.0.100-preview9-014004 and kept System.IO.Pipelines 4.5.3 but it happened again. I also noticed other segments can be empty too:

@davidfowl I am using new Pipe() with no explicit options (source).

[davidfowl]

@Trolldemorted you need to update the package to preview9 of System.IO.Pipelines

[Trolldemorted]

I upgraded to 4.6.0-preview9.19421.4, but it still happens. Might be my imagination but it happens less frequently, though.

[davidfowl]

@Trolldemorted if you can provide a repro (set of steps to reproduce the empty segments) that would be great. It would mean logging what you wrote and how you called AdvancedTo on the other side.

[Trolldemorted]

A repro for an empty first segment is tricky, as they happen pretty rarely (last time after >2GB of input). However, I found a small input for an empty non-first segment:

https://github.com/Trolldemorted/EmptySegmentsBug/blob/371c7ceca7412f4313257c720965162a1b56ae76/EmptySegmentsBug/Program.cs

If you have VS and dotnet core 3 preview9 installed, opening it and pressing F5 should throw the "segment empty!" InvalidDataException, at least it does that reliably on my laptop.

Thanks a lot for your patience!

[marklio]

Marking this as reliability, as First.Span[0] and FirstSpan[0] are extremely common when processing ROS.