Issue #10933 - Fix AsyncIOServlet test issues #10949
Conversation
joakime
changed the title
|
@lorban @sbordet @lachlan-roberts This is going to need careful review. Your early thoughts would be appreciated |
|
@gregw what release of Jetty 12 do you want to see this in? |
It doesn't need to be in the next release... but if it is ready, then no reason why not. I'll add to the project and we can punt if it is not ready |
gregw
changed the title
|
@sbordet @lorban @lachlan-roberts nudge! |
|
@gregw please add someone as reviewer, or we won't see it. |
gregw
requested review from
sbordet,
lorban and
lachlan-roberts
and removed request for
lorban
oops |
jetty-core/jetty-server/src/test/java/org/eclipse/jetty/server/RequestListenersTest.java
Outdated
Show resolved
Hide resolved
| { | ||
| _failureListener = true; | ||
| _servletChannel.getRequest().addFailureListener(this::asyncError); | ||
| } |
There was a problem hiding this comment.
Can't you just add this in the constructor, like the idle timeout listener?
Also, should not there be a similar code in ee9?
There was a problem hiding this comment.
Only needed if startAsync is called
Added to ee9, but needs your PR to really test it.
Most often failures come from the read side, so failure listeners are now serialized in the _readInvoker. This avoids that a failure while parsing a request (e.g. an early EOF) results in concurrent executions of the invokeOnContentAvailable task and the invokeOnFailureListeners task. Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Fixed HttpChannelTest.testOnFailure(). Signed-off-by: Simone Bordet <simone.bordet@gmail.com>
Fixes #10933