You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Peter Roßbach <pr...@objektpark.de> on 2010/06/27 13:11:40 UTC
Proposal: Some todo's at AsyncContext detected...
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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Konstantin Kolinko <kn...@gmail.com>.
2010/6/30 Marc Guillemot <mg...@yahoo.fr>:
> Hi,
>
> What you write means that the freshly release Tomcat 7 doesn't fully
> implement Servlet API 3.0, isn't it?
>
1. It was released as Beta
2. It passes TCK
Best regards,
Konstantin Kolinko
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Mark Thomas <ma...@apache.org>.
On 30/06/2010 21:23, Peter Roßbach wrote:
> 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?
I don't know - I haven't run the TCK with your patch applied. Just the
fact the the findings led to the patch which in turn caused TCKs
failures suggests at least something isn't quite right. It could also be
the analysis is OK but the patch has issues or that there is a problem
with the TCK.
I wouldn't be at all surprised to find part of the spec not covered by
the TCK. I would be surprised to find a bug in the TCK. My own
experiences with potential TCK bugs is that the TCK has always been correct.
Regardless, until the TCK is proved wrong Jean-Frederic's veto is valid
and your patch needs to be reverted.
Mark
>
> 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Peter Roßbach <pr...@objektpark.de>.
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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Mark Thomas <ma...@apache.org>.
On 01/07/2010 09:28, Marc Guillemot wrote:
> For me there is a point that is particularly interesting in Peter's email:
> > It seams that we better start a open test suite implementation at the
> > new Servlet 3.0 API.
>
> Tomcat's test suite is quite minimal even if I recognize that promising
> progress has been made in the last months (starting from a desolate
> situation). TCK is opaque and not complete. What about an open source
> Servlet 3.0 compatibility test suite. Other projects would surely be
> interested too.
Not an itch I feel the need to scratch. I'm more interested in extending
Tomcat's unit tests. Of course, that doesn't stop anyone else working on
such a test suite if they feel so inclined.
Mark
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Rainer Jung <ra...@kippdata.de>.
On 04.07.2010 22:11, Peter Roßbach wrote:
> Hmm,
>
> Spec says:
>
> public boolean isAsyncStarted() - Returns true if async processing
> has started on this request, and false otherwise. If this request has been
> dispatched using one of the AsyncContext.dispatch methods since it was
> put in asynchronous mode, or a call to AsynContext.complete is made, this
> method returns false.
>
> ===
> But the example with Bug 49528 don't call directly dispatch, it calls
> AsyncContext.start().
> At Tomcat we start doesn't start a Container Thread. We internally call
> a doInternalDispatch at StandardWrapperValve
> with the callee thread.
> ...
> } else {
> if (request.isAsyncDispatching()) {
> //TODO SERVLET3 - async
> ((AsyncContextImpl)request.getAsyncContext()).doInternalDispatch();
> ...
>
> I revert the fix, and correct the testcase.
I wasn't commenting a specific BZ issue or your test case. I was
commenting the TCK failure JFC noticed. It simply checks the cited
requirement on isAsyncStarted() and fails after your change that starts
the AsyncTimeout thread. Without your change it works.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Peter Roßbach <pr...@objektpark.de>.
Hmm,
Spec says:
public boolean isAsyncStarted() - Returns true if async processing
has started on this request, and false otherwise. If this request has
been
dispatched using one of the AsyncContext.dispatch methods since it was
put in asynchronous mode, or a call to AsynContext.complete is made,
this
method returns false.
===
But the example with Bug 49528 don't call directly dispatch, it calls
AsyncContext.start().
At Tomcat we start doesn't start a Container Thread. We internally
call a doInternalDispatch at StandardWrapperValve
with the callee thread.
...
} else {
if (request.isAsyncDispatching()) {
//TODO SERVLET3 - async
((AsyncContextImpl)request.getAsyncContext()).doInternalDispatch();
...
I revert the fix, and correct the testcase.
Peter
Am 04.07.2010 um 21:23 schrieb Rainer Jung:
> On 01.07.2010 18:42, Rainer Jung wrote:
>> On 01.07.2010 17:20, Marc Guillemot wrote:
>>> jean-frederic clere wrote:
>>>> ...
>>>> I run a daily test and just looked to what breaks it and complain.
>>>>
>>>> Now I am looking to Peter's application to find what is broken.
>>>
>>> so you're the continuous integration server ;-)
>>>
>>> Can you publish break information or is it forbidden by the TCK
>>> license/agreement?
>>
>> Exact details are forbidden, but I'm sure there'll be some
>> information
>
> The symptom is: it breaks
>
> "If this request has been dispatched using one of the
> AsyncContext.dispatch methods since it was put in asynchronous mode,
> this method returns false."
>
> which is part of the spec description of "public boolean
> isAsyncStarted()".
>
> Regards,
>
> Rainer
>
>
> ---------------------------------------------------------------------
> 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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Rainer Jung <ra...@kippdata.de>.
On 01.07.2010 18:42, Rainer Jung wrote:
> On 01.07.2010 17:20, Marc Guillemot wrote:
>> jean-frederic clere wrote:
>>> ...
>>> I run a daily test and just looked to what breaks it and complain.
>>>
>>> Now I am looking to Peter's application to find what is broken.
>>
>> so you're the continuous integration server ;-)
>>
>> Can you publish break information or is it forbidden by the TCK
>> license/agreement?
>
> Exact details are forbidden, but I'm sure there'll be some information
The symptom is: it breaks
"If this request has been dispatched using one of the
AsyncContext.dispatch methods since it was put in asynchronous mode,
this method returns false."
which is part of the spec description of "public boolean isAsyncStarted()".
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Peter Roßbach <pr...@objektpark.de>.
I am very sure the NIO and APR Connector affected, but I don't have
find a fix.
Peter
Am 02.07.2010 um 19:48 schrieb Mark Thomas:
> On 02/07/2010 15:15, jean-frederic clere wrote:
>> On 07/02/2010 03:08 PM, Marc Guillemot wrote:
>>> Rainer Jung wrote:
>>>> ...
>>>> Exact details are forbidden, but I'm sure there'll be some
>>>> information
>>>> as soon as someone had time to dig into it.
>>>
>>> this was exactly what I feared. This is an additional argument for
>>> an
>>> open source test suite.
>>
>> I think that patch is wrong... According to my tests the thread is
>> created but the timeout doesn't happen... I need more time to come
>> with
>> another patch.
>
> Some random comments having glanced very briefly at this.
>
> I'm tempted to revert the whole of r958362 but since it might only
> need a tweak to work correctly I'll leave it for now. Note that if
> we get to the next 7.0.x release and it still breaks the TCK it will
> have to be reverted.
>
> Why only the BIO connector? Are NIO and APR/native not affected?
>
> The threads are not daemon threads so Tomcat no longer shuts down
> correctly - I'll fix this in a sec.
>
> Mark
>
>>
>> Cheers
>>
>> Jean-Frederic
>>
>>>
>>> Cheers,
>>> Marc.
>>
>>
>> ---------------------------------------------------------------------
>> 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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Mark Thomas <ma...@apache.org>.
On 02/07/2010 15:15, jean-frederic clere wrote:
> On 07/02/2010 03:08 PM, Marc Guillemot wrote:
>> Rainer Jung wrote:
>>> ...
>>> Exact details are forbidden, but I'm sure there'll be some information
>>> as soon as someone had time to dig into it.
>>
>> this was exactly what I feared. This is an additional argument for an
>> open source test suite.
>
> I think that patch is wrong... According to my tests the thread is
> created but the timeout doesn't happen... I need more time to come with
> another patch.
Some random comments having glanced very briefly at this.
I'm tempted to revert the whole of r958362 but since it might only need
a tweak to work correctly I'll leave it for now. Note that if we get to
the next 7.0.x release and it still breaks the TCK it will have to be
reverted.
Why only the BIO connector? Are NIO and APR/native not affected?
The threads are not daemon threads so Tomcat no longer shuts down
correctly - I'll fix this in a sec.
Mark
>
> Cheers
>
> Jean-Frederic
>
>>
>> Cheers,
>> Marc.
>
>
> ---------------------------------------------------------------------
> 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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Peter Roßbach <pr...@objektpark.de>.
Hi Jean Frederic,
I also detect this strange behaviour. Then I fix it at JioEndpoint.
I check the async timeout feature today and it also works at
NioEndpoint.
Can I check the APR Connector with a normal TomcatTestCase?
Peter
Am 02.07.2010 um 16:49 schrieb jean-frederic clere:
> On 07/02/2010 04:11 PM, Peter Roßbach wrote:
>> Hi Jean-Frederic,
>>
>> had you check my test case TestAsyncListener?
>>
>> Can you checkin or send me your test case.
>
> Normally the setTimeout will call the connector
> (ACTION_ASYNC_SETTIMEOUT). Which I think it does but the timeout
> doesn't
> occur, at least with the code I am testing.
> ...
> Cheers
>
> Jean-Frederic
>
>>
>> Peter
>>
>> Am 02.07.2010 um 15:15 schrieb jean-frederic clere:
>>
>>> On 07/02/2010 03:08 PM, Marc Guillemot wrote:
>>>> Rainer Jung wrote:
>>>>> ...
>>>>> Exact details are forbidden, but I'm sure there'll be some
>>>>> information
>>>>> as soon as someone had time to dig into it.
>>>>
>>>> this was exactly what I feared. This is an additional argument
>>>> for an
>>>> open source test suite.
>>>
>>> I think that patch is wrong... According to my tests the thread is
>>> created but the timeout doesn't happen... I need more time to come
>>> with
>>> another patch.
>>>
>>> Cheers
>>>
>>> Jean-Frederic
>>>
>>>>
>>>> Cheers,
>>>> Marc.
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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
>>
>>
>
> <TestAsyncServlet.java>
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Peter Roßbach <pr...@objektpark.de>.
Hi Jean-Frederic,
had you check my test case TestAsyncListener?
Can you checkin or send me your test case.
Peter
Am 02.07.2010 um 15:15 schrieb jean-frederic clere:
> On 07/02/2010 03:08 PM, Marc Guillemot wrote:
>> Rainer Jung wrote:
>>> ...
>>> Exact details are forbidden, but I'm sure there'll be some
>>> information
>>> as soon as someone had time to dig into it.
>>
>> this was exactly what I feared. This is an additional argument for an
>> open source test suite.
>
> I think that patch is wrong... According to my tests the thread is
> created but the timeout doesn't happen... I need more time to come
> with
> another patch.
>
> Cheers
>
> Jean-Frederic
>
>>
>> Cheers,
>> Marc.
>
>
> ---------------------------------------------------------------------
> 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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by jean-frederic clere <jf...@gmail.com>.
On 07/02/2010 03:08 PM, Marc Guillemot wrote:
> Rainer Jung wrote:
>> ...
>> Exact details are forbidden, but I'm sure there'll be some information
>> as soon as someone had time to dig into it.
>
> this was exactly what I feared. This is an additional argument for an
> open source test suite.
I think that patch is wrong... According to my tests the thread is
created but the timeout doesn't happen... I need more time to come with
another patch.
Cheers
Jean-Frederic
>
> Cheers,
> Marc.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Marc Guillemot <mg...@yahoo.fr>.
Rainer Jung wrote:
> ...
> Exact details are forbidden, but I'm sure there'll be some information
> as soon as someone had time to dig into it.
this was exactly what I feared. This is an additional argument for an
open source test suite.
Cheers,
Marc.
--
Blog: http://mguillem.wordpress.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Rainer Jung <ra...@kippdata.de>.
On 01.07.2010 17:20, Marc Guillemot wrote:
> jean-frederic clere wrote:
>> ...
>> I run a daily test and just looked to what breaks it and complain.
>>
>> Now I am looking to Peter's application to find what is broken.
>
> so you're the continuous integration server ;-)
>
> Can you publish break information or is it forbidden by the TCK
> license/agreement?
Exact details are forbidden, but I'm sure there'll be some information
as soon as someone had time to dig into it.
Regards,
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Marc Guillemot <mg...@yahoo.fr>.
jean-frederic clere wrote:
> ...
> I run a daily test and just looked to what breaks it and complain.
>
> Now I am looking to Peter's application to find what is broken.
so you're the continuous integration server ;-)
Can you publish break information or is it forbidden by the TCK
license/agreement?
Cheers,
Marc.
--
Blog: http://mguillem.wordpress.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by jean-frederic clere <jf...@gmail.com>.
On 07/01/2010 09:28 AM, Marc Guillemot wrote:
> Hi Mark,
>
> nice to see some activity on this thread.
>
>> 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.
>
> I do follow it and therefore I'm interested to read more about the
> problems. Jean-Frederic wasn't really generous with comments :-( and
> Peter's email addresses many points.
I run a daily test and just looked to what breaks it and complain.
Now I am looking to Peter's application to find what is broken.
Cheers
Jean-Frederic
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Marc Guillemot <mg...@yahoo.fr>.
Hi Mark,
nice to see some activity on this thread.
> 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.
I do follow it and therefore I'm interested to read more about the
problems. Jean-Frederic wasn't really generous with comments :-( and
Peter's email addresses many points.
For me there is a point that is particularly interesting in Peter's email:
> It seams that we better start a open test suite implementation at the
> new Servlet 3.0 API.
Tomcat's test suite is quite minimal even if I recognize that promising
progress has been made in the last months (starting from a desolate
situation). TCK is opaque and not complete. What about an open source
Servlet 3.0 compatibility test suite. Other projects would surely be
interested too.
Cheers,
Marc.
--
Blog: http://mguillem.wordpress.com
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Mark Thomas <ma...@apache.org>.
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.
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
Re: Proposal: Some todo's at AsyncContext detected...
Posted by Marc Guillemot <mg...@yahoo.fr>.
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 :-(
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