I'm +1000000 on Stuarts concern about multiple threads dispatched into the
filter/servlet chain at the same time.
Originally the async servlet design was done not for async IO. It's design
was primarily motivated by startAsync() to AsyncContext.dispatch() style
handling, so that all request handling and response generation was still to
be done within a container thread dispatched to the filter/servlet using
blocking APIs.
This design was modified to support the startAsync() to
AsyncContext.complete() style of usage where the asynchronous thread writes
the response and then calls complete. The problem is that we never
really well defined what methods on the request/response can be safely
called by such asynchronous threads.
We have now added async IO as a further extension this start->complete
style, but still have not well defined what methods can be called. I think
if we add asynchronous concurrent calls to the filter/servlet then we will
have a real mess.
Consider the following example:
- Context A has Servlet X
- Context B has Servlets Y & Z each with a Filter P and Q mapped in
front of them
- Context A Servlet X does a cross context forward to Context B Servlet
Y
- Context B Filter P does does a dispatch forward to Servlet Z
- Context B Filter Q calls startAsync, starts a new thread which calls
chain.doFilter, the returns.
So now we have 2 threads in the servlet container handling the same
request. One of them is trying to call Context B Servlet Z and the other
is returning back via the stack of invocations.
In this situation, what are the return values from methods like
getContextPath, getSerlvetPath and getPathInfo? as the original
thread returns back via its call stack, the return values for these methods
will change from B/Z to B/Y and finally back to A/X. The async thread
calling doFilter can reasonably expect these methods to return B/Z for the
duration of the call.
Note that we can't solve this problem by request wrapping as the spec
requires == for the request objects passed into forward with that used in
the next dispatch.
Are we going to have to implement these methods with ThreadLocals?
The Request/Response and RequestDispatcher classes were just not designed
for multithreaded access. We have gone to considerable lengths to ensure
only a single thread is ever dispatched to the servlet container (eg
onWritePossible and onReadPossible are delayed until the original thread
exits the container). So I don't think we can realistically throw this
all away and suddenly allow arbitrary threads to be simultaneously calling
the filter/servlet
cheerss
On 9 December 2014 at 00:13, Stuart Douglas <stuart.w.douglas_at_gmail.com>
wrote:
>
>
> Edward Burns wrote:
>
>> On Tue, 09 Dec 2014 08:04:50 +1100, Stuart Douglas<sdouglas_at_redhat.com>
>>>>>>> said:
>>>>>>>
>>>>>>
>> MT> Section 6.2.3 includes the following text:
>> MT> <quote>
>> MT> A Filter and the target servlet or resource at the end of the filter
>> MT> chain must execute in the same invocation thread.
>> MT> </quote>
>> MT>
>> MT> As a result of a bug raised against Apache Tomcat [1] I am seeking
>> MT> clarification of the meaning of that sentence.
>> MT>
>> MT> As written, the meaning seems unambiguous. However, does it apply
>> when
>> MT> the request is in asynchronous mode? To put it another way is the
>> MT> following legal?
>> MT> - filter calls ServletRequest.startAsync()
>> MT> - filter calls AsyncContext.start(Runnable)
>> MT> - That Runnable called FilterChain.doFilter()
>>
>> SD> Another potential problem with doing this is that
>> AsyncContext.start()
>> SD> does not wait for the current call stack to return to the container
>> (at
>> SD> least it is not required to). This means that you will have two
>> threads
>> SD> potentially modifying the response, which is not thread safe.
>>
>> SD> We could modify the behaviour of AsyncContext.start() to wait for the
>> SD> request to return to the container before actually running the task.
>>
>> SD> I think that this is actually a real thread safety problem in the
>> SD> Servlet API, any code that uses AsyncContext.start() and then wants
>> to
>> SD> modify the response cannot be sure the original call stack has
>> returned,
>> SD> and could potentially have thread safety issues.
>>
>> Being new to this spec, I went looking for some catch-all text that
>> calls out these obvious thread safety issues. Indeed, there is section
>> 2.3.3.4 Thread Safety:
>>
>> Spec> Other than the startAsync and complete methods, implementations of
>> Spec> the request and response objects are not guaranteed to be thread
>> Spec> safe. This means that they should either only be used within the
>> Spec> scope of the request handling thread or the application must
>> Spec> ensure that access to the request and response objects are thread
>> Spec> safe.
>>
>> Spec> If a thread created by the application uses the container-managed
>> Spec> objects, such as the request or response object, those objects
>> Spec> must be accessed only within the object's life cycle as
>> Spec> defined in sections Section 3.12, "Lifetime of the Request
>> Spec> Object" on page 3-31and Section 5.7, "Lifetime of the
>> Spec> Response Object" on page 5-50 respectively. Be aware that
>> Spec> other than the startAsync, and complete methods, the request and
>> Spec> response objects are not thread safe. If those objects were
>> Spec> accessed in the multiple threads, the access should be
>> Spec> synchronized or be done through a wrapper to add the thread
>> Spec> safety, for instance, synchronizing the call of the methods to
>> Spec> access the request attribute, or using a local output stream for
>> Spec> the response object within a thread.
>>
>> Need we say more?
>>
>
> I guess my main concern here is that there is not really any way to hand
> off to another thread so that only one thread can be using the
> request/response at a time.
>
> Basically I am talking about some kind of AsyncContext.delayStart()
> construct that will not dispatch the task until the current call stack has
> returned to the container (or just change the semantics of start() to
> match). This will make sure that only one thread is using the request and
> response, without needing to resort to synchronization or other thread
> safety constructs.
>
> Stuart
>
>
>
>
>
>> Ed
>>
>>
--
Greg Wilkins <gregw_at_intalio.com> @ Webtide - *an Intalio subsidiary*
http://eclipse.org/jetty HTTP, SPDY, Websocket server and client that scales
http://www.webtide.com advice and support for jetty and cometd.