About the twice called #onDetach() - when I found it (WICKET-4012) Igor explained to me that there is no contract for #onDetach() to be called just once per request cycle, so it is fine. And usually the impl of #onDetach() is either empty or something trivial and fast.
But I think we should try to remove one of the calls and call cycle.detach() after flushing the response. I prefer to do this change in master branch (7.x) only for now. On Wed, Jun 26, 2013 at 11:18 AM, Martin Grigorov <[email protected]>wrote: > Hi, > > > On Wed, Jun 26, 2013 at 10:38 AM, Andrea Del Bene <[email protected]>wrote: > >> I think Martin had created an issue about it some weeks ago... > > > Yes. > For the async dispatching - few weeks ago > (WICKET-5152<https://issues.apache.org/jira/browse/WICKET-5152>) > , for the twice called #onDetach() around 100 weeks ago > (WICKET-4012<https://issues.apache.org/jira/browse/WICKET-4012> > ). > > >> >> 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. >>> >> > I am not sure you actually can. > If you start async context then you cannot flush the response's buffer. > This is how I ended with WICKET-5152. But I may be wrong. > > > >>> Regards, >>> >>> __ >>> Cedric Gatay (@Cedric_Gatay >>> <http://twitter.com/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 < >>> [email protected]> wrote: >>> >>> The issue reported in >>>> https://issues.apache.org/**jira/browse/WICKET-5241<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. >>>> >>> > I think the main differentiator here is the IO type - BIO or NIO. > With BIO you would end with what you describe. > With NIO a new thread would be used for the following request and all > should be fine. > > >> >>>> What are your thoughts? >>>> >>>> Martijn >>>> >>>> >> >
