Fix #5605 Unblock non container Threads #5936
Conversation
Ensure that HttpInput is always closed to EOF, EarlyEOF or Error, so that non container threads doing blocking reads will not block forever, even if late. Delay recycling of HttpInput until next request is received.
|
@lorban @sbordet This is looking reasonable on the read side. I added some explicit tests which it passes. It has also passed Ludo's stress test reproduction. However, I have left it as draft for now, as I think we may need to do something similar on the write side. Give me your thoughts on the read side approach and then I'll look at the write side. |
Ensure that HttpInput is always closed to EOF, EarlyEOF or Error, so that non container threads doing blocking reads will not block forever, even if late. Delay recycling of HttpInput until next request is received.
test and fixes for the write side.
test and fixes for the write side.
Don't consumeAll before upgrade
reorder
fix test
cleanup debug
revert fillInterest cancellation and just abort connection instead. tested for all transports
|
actually hold off... I think I can see some simplifications. I think we should abort if we have pending async operations as well.... |
Simplification. Always abort on any pending read or write in completion.
…05-wakeup-blocked-threads
Simplification. Always abort on any pending read or write in completion.
jetty-server/src/main/java/org/eclipse/jetty/server/HttpOutput.java
Outdated
Show resolved
Hide resolved
jetty-util/src/main/java/org/eclipse/jetty/util/SharedBlockingCallback.java
Show resolved
Hide resolved
tests/test-http-client-transport/src/test/java/org/eclipse/jetty/http/client/BlockedIOTest.java
Show resolved
Hide resolved
jetty-server/src/main/java/org/eclipse/jetty/server/HttpInput.java
Outdated
Show resolved
Hide resolved
jetty-server/src/main/java/org/eclipse/jetty/server/Request.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
You've go my pre-approval. Changes look good the way they are besides a minor nit in one test that is more a matter of preference than anything else.
But the original bug reporter said he would test this branch early this week so I think it's worth holding off our merge until we get feedback.
refactored the complete method to consider unrecoverable API states no matter what the httpout state actually is. This avoid duplication of OPEN, CLOSING, CLOSED etc. handling.
|
@sbordet nudge |
|
rerunning CI, which was aborted for some reason??? |
|
@lorban write side was something else. |
|
@gregw yes, indeed. I forgot to update this PR after we discovered what the problem was on Friday, and the fact that it was specific to 10.0.x. |
|
@sbordet nudge! |
jetty-server/src/main/java/org/eclipse/jetty/server/HttpConnection.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
jetty-server/src/test/java/org/eclipse/jetty/server/BlockingTest.java
Outdated
Show resolved
Hide resolved
…05-wakeup-blocked-threads
Ensure that
HttpInputis always closed to EOF, EarlyEOF or Error, so that non container threads doing blocking readswill not block forever, even if late. Delay recycling of HttpInput until next request is received.
Fixes #5605 replaces #5923