On 31/12/2014 16:38, Greg Wilkins wrote:
>
>
> On 19 December 2014 at 10:05, Mark Thomas <markt_at_apache.org
> <mailto:markt_at_apache.org>> wrote:
>
>
> However, I don't see why we have this restriction in the first place. As
> I have stated elsewhere on this thread, no explanation was given when
> this restriction was introduced and right now I don't see why we need it
> for sync or async.
>
>
> I agree that this issue is really independent of sync or async.
>
> However, I do not believe it was a restriction that was added - only a
> clarification of the expected behaviour. The reasons I believe that
> included:
>
> * There is an expectation that exceptions thrown by a servlet/filter
> may be caught by an upstream filter or servlet using a
> RequestDispatcher. Allowing arbitrary threads to call into
> doFilter will break this.
That expectation is going to break whether async is started in the
servlet or a filter.
> * ThreadLocals are frequently use by authentication and authorisation
> mechanisms, specially when invoking JEE components. Allowing the
> filter chain to not be scoped by a thread will break all usage of
> thread locals.
ThreadLocals are already broken as soon as async moves to a new thread.
This just 'enables' them to be broken earlier in the processing chain.
> * Code within a filter/servlet chain can expect to be single threaded
> with respect to a particular request. ie while there may be many
> threads within a filter/servlet, there will only be a single thread
> per request. Allowing arbitrary threads to call doFilter would
> allows for the possibility of a request being dispatched multiple times.
An application could do this now by calling Servlet.service() from
within an async thread. We already have the possibility of a request
being dispatched multiple times.
> * If a new thread calls doFilter before the dispatched thread returns
> to the container, then the values of methods like getPathInfo will
> be different for each calling thread.
That depends on how the behaviour of getPathInfo() is defined for async
doesn't it?
> * Such a pattern does no scale to multiple uses. If filter A used an
> new thread to call filter B which used a new thread to call filter
> C, then what can filter A assume when it's new thread returns? Is
> the request handled and the response generated? It does not know
> because that depends on thread created by B. To resolve such
> issues, we would need to create a whole bunch of listeners, that we
> pretty much have already for normally dispatched requests.
The same issue exists now for Servlet.service()
> The original use case [1] that triggered this thread looks reasonable
> and I'd like to support it if possible. The clarification proposed above
> would not allow the use case to be supported.
>
>
> I'm not so sure it is a valid use case. The issue is that a different
> threading paradigm (fibers from quasar) want to be used to invoke
> servlets. I think changing the very nature of the threading of a
> servlet container has to be something done with the involvement of the
> container and not as a filter in an application.
That is a fair point.
> If the application wishes to use fibers to generate a response, then
> they are free to create a framework to do so that writes (async or
> blocking) to the servlet response.
Agreed.
> But I don't see why it is
> reasonable that such a new/different threading paradigm should expect to
> be able to takeover the dispatch and handling model of the servlet
> container.
You have convinced me.
> Specially if authentication and authorisation are involved.
I am not a fan of ThreadLocal at all so that part of the argument
doesn't carry much weight with me.
While I don't agree with many of the points you have made above, you
have convinced me that this particular use case isn't a valid one.
Absent a valid use case, I am happy with the clarification that an ISE
should be thrown.
However, many of the points you raise above are not unique to calling
FilterChain.doFilter() from an async thread and apply equally to calling
Servlet.service(). Do we want to add a similar restriction (although I
don't think the container could enforce it) that Servlet.service() must
only be called by the 'container invocation thread'?
Mark