-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Fix stream suspending with FeedableBodyGenerator. #948
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
Conversation
I have backported the code to 1.9 and tested it with netty3. I'm not sure if the instanceof is necessary but i want to avoid breaking other callers with NPEs. |
Thanks, but would it be possible for you to add some tests, please? |
I added some test for the feedable body generator. |
I'm actually wondering how this thing was tested before. I had a lot of trouble analyzing the errors. I have no idea if this is compatible with netty 4. |
Currently, there's no test for FeedableBodyGenerator. But it would be great to have one. Please have a look at our test suite: you can start a Jetty server, and configure how it replies (eg echo). |
The unit test in my second commit should cover the body generator. Unfortunately the BodyChunkedInput is still lacking one. Without this change the netty thread did non stop call read on the body. I will see what I can do to make a better test. But the change is necessary. |
I have a test now with netty 3 and 4. With netty 3 everything seems to work. With netty 4 the server replies the chunk boundaries. Any ideas? |
Now it seems to work with both version. I copied the netty 3 issue hack from the InputStreamBodyGenerator. |
netty expects null as outgoing buffer to suspend the stream. see ChunkedWriteHandler#222 - Added unit test for feedable body generator - Fix netty 4 issue with feedable body generator - provide integration test
@bomgar LGTM, thanks a lot! I'll probably change a few bits, like making the default behavior target Netty 4 and not old Netty 3. |
Fix stream suspending with FeedableBodyGenerator.
netty expects null as outgoing buffer to suspend the stream.
see ChunkedWriteHandler#222
Also empty buffers need to be ignored to avoid suspending the stream unnecessarily.