-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
servlet: ignore IllegalStateException from AsyncContext.complete() #11819
base: master
Are you sure you want to change the base?
Conversation
asyncContext.complete(); | ||
} catch (IllegalStateException ignored) { | ||
// Tomcat can throw: | ||
// Calling [asyncComplete()] is not valid for a request with Async state [COMPLETING] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried reproing by inducing a timeout and I see that the org.eclipse.jetty.server.HttpChannelState is getting set to EXPIRE
if (this._requestState == HttpChannelState.RequestState.ASYNC) {
this._requestState = HttpChannelState.RequestState.EXPIRE;
timeout:615, HttpChannelState (org.eclipse.jetty.server)
run:148, AsyncContextEvent (org.eclipse.jetty.server)
call:539, Executors$RunnableAdapter (java.util.concurrent)
run:264, FutureTask (java.util.concurrent)
run:304, ScheduledThreadPoolExecutor$ScheduledFutureTask (java.util.concurrent)
runWorker:1136, ThreadPoolExecutor (java.util.concurrent)
run:635, ThreadPoolExecutor$Worker (java.util.concurrent)
run:833, Thread (java.lang)
and that the exception thrown from asyncContext.complete(); has the message
s=HANDLING rs=EXPIRE os=OPEN is=IDLE awp=false se=false i=false al=1
It did not have the state COMPLETING.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- The message is different in different servers (Tomcat / Jetty), as state names is an implementation detail.
- It happens multiple times when running tests for this module, like connection closed by client after deadline exceeded, etc
- I have tried to add more checks before this call, some exceptions can be avoided with that, but then decided it does not worth it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IllegalStateException means we've either called something incorrectly (so we should fix it) or there's a bug in the servlet container (and we should file a bug). I don't think we should have something like this without a tracking issue to fix whatever the actual bug is, as this should be removed medium-term.
There are errors logged in case of DEADLINE_EXCEEDED which I would like to avoid.