Fix StreamPipeReader.CopyToAsync not advancing past buffered data#124989
Open
BrennanConroy wants to merge 2 commits intodotnet:mainfrom
Open
Fix StreamPipeReader.CopyToAsync not advancing past buffered data#124989BrennanConroy wants to merge 2 commits intodotnet:mainfrom
BrennanConroy wants to merge 2 commits intodotnet:mainfrom
Conversation
Both CopyToAsync overloads (PipeWriter and Stream destinations) iterate buffered segments, but on successful drain the loop exits with segment set to null. The finally block only called AdvanceTo when segment was non-null, so buffered data remained in _readHead/_readIndex and would be returned again on subsequent reads. Add an else-if branch to the finally block that advances past all buffered data (_readTail) when the segment loop completes normally. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a StreamPipeReader.CopyToAsync buffering bug where successfully draining all buffered segments could leave _readHead/_readIndex unchanged, causing the same buffered data to be returned again on subsequent reads.
Changes:
- Update
StreamPipeReader.CopyToAsync(bothPipeWriterandStreamdestinations) to advance past all buffered data when the buffered-segment loop completes normally. - Add regression tests ensuring
CopyToAsyncadvances buffered data for both destination overloads.
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 |
Ensures buffered segments are fully advanced/cleared after a successful buffered drain in both CopyToAsync overloads. |
src/libraries/System.IO.Pipelines/tests/PipeReaderCopyToAsyncTests.cs |
Adds tests validating that buffered data is not re-observed after CopyToAsync completes. |
stephentoub
approved these changes
Feb 27, 2026
…ests.cs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Both CopyToAsync overloads on StreamPipeReader (PipeWriter and Stream destinations) iterate buffered segments, but on successful drain the loop exits with segment set to null. The finally block only called AdvanceTo when segment was non-null, so buffered data remained in _readHead/_readIndex and would be returned again on subsequent reads.
Add an else-if branch to the finally block that advances past all buffered data (_readTail) when the segment loop completes normally.