Skip to content

Fix StreamPipeReader.CopyToAsync not advancing past buffered data#124989

Merged
BrennanConroy merged 2 commits intodotnet:mainfrom
BrennanConroy:brecon/pipelines-copytoasync-data-dup
Mar 2, 2026
Merged

Fix StreamPipeReader.CopyToAsync not advancing past buffered data#124989
BrennanConroy merged 2 commits intodotnet:mainfrom
BrennanConroy:brecon/pipelines-copytoasync-data-dup

Conversation

@BrennanConroy
Copy link
Copy Markdown
Member

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.

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>
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 (both PipeWriter and Stream destinations) to advance past all buffered data when the buffered-segment loop completes normally.
  • Add regression tests ensuring CopyToAsync advances 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.

…ests.cs

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings February 27, 2026 23:35
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

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

@BrennanConroy BrennanConroy enabled auto-merge (squash) February 27, 2026 23:43
@BrennanConroy BrennanConroy merged commit 20650a9 into dotnet:main Mar 2, 2026
91 of 94 checks passed
@BrennanConroy BrennanConroy deleted the brecon/pipelines-copytoasync-data-dup branch March 2, 2026 19:33
@github-actions github-actions bot locked and limited conversation to collaborators Apr 2, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants