Skip to content

RetryFilter: Cleanup pending byte stream#29245

Merged
yashykt merged 2 commits intogrpc:masterfrom
yashykt:RetryCodeCleanpu
Apr 1, 2022
Merged

RetryFilter: Cleanup pending byte stream#29245
yashykt merged 2 commits intogrpc:masterfrom
yashykt:RetryCodeCleanpu

Conversation

@yashykt
Copy link
Copy Markdown
Member

@yashykt yashykt commented Mar 29, 2022

Fixes the memory leak observed in internal bug b/226254012

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Mar 30, 2022

As a quick experiment, I tried out #29263 to not wait on the ByteStream being cleared out before we invoke recv_trailing_metadata and that started causing issues in our server streaming tests which expect the messages to be received before the status.

I think we should be able to fix it if we change our surface to always wait for recv_message ops to complete and the byte stream to be read before completing the recv_status op, but this change seems easier.

Comment thread src/core/ext/filters/client_channel/retry_filter.cc Outdated
Copy link
Copy Markdown
Member

@markdroth markdroth left a comment

Choose a reason for hiding this comment

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

Thanks for the fix!

@yashykt
Copy link
Copy Markdown
Member Author

yashykt commented Apr 1, 2022

Thanks for reviewing!

@yashykt yashykt merged commit 3cb6432 into grpc:master Apr 1, 2022
@copybara-service copybara-service Bot added the imported Specifies if the PR has been imported to the internal repository label Apr 1, 2022
@yashykt yashykt deleted the RetryCodeCleanpu branch May 18, 2023 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bloat/none imported Specifies if the PR has been imported to the internal repository lang/core per-call-memory/neutral perf-change/improvement release notes: yes Indicates if PR needs to be in release notes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants