Skip to content

Fixes #62167. WriteAsync may truncate data if called after .Advance(int) #62306

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 1 commit into from
Dec 3, 2021
Merged

Fixes #62167. WriteAsync may truncate data if called after .Advance(int) #62306

merged 1 commit into from
Dec 3, 2021

Conversation

Kuinox
Copy link
Contributor

@Kuinox Kuinox commented Dec 3, 2021

Fixes #62167.
It took me way too much time to pinpoint where this issue came from ^^'.

@ghost ghost added area-System.IO.Pipelines community-contribution Indicates that the PR has been added by a community member labels Dec 3, 2021
halter73
halter73 approved these changes Dec 3, 2021
View reviewed changes
Copy link
Member

@halter73 halter73 left a comment

Choose a reason for hiding this comment

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

Great find!

@halter73
Copy link
Member

halter73 commented Dec 3, 2021 •
edited
Loading

While this is an unusual writing pattern, it is valid. You can get corrupted data without this fix. We should service this.

@danmoseley
Copy link
Member

thanks @Kuinox !

@Kuinox
Copy link
Contributor Author

Kuinox commented Dec 3, 2021

While this is an unusual writing pattern

I use this pattern to write an user provided buffer.
In the first part, I write the small packet header, then I call WriteAsync to write the user data.

davidfowl
davidfowl approved these changes Dec 3, 2021
View reviewed changes
Copy link
Member

@davidfowl davidfowl left a comment

Choose a reason for hiding this comment

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

Old bug it seems!

@BrennanConroy BrennanConroy merged commit e4faf19 into dotnet:main Dec 3, 2021
@BrennanConroy
Copy link
Member

Thanks a lot @Kuinox!

@BrennanConroy
Copy link
Member

/backport to release/6.0

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2021

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/1536724263

@github-actions github-actions bot mentioned this pull request Dec 3, 2021
Merged
@BrennanConroy
Copy link
Member

/backport to release/5.0

@github-actions
Copy link
Contributor

github-actions bot commented Dec 3, 2021

Started backporting to release/5.0: https://github.com/dotnet/runtime/actions/runs/1536755422

@github-actions github-actions bot mentioned this pull request Dec 3, 2021
Merged
@BrennanConroy BrennanConroy mentioned this pull request Dec 3, 2021
Merged
@Kuinox
Copy link
Contributor Author

Kuinox commented Dec 4, 2021

Thanks everyone !

@davidfowl
Copy link
Member

Thank you!

@ghost ghost locked as resolved and limited conversation to collaborators Jan 7, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@halter73 halter73 halter73 approved these changes

@davidfowl davidfowl davidfowl approved these changes

@BrennanConroy BrennanConroy BrennanConroy approved these changes

Assignees
No one assigned
Labels
area-System.IO.Pipelines community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Calling Pipe.Writer.WriteAsync after Pipe.Writer.Advance flush truncated data to the PipeReader.
5 participants
@Kuinox @halter73 @danmoseley @BrennanConroy @davidfowl