Skip to content

Conversation

@infiniteregrets
Copy link

hi, thank you for your work on this library!

after upgrading async-compression, our custom protocol stopped working i.e. receiving responses like for eg, ACKs would get stuck and never reach the client, this happened on release 0.4.33 so i ran git bisect:

good: b79f66d40697ab7dadf791a416ab2ba15cccc3a3
bad: 2aa1b5f8122618004b9bbab6dc679bafca616ff2

and 6c0835eb5dd26cb6176acd0455437be13fdaadac being the one where this problem happens (#402)

i went over the changes and it seemed like the loop exited before setting the counter was set so i set:

self.state = State::Encoding(read)

and it seemed to work on that commit with my sample code, but then again broken when i applied this change on main, so i ran git bisect again etc comparing against my working version and saw some behavior was changed changing AsyncRead impl back to returning Ready whenever data was written to buffer worked

idk how correct this is, but wanted to try to fix this as it did break things with Tower's compression layer enabled for us when we upgraded deps to move from 0.4.32 -> 0.4.36

happy to work on this further and improve this. appreciate any feedback to fix this (:

Copy link
Collaborator

@NobodyXu NobodyXu left a comment

Choose a reason for hiding this comment

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

Thanks!

However when I checked the diff, it seems to be no different from the latest main branch.

We already have an early return

let mut input = PartialBuffer::new(match reader.as_mut().poll_fill_buf(cx)? {

And the read state assignment is no-op.

@NobodyXu
Copy link
Collaborator

NobodyXu commented Dec 22, 2025

cc @infiniteregrets can u try the latest release of async-compression please?

@infiniteregrets
Copy link
Author

cc @infiniteregrets can u try the latest release of async-compression please?

Yeah the first change might just be relevant to that broken commit, I'll re run it. But other changes seem to fix my issues so I'm not sure... will get back to you

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