Skip to content

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

Merged
merged 1 commit into from
Aug 15, 2015

Conversation

bomgar
Copy link
Contributor

@bomgar bomgar commented Aug 14, 2015

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.

@bomgar
Copy link
Contributor Author

bomgar commented Aug 14, 2015

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.

@slandelle
Copy link
Contributor

Thanks, but would it be possible for you to add some tests, please?

@bomgar
Copy link
Contributor Author

bomgar commented Aug 14, 2015

I added some test for the feedable body generator.

@bomgar
Copy link
Contributor Author

bomgar commented Aug 14, 2015

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.

@slandelle
Copy link
Contributor

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).

@bomgar
Copy link
Contributor Author

bomgar commented Aug 14, 2015

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.

@bomgar
Copy link
Contributor Author

bomgar commented Aug 15, 2015

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?

@bomgar
Copy link
Contributor Author

bomgar commented Aug 15, 2015

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
@slandelle
Copy link
Contributor

@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.

slandelle added a commit that referenced this pull request Aug 15, 2015
Fix stream suspending with FeedableBodyGenerator.
@slandelle slandelle merged commit 2ec4e83 into AsyncHttpClient:master Aug 15, 2015
slandelle added a commit that referenced this pull request Aug 15, 2015
@slandelle slandelle added this to the 1.9.31 milestone Aug 15, 2015
slandelle added a commit that referenced this pull request Aug 15, 2015
slandelle added a commit that referenced this pull request Aug 15, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants