Hi, I think we could leverage async support built-in in Servlet 3.x. To prevent developers doing long tasks in the detach method, I would suggest an error log trace telling the problem.
Regards, __ Cedric Gatay (@Cedric_Gatay <http://twitter.com/Cedric_Gatay>) http://code-troopers.com | http://www.bloggure.info | http://cedric.gatay.fr On Tue, Jun 25, 2013 at 1:45 PM, Martijn Dashorst < martijn.dasho...@gmail.com> wrote: > The issue reported in > https://issues.apache.org/jira/browse/WICKET-5241 gives pause to two > things: > > 1. The response is not written to the client until the detach has been > performed > > 2. In wicket 1.5 and up the detach phase is done twice > > Ad 1: > > It has been my assumption (apparently wrongly so) that the detach > phase would run after we sent the response to the client, and that we > were unable to report any detach errors to the client due to this. In > Wicket 6 (probably 5 as well) the filter calls > RequestCycle#processRequestAndDetach(), and then flushes the response > to the client. This lets all browsers pause until after the detach > phase. > > The courtesy flush > > In my opinion we should flush first, making life better for our > clients, and then clean up after us (detach the used objects). > > The downside > > I'm not 100% sure if this goes well when you have a long lasting > detach, say 10 seconds or so, and a client sends a new request to the > page, or when the page is stateful, and wicket sends a redirect. > > Here's an improved WicketFilter#processRequestCycle implementation > that flushes prior to detaching: > > protected boolean processRequestCycle(RequestCycle requestCycle, > WebResponse webResponse, HttpServletRequest > httpServletRequest, > HttpServletResponse httpServletResponse, final > FilterChain chain) > throws IOException, ServletException { > // Assume we are able to handle the request > boolean requestHandledByWicket = true; > > try { > requestHandledByWicket = > requestCycle.processRequest(); > if (requestHandledByWicket) { > webResponse.flush(); > } > } finally { > requestCycle.detach(); > } > > if (!requestHandledByWicket && chain != null) { > chain.doFilter(httpServletRequest, > httpServletResponse); > } > return requestHandledByWicket; > } > > Unfortunately, this doesn't work for stateful pages, as they redirect > directly to a buffer. With curl, the request is blocked on the socket > (the connection is reused) making the request block until the detach > has passed. This is also true for at least safari. If the connection > is not reused (new curl request to the stateful redirect url), then > processing can continue as expected. > > With servlet 3 we could potentially do the request asynchronously, not > blocking the socket anymore during detach, with the additional caveat > that newly incoming requests (e.g. a link click) to the detaching page > would still have to wait for the lock to be released—you don't want to > start working with a still detaching page... Note that this is only > relevant for detach cycles that take longer than say 100ms. Anything > shorter would only be noticeable in artificial test scenarios I > imagine. > > What are your thoughts? > > Martijn >
