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

HttpDechunker sends EOF before response is finished #84

Closed
stephenjudkins opened this issue May 10, 2012 · 2 comments
Closed

HttpDechunker sends EOF before response is finished #84

stephenjudkins opened this issue May 10, 2012 · 2 comments

Comments

@stephenjudkins
Copy link
Contributor

In some cases, HttpDechunker sends an EOF before it has finished writing the all chunks to StreamResponse#messages. This is because in HttpDechunker.scala#L39, when chunk.readable is false, the sendOf offer is set to Offer.const(()). In L43, this offer is synchronized on in order to send EOF message only after it's finished. However, when chunk.readable is false, an EOF is sent immediately irrespective of any other chunks that haven't been read.

stephenjudkins added a commit to stephenjudkins/finagle that referenced this issue May 10, 2012
stephenjudkins added a commit to stephenjudkins/finagle that referenced this issue May 10, 2012
This is difficult to test because it relies on "setReadable" not propogating fast enough. Messages needs to be sent to HttpDechunker after setReadable(false), which does happen.
@mariusae
Copy link
Contributor

This is somewhat interesting—I'm a bit split about what to do exactly. You can always get the behavior you want by sequencing syncs; but it seems like serialization of messages is desirable generally. I think, however, that the correct solution is to sequence these explicitly: don't wait for the next upstream until the previous syncd.

stephenjudkins added a commit to stephenjudkins/finagle that referenced this issue May 18, 2012
@mariusae
Copy link
Contributor

Fixed with 919fd54. Thanks!

suncelesta pushed a commit to suncelesta/suncelesta.github.com that referenced this issue Aug 2, 2014
…l messages

Github-issue: twitter/finagle#84
Signed-off-by: marius a. eriksen <marius@twitter.com>
vkostyukov pushed a commit to finagle/finagle-stream that referenced this issue Mar 17, 2016
…l messages

Github-issue: twitter/finagle#84
Signed-off-by: marius a. eriksen <marius@twitter.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants