tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Peter Roßbach ...@objektpark.de>
Subject Re: Proposal: Some todo's at AsyncContext detected...
Date Wed, 30 Jun 2010 19:23:10 GMT
Hi Marc,


Am 30.06.2010 um 20:03 schrieb Mark Thomas:

> On 30/06/2010 17:01, Marc Guillemot wrote:
>> Hi,
>>
>> What you write means that the freshly release Tomcat 7 doesn't fully
>> implement Servlet API 3.0, isn't it?
>>
>> Interesting to see that nobody reacts :-(
>
> If you were following the dev list you would have seen that Jean- 
> Frederic has already vetoed the change Peter made on the basis that  
> it breaks the TCK. That suggests that Peter's analysis of the  
> current situation is not 100% correct.
>
Which of my findings aren't correct?

Peter

> My recollection is that the last thing that Filip implemented for  
> Servlet 3.0 was the necessary timeout features. I'd be surprised if  
> Filip, as a member of the Servlet Expert Group that wrote the spec,  
> missed something fundamental. I wouldn't be surprised if there were  
> some edge cases missed. I do know that Filip tended to leave a lot  
> of the Servlet 3.0 TODOs in the code, even when he had implemented  
> the appropriate feature. Those certainly need cleaning up.
>


> Looking at Peter's test case - along with the other issues folks  
> have raised in Bugzilla for the 7.0.0 release - is on my todo list  
> but like the other committers I have a day job. Whilst many of the  
> committers are fortunate enough (me included) to be able to allocate  
> some work time to Tomcat development, it doesn't follow one of the  
> other committers will immediately have the time necessary to look at  
> Peter's e-mail and comment in detail. I can't speak for the others,  
> but my free time is currently occupied with another Tomcat 7 issue -  
> namely http://tomcat.markmail.org/thread/25g353eqvqfqniiz
>
> Like any area of Tomcat, help is always appreciated. If you are  
> interested in this issue and would like to help out I would  
> encourage you to look at Peter's test case, compare the expected  
> behaviour to what the spec defines and offer your view on whether  
> there is a bug that needs fixing or not. If there is an issue,  
> patches would be even better.
>
> Mark
>
>
>>
>> Cheers,
>> Marc.
>>
>> Peter Roßbach wrote:
>>> Hi!
>>>
>>> I detected some Todo's at AsyncContext
>>>
>>> - AsyncContext.createListener
>>> We don't make a resource injection or make it configurable!
>>>
>>> Servlet 3.0 API comment:
>>> "This method supports resource injection if the given clazz  
>>> represents
>>> a Managed Bean. See the Java EE platform and JSR 299 specifications
>>> for additional details about Managed Beans and resource injection."
>>>
>>> - I miss the is[Started,Completed] methods at AsyncContext interface
>>> The only way to detect a completed state at AsyncContext inside an
>>> AsyncListener is:
>>> if (ac.getResponse() != null)
>>> AsyncListener got an event, with empty state... -- Very strange!
>>>
>>> Can't we clear all AsyncListener after send complete and before we
>>> refresh the AsyncContext state?
>>> How can a user handle a completed situation?
>>> see o.a.c.core.TestAsyncListener
>>>
>>> - AsyncContext.start() dosen't really start a thread!
>>> We create wrapper of a Runnable, but we don't start a thread...
>>> Error handling seam a little bit strange.
>>> We only throw a RuntimeException(ex) and log an error!
>>>
>>> Servlet API comment:
>>> "Causes the container to dispatch a thread, possibly from a managed
>>> thread pool, to run the specified Runnable. The container may
>>> propagate appropriate contextual information to the Runnable."
>>>
>>> How can we implement that?
>>> Start everytime a new thread without pooling?
>>> Use the connector executor pool from request?
>>> Then we must extend the ProtocolHandler interface with getExecutor()
>>> signature.
>>> Use a separate Executor pool per Engine/Host or Context
>>> How do we have to implemented the error handling?
>>>
>>> - JioEndpoint has a timeout detection thread, but we don't start it!
>>> I easy fix it with revision 958362,
>>> but the polling strategy is slow and ineffectiv.
>>> I didn't make a test at NIO/APR-HTTP or AJP-Connectors.
>>>
>>> - After the timeout event is emitted, the completed method is
>>> automatically called!
>>> Servlet API 3.0 Spec says:
>>> ====
>>> ■In the event that an asynchronous operation times out, the  
>>> container
>>> must run
>>> through the following steps:
>>> ■ Invoke the AsyncListener.onTimeout method on all the  
>>> AsyncListener
>>> instances registered with the ServletRequest on which the  
>>> asynchronous
>>> operation was initiated.
>>> ■ If none of the listeners called AsyncContext.complete() or any  
>>> of the
>>> AsyncContext.dispatch methods, perform an error dispatch with a  
>>> status
>>> code equal to HttpServletResponse.SC_INTERNAL_SERVER_ERROR.
>>> ■ If no matching error page was found, or the error page did not  
>>> call
>>> AsyncContext.complete() or any of the AsyncContext.dispatch
>>> methods, the container MUST call AsyncContext.complete().
>>> ===
>>> I miss the implementation that we call the error page!
>>>
>>> - Currently a AsyncListener can't reinit a async cycle!
>>> see o.a.c.connector.Request.startAsync
>>> ... L1555ff
>>> if (asyncContext==null) {
>>> asyncContext = new AsyncContextImpl(this);
>>> } else if (asyncContext.isStarted()) {
>>> throw new IllegalStateException("Already started.");
>>> }
>>> ...
>>>
>>> What does this spec definition really mean?
>>> === Ch 2 Page 18
>>> ■public void onStartAsync(AsyncEvent event) - Is used to notify  
>>> the
>>> listener that a new asynchronous cycle is being initiated via a call
>>> to one of the
>>> ServletRequest.startAsync methods. The AsyncContext corresponding
>>> to the asynchronous operation that is being reinitialized may be
>>> obtained by
>>> calling AsyncEvent.getAsyncContext on the given event.
>>> ====
>>>
>>> - AsyncListener doesn't receive a onStartAsync event.
>>> We don't implement it.
>>>
>>> At AsyncListenerWrapper fireOnStartAsync is missing
>>> As AsyncContext.startAsync method doesn't emit an event
>>>
>>> - ContextClassLoader at AsyncContext.dispatch is not set!
>>> I can't see that we correcly set the application ContextClassLoader
>>> before
>>> we dispatch the Request. What must we do at a CrossContext dispatch?
>>>
>>> - After first complete async cycle, an AsyncListner receive
>>> onStartAsync as code start next async cycle?
>>>
>>> Why doesn't the Servlet 3.0 TCK check basic AsyncContext  
>>> functionalities?
>>> It seams that we better start a open test suite implementation at  
>>> the
>>> new Servlet 3.0 API.
>>>
>>> Regards,
>>> Peter
>>
>>
>> ---------------------------------------------------------------------
>> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
>> For additional commands, e-mail: dev-help@tomcat.apache.org
>>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Mime
View raw message