jakartaee / servlet

Jakarta Servlet
https://eclipse.org/ee4j/servlet
Other
259 stars 83 forks source link

AsyncContext ISE inconsistency #103

Open glassfishrobot opened 10 years ago

glassfishrobot commented 10 years ago

The javadoc for when AsyncContext should throw an ISE is inconsistent:

getRequest() IllegalStateException - if complete() or any of the dispatch() methods has been called in the asynchronous cycle

addListener(...) IllegalStateException - if this method is called after the container-initiated dispatch, during which one of the ServletRequest.startAsync() methods was called, has returned to the container

So getRequest fails with an ISE immediately after dispatch or complete have been called (even if called in another thread), while addListener waits until after the current dispatch returns before throwing any ISE.

These should be consistent, because a filter that is trying to add a Listener may need to call getRequest in order to obtain the wrapped request. Currently there is a race for such code that will fail if another thread calls dispatch/complete before the thread dispatched to the filters/servlet has returned.

glassfishrobot commented 6 years ago
glassfishrobot commented 10 years ago

@glassfishrobot Commented Reported by gregwilkins

glassfishrobot commented 10 years ago

@glassfishrobot Commented gregwilkins said: Note also, it needs to be clarified if getRequest() can be called during an onComplete callback (triggered by a call to complete()). Strictly speaking complete() "has been called", but getting the request is valuable in an onComplete callback for logging, and request attribute access.

glassfishrobot commented 10 years ago

@glassfishrobot Commented gregwilkins said: On further analysis, I believe that throwing an ISE from getRequest() is unnecessary before the onComplete() callbacks have been called.

I have a use case where a onComplete callback is used to log a request/response as follows:

@Override public void onComplete(AsyncEvent event) throws IOException

{ AsyncContextState context = (AsyncContextState)event.getAsyncContext(); Request request=context.getHttpChannelState().getBaseRequest(); Response response=request.getResponse(); _requestLog.log(request,response); }

If getRequest is unavailable once dispatch() has been called, then such use-cases will need to take copies of the request/response and create per request instances of the listener. This makes it pointless passing in the event to onComplete as it cannot be used to access anything about the request.

glassfishrobot commented 9 years ago

@glassfishrobot Commented tremes said: Hi! I could be wrong but I can see some next inconsistency. When I do this then the ISE is thrown:

public void onComplete(AsyncEvent event) {
       event.getAsyncContext().getResponse());
}

But what about this. Shouldn't be ISE thrown as well??

public void onComplete(AsyncEvent event) {
       event.getSuppliedResponse();
}
glassfishrobot commented 7 years ago

@glassfishrobot Commented This issue was imported from java.net JIRA SERVLET_SPEC-103