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.

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

bug: Flush the write buffer before putting it to the pool #1672

Merged
merged 1 commit into from
Dec 2, 2023

Conversation

Jille
Copy link
Contributor

@Jille Jille commented Dec 1, 2023

A few lines later we check if s.ReduceMemoryUsage && hijackHandler == nil and call releaseWriter. We need to flush the buffer before returning it to the pool to avoid the data getting lost.

A few lines later we check if `s.ReduceMemoryUsage && hijackHandler == nil`
and call releaseWriter. We need to flush the buffer before returning it
to the pool to avoid the data getting lost.
@Jille
Copy link
Contributor Author

Jille commented Dec 1, 2023

This is a theoretical bugfix. I've not observed any problems in practice, I was just reading through the code.
I'm not experienced with fasthttp at all, so there's a solid chance I'm missing something.

@erikdubbelboer erikdubbelboer merged commit 9b4e42a into valyala:master Dec 2, 2023
13 of 14 checks passed
@erikdubbelboer
Copy link
Collaborator

Thanks! I haven't observed it either but from the code it makes sense that it could happen.

@Jille Jille deleted the reduce-memory-usage-flush branch December 3, 2023 20:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants