Always read from underlying stream in StreamPipeReader

[BrennanConroy]

As part of dotnet/aspnetcore#62895 we realized that StreamPipeReader was internally storing the fact that the underlying Stream returned 0 indicating end of stream. This causes problems with code that might modify the Stream.Position after the Stream was completely read from. Meaning future read calls on StreamPipeReader will always complete without re-reading the Stream.

This behavior doesn't seem necessary so relaxing the restrictions to always read from the underlying Stream, assuming the PipeReader isn't completed and there isn't any unexamined buffered data.

[Copilot]

Pull Request Overview

This PR fixes an issue in StreamPipeReader where the reader would incorrectly cache that the underlying stream had reached its end, preventing it from re-reading data when the stream position was reset. The change removes internal state tracking of stream completion and always attempts to read from the underlying stream when there's no unexamined buffered data.

• Removes the _isStreamCompleted field that was preventing subsequent reads after the stream reached EOF
• Updates read logic to determine completion status dynamically rather than caching it
• Modifies test cases to use standard MemoryStream instead of custom test streams and adds a new test for stream position reset scenarios

Reviewed Changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
src/libraries/System.IO.Pipelines/src/System/IO/Pipelines/StreamPipeReader.cs	Removes _isStreamCompleted field and associated logic that cached stream completion state
src/libraries/System.IO.Pipelines/tests/StreamPipeReaderTests.cs	Updates existing tests and adds new test case for stream position reset behavior

[NinoFloris]

This would mean wrapping a networkstream would now throw after the EOF is read? (As it'll transition to non-readable, causing an exception on the next ReadAsync). EDIT: nevermind, this won't happen until Close is called.

[BrennanConroy]

Right, most Streams should continue working until Close/Dispose is called on them.

[davidfowl]

I’m remembering this was added for a reason ….

[BrennanConroy]

I’m remembering this was added for a reason ….

Was it a good reason? 😆

[BrennanConroy]

Would checking Stream.CanSeek to determine if we should store completion state be more desirable?