Skip to content
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

if actual bytes to be written is negative, set to zero #58575

Merged
merged 4 commits into from
Oct 25, 2024
Merged

if actual bytes to be written is negative, set to zero #58575

merged 4 commits into from
Oct 25, 2024

Conversation

bhaskarqlik
Copy link
Contributor

@bhaskarqlik bhaskarqlik commented Oct 22, 2024 •
edited
Loading

if actual bytes to be written is negative, set to zero

  • You've read the Contributor Guide and Code of Conduct.
  • You've included unit or integration tests for your change, where applicable.
  • You've included inline docs for your change, where applicable.
  • There's an open issue for the PR that you are making. If you'd like to propose a new feature or change, please open an issue to discuss the change or find an existing issue.

Description

Fixes #47452

When actual is zero, it will wait for stream window update.

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions label Oct 22, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Oct 22, 2024
@bhaskarqlik bhaskarqlik changed the title wait for window updates, if stream window becomes negative wait for window updates, if actual bytes to written becomes negative Oct 22, 2024
@bhaskarqlik bhaskarqlik changed the title wait for window updates, if actual bytes to written becomes negative wait for window updates, if actual bytes to be written becomes negative Oct 22, 2024
@bhaskarqlik bhaskarqlik changed the title wait for window updates, if actual bytes to be written becomes negative if actual bytes to be written becomes negative, set to zero Oct 22, 2024
@bhaskarqlik bhaskarqlik changed the title if actual bytes to be written becomes negative, set to zero if actual bytes to be written is negative, set to zero Oct 22, 2024
@bhaskarqlik bhaskarqlik marked this pull request as ready for review October 22, 2024 20:52
amcasey
amcasey approved these changes Oct 24, 2024
View reviewed changes
Copy link
Member

@amcasey amcasey left a comment

Choose a reason for hiding this comment

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

I added a suggestion for a more detailed comment, but I don't feel strongly about it - feel free to ignore it if it's too much.

@amcasey
Copy link
Member

amcasey commented Oct 24, 2024

Any remaining concerns @mgravell?

@amcasey amcasey enabled auto-merge (squash) October 25, 2024 18:32
@amcasey amcasey merged commit 397ef0a into dotnet:main Oct 25, 2024
27 checks passed
@dotnet-policy-service dotnet-policy-service bot added this to the 10.0-preview1 milestone Oct 25, 2024
@bhaskarqlik
Copy link
Contributor Author

bhaskarqlik commented Oct 31, 2024 •
edited
Loading

@amcasey thanks for merging this
can we also back port this fix to 8.0, let me know if I need to create a PR for it.

@amcasey
Copy link
Member

amcasey commented Nov 4, 2024

@BrennanConroy @adityamandaleeka to consider for backporting

@Krish-1469
Copy link

@BrennanConroy @adityamandaleeka to consider for backporting

Hi,
We are also facing this issue with 8.0 , Can you please let us know when it is planned to be merged to 8.0 and to be released.

Thanks

@mgravell
Copy link
Member

Backport isn't an automatic / "given"; however, the change is small, reasonably safe, and doesn't involve API, so it isn't a hard "no" either, and there's multiple people requesting it. I'll raise it with the team.

captainsafia pushed a commit that referenced this pull request Feb 11, 2025
@bhaskarqlik @amcasey @captainsafia
if actual bytes to be written is negative, set to zero (#58575)
d6253af 
* wait for window updates, if stream window becomes negative

* update comment

* fix typo

* update comment per suggestion

Co-authored-by: Andrew Casey <[email protected]>

---------

Co-authored-by: Andrew Casey <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgravell mgravell mgravell left review comments

@amcasey amcasey amcasey approved these changes

@halter73 halter73 Awaiting requested review from halter73 halter73 is a code owner

@BrennanConroy BrennanConroy Awaiting requested review from BrennanConroy BrennanConroy is a code owner

@JamesNK JamesNK Awaiting requested review from JamesNK JamesNK is a code owner

Assignees

@adityamandaleeka adityamandaleeka

Labels
area-networking Includes servers, yarp, json patch, bedrock, websockets, http client factory, and http abstractions community-contribution Indicates that the PR has been added by a community member
Projects
None yet
Milestone
10.0-preview1
Development

Successfully merging this pull request may close these issues.

Decreasing the window size below the amount of data sent can prematurely abort the connection
5 participants
@bhaskarqlik @amcasey @Krish-1469 @mgravell @adityamandaleeka