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

IOException "Closed while Pending/Unready" with WebFlux on Jetty #24050

Closed
kptfh opened this issue Nov 21, 2019 · 7 comments
Closed

IOException "Closed while Pending/Unready" with WebFlux on Jetty #24050

kptfh opened this issue Nov 21, 2019 · 7 comments

Comments

@kptfh
Copy link

@kptfh kptfh commented Nov 21, 2019

Affects: \Greenwich.SR3

Avoid calling Servlet APIs such as ServletOutputStream.close() or AsyncContext.complete() when previous writes are pending

10:24:13.788 [qtp640161448-137] TRACE org.springframework.http.server.reactive.ServletHttpHandlerAdapter - [5df01b5c] Handling completed
10:24:13.785 [qtp640161448-20] WARN org.eclipse.jetty.server.HttpOutput - java.io.IOException: Closed while Pending/Unready

You can find more details in Guys from jetty investigation

It can be easily reproduced with Reactive Feign test

@jhoeller

This comment has been minimized.

Copy link
Contributor

@jhoeller jhoeller commented Nov 21, 2019

@rstoyanchev This looks valid... Interesting that it only materializes with HTTP/2.

@kptfh

This comment has been minimized.

Copy link
Author

@kptfh kptfh commented Nov 21, 2019

The HTTP/1.1 version works well because the (small) writes are never stalled and all happen in different connections.

For HTTP/2, the concurrency makes it so a request trying to write the response content is stalled because other threads are currently writing to the single connection that HTTP/2 uses.
Because the write is non-blocking, it queues the buffer to write and return immediately, allowing the code that calls AsyncContext.complete() to run before the write is finished.

@rstoyanchev rstoyanchev changed the title Inappropriate usage of the Servlet 4.0 async I/O APIs IOException "Closed while Pending/Unready" with WebFlux on Jetty Nov 22, 2019
@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Nov 22, 2019

It isn't documented anywhere that isReady must be called before AsyncContext.complete(), so I don't see how it can be called bad or even inappropriate, and if I've followed the discussion under eclipse-ee4j/servlet-api#273 in Tomcat and Undertow this isn't an issue.

That said I do understand the reasons and would be happy if we can help to eliminate the issue from our end, but from an initial look there doesn't seem to be an easy solution. The code that completes the AsyncContext at the end of the reactive chain is quite a bit removed from the code that does the writing through the WriteListener callbacks. I will keep looking.

For what it's worth the concerns that @gregw explained under eclipse-ee4j/servlet-api#273 I think don't apply to WebFlux where completion is guaranteed to occur after writing (i.e. they don't compete) and also we do not do any further processing after the call to AsyncContext#complete. I suppose that doesn't help in the general case but I thought it would be worth mentioning.

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Nov 22, 2019

I've identified a potential fix in our WriteListener.

@rstoyanchev

This comment has been minimized.

Copy link
Contributor

@rstoyanchev rstoyanchev commented Nov 22, 2019

Thanks for the report @kptfh. I've verified the fix with the Reactive Feign test, but if you can confirm the fix from your end with a 5.1.12.BUILD-SNAPSHOT that would be great.

@sbordet

This comment has been minimized.

Copy link

@sbordet sbordet commented Nov 27, 2019

@rstoyanchev I am able to confirm that 5.1.12.BUILD-SNAPSHOT and 5.2.2.BUILD-SNAPSHOT work fine with a vanilla Jetty reactive HttpClient. Thanks!

@kptfh

This comment has been minimized.

Copy link
Author

@kptfh kptfh commented Nov 27, 2019

Reactive Feign tests also got fixed. Thank you. Will wait for release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.