You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Violeta Georgieva <vi...@apache.org> on 2017/05/02 08:52:34 UTC

asyncError() is not valid while in Async state DISPATCHING

Hi,

I have a question about Async state DISPATCHING.

I have the following scenario
- Application invokes startAsync() and then continues processing in a
separate non-container thread A.
- At some point thread A makes dispatch() -> Async state is changed to
DISPATCHING
- At the same time a socket error occurs and thread B tries to change  the
Async state to ERROR, as the state was already changed to DISPATCHING, the
exception below occurs and the state cannot be changed to ERROR.

Isn't that a problem as onError event will not be called?
Why don't we treat DISPATCHING as DISPATCHED in asyncError?
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388

Thanks,
Violeta

org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception while
processing an asynchronous request

 java.lang.IllegalStateException: Calling [asyncError()] is not valid for a
request with Async state [DISPATCHING]

at
org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)

at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)

at org.apache.coyote.Request.action(Request.java:390)

at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)

at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)

at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)

at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)

at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)

at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)

at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)

at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)

at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)

at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)

at java.lang.Thread.run(Thread.java:745)

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 26/06/17 14:58, Violeta Georgieva wrote:
> 2017-06-26 16:35 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>
>> On 26/06/17 14:18, Violeta Georgieva wrote:
>>> 2017-06-26 16:13 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>>
>>>> On 23/06/17 14:47, Violeta Georgieva wrote:
>>>>> 2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>>>> On 13/06/17 11:05, Violeta Georgieva wrote:
>>>>>>> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>>>>
>>>> <snip/>
>>>>
>>>>>>>> java.lang.NullPointerException: null
>>>>>>
>>>>>> Appears to be the same cause as above, but triggered at a different
>>> point.
>>>>>>
>>>>>
>>>>> What about at least not throw NPE?
>>>>
>>>> Which object is null? Since the stack trace is from a dev version, I'm
>>>> not exactly sure where the null is in this case.
>>>
>>> The first thread makes a dispatch and clears the request and response
>>>
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L209
>>>
>>> the second thread handles the error and tries to set a status code
>>>
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L411
>>
>> This should be fixed in trunk. If this is sufficient, we can back-port.
>>
> 
> I just tested it. I did not see any NPEs. Please back-port it.

Done.

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
2017-06-26 16:35 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 26/06/17 14:18, Violeta Georgieva wrote:
> > 2017-06-26 16:13 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>
> >> On 23/06/17 14:47, Violeta Georgieva wrote:
> >>> 2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>>> On 13/06/17 11:05, Violeta Georgieva wrote:
> >>>>> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
> >>
> >> <snip/>
> >>
> >>>>>> java.lang.NullPointerException: null
> >>>>
> >>>> Appears to be the same cause as above, but triggered at a different
> > point.
> >>>>
> >>>
> >>> What about at least not throw NPE?
> >>
> >> Which object is null? Since the stack trace is from a dev version, I'm
> >> not exactly sure where the null is in this case.
> >
> > The first thread makes a dispatch and clears the request and response
> >
https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L209
> >
> > the second thread handles the error and tries to set a status code
> >
https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L411
>
> This should be fixed in trunk. If this is sufficient, we can back-port.
>

I just tested it. I did not see any NPEs. Please back-port it.

Thanks,
Violeta

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

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 26/06/17 14:18, Violeta Georgieva wrote:
> 2017-06-26 16:13 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>
>> On 23/06/17 14:47, Violeta Georgieva wrote:
>>> 2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>> On 13/06/17 11:05, Violeta Georgieva wrote:
>>>>> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>>
>> <snip/>
>>
>>>>>> java.lang.NullPointerException: null
>>>>
>>>> Appears to be the same cause as above, but triggered at a different
> point.
>>>>
>>>
>>> What about at least not throw NPE?
>>
>> Which object is null? Since the stack trace is from a dev version, I'm
>> not exactly sure where the null is in this case.
> 
> The first thread makes a dispatch and clears the request and response
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L209
> 
> the second thread handles the error and tries to set a status code
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L411

This should be fixed in trunk. If this is sufficient, we can back-port.

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
2017-06-26 16:13 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 23/06/17 14:47, Violeta Georgieva wrote:
> > 2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >> On 13/06/17 11:05, Violeta Georgieva wrote:
> >>> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>
> <snip/>
>
> >>>> java.lang.NullPointerException: null
> >>
> >> Appears to be the same cause as above, but triggered at a different
point.
> >>
> >
> > What about at least not throw NPE?
>
> Which object is null? Since the stack trace is from a dev version, I'm
> not exactly sure where the null is in this case.

The first thread makes a dispatch and clears the request and response
https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L209

the second thread handles the error and tries to set a status code
https://github.com/apache/tomcat/blob/trunk/java/org/apache/catalina/core/AsyncContextImpl.java#L411


> >>>> at
> >>>
> >
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
> >>> ~[tomcat-embed-core.jar!/:8.5.16-dev]
> >>
> >> <snip/>
> >>
> >> This is consistent with what I'd expect. In both of the above cases any
> >> AsyncListener.onError() methods should execute but if the app continues
> >> to do things on the non-container thread then there possibility of
> >> exceptions remains.
> >>
> >
> > Ok I understand.
> >
> > Can we back port the change to Tomcat 8.5 so that it is guaranteed that
the
> > AsyncListeners will be invoked on error.
>
> Next on my TODO lis.
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 23/06/17 14:47, Violeta Georgieva wrote:
> 2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
>> On 13/06/17 11:05, Violeta Georgieva wrote:
>>> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:

<snip/>

>>>> java.lang.NullPointerException: null
>>
>> Appears to be the same cause as above, but triggered at a different point.
>>
> 
> What about at least not throw NPE?

Which object is null? Since the stack trace is from a dev version, I'm
not exactly sure where the null is in this case.

>>>> at
>>>
> org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
>>> ~[tomcat-embed-core.jar!/:8.5.16-dev]
>>
>> <snip/>
>>
>> This is consistent with what I'd expect. In both of the above cases any
>> AsyncListener.onError() methods should execute but if the app continues
>> to do things on the non-container thread then there possibility of
>> exceptions remains.
>>
> 
> Ok I understand.
> 
> Can we back port the change to Tomcat 8.5 so that it is guaranteed that the
> AsyncListeners will be invoked on error.

Next on my TODO lis.

Mark


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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi,

2017-06-13 15:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 13/06/17 11:05, Violeta Georgieva wrote:
> > 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
> >>
> >> Hi,
> >>
> >>
> >> 2017-06-12 21:43 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>>
> >>> On 09/06/17 16:26, Violeta Georgieva wrote:
> >>>> 2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>>
> >>> <snip/>
> >>>
> >>>>> I've spent some time working through the various possible
> > combinations
> >>>>> of events and have concluded it is impossible to completely fix this
> >>>>> without imposing additional requirements on applications that the
> >>>>> specification doesn't mention.
> >>>>>
> >>>>> However, I believe that we can do better than the current
> >>>>> implementation. What I have on mind would:
> >>>>>
> >>>>> - always trigger AsyncListener.onError() for all listeners
> >>>>> - generally, process the complete() dispatch() call from the
> >>>>>   AsyncListener rather than any from the non-container thread
> >>>>> - generally, throw an ISE if complete() or dispatch() is called
> >>>>>   from the non-container thread after that thread experiences an I/O
> >>>>>    error
> >>>>> - leave a small timing window where it was possible that the
> > complete()
> >>>>>   or dispatch() from the non-container thread would be used rather
> > than
> >>>>>   from the AsyncListener. In that case the AsyncListener would see
> > the
> >>>>>   ISE but any remaining AsyncListener instances would still be
called
> >>>>>
> >>>>> I don't see a way of doing better than this without spec changes /
> >>>>> clarifications.
> >>>>>
> >>>>> WDYT?
> >>>>
> >>>> +1
> >>>> I'm able to test the new behavior with my real web app.
> >>>
> >>> Excellent. I've committed my proposed fix. The async unit tests pass
> >>> which is generally a good sign. If this works better with your real
web
> >>> application then we can look to back-port this.
> >>
> >> I'm seeing now the following exceptions:
>
> In the same run or different runs?
>
> > I back ported the fix to my local 8.5 branch in order to be able to
test it
> > ...
> >
> >>
> >> java.lang.IllegalStateException: Calling [asyncComplete()] is not valid
> > for a request with Async state [MUST_DISPATCH]
>
> It appears the application called dispatch() from a non-container thread
> once the error state had been entered. We could block that but there is
> a risk it will break valid use cases. Fundamentally, I don't believe the
> app should be doing this.

Ok I understand.

>
> >> at
> >
org.apache.coyote.AsyncStateMachine.doComplete(AsyncStateMachine.java:317)
> > ~[tomcat-embed-core.jar!/:8.5.16-dev]
>
> <snip/>
>
> >> ==================
> >>
> >> java.lang.NullPointerException: null
>
> Appears to be the same cause as above, but triggered at a different point.
>

What about at least not throw NPE?

> >> at
> >
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
> > ~[tomcat-embed-core.jar!/:8.5.16-dev]
>
> <snip/>
>
> This is consistent with what I'd expect. In both of the above cases any
> AsyncListener.onError() methods should execute but if the app continues
> to do things on the non-container thread then there possibility of
> exceptions remains.
>

Ok I understand.

Can we back port the change to Tomcat 8.5 so that it is guaranteed that the
AsyncListeners will be invoked on error.

Thanks,
Violeta

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

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 13/06/17 11:05, Violeta Georgieva wrote:
> 2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>>
>> Hi,
>>
>>
>> 2017-06-12 21:43 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>
>>> On 09/06/17 16:26, Violeta Georgieva wrote:
>>>> 2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>
>>> <snip/>
>>>
>>>>> I've spent some time working through the various possible
> combinations
>>>>> of events and have concluded it is impossible to completely fix this
>>>>> without imposing additional requirements on applications that the
>>>>> specification doesn't mention.
>>>>>
>>>>> However, I believe that we can do better than the current
>>>>> implementation. What I have on mind would:
>>>>>
>>>>> - always trigger AsyncListener.onError() for all listeners
>>>>> - generally, process the complete() dispatch() call from the
>>>>>   AsyncListener rather than any from the non-container thread
>>>>> - generally, throw an ISE if complete() or dispatch() is called
>>>>>   from the non-container thread after that thread experiences an I/O
>>>>>    error
>>>>> - leave a small timing window where it was possible that the
> complete()
>>>>>   or dispatch() from the non-container thread would be used rather
> than
>>>>>   from the AsyncListener. In that case the AsyncListener would see
> the
>>>>>   ISE but any remaining AsyncListener instances would still be called
>>>>>
>>>>> I don't see a way of doing better than this without spec changes /
>>>>> clarifications.
>>>>>
>>>>> WDYT?
>>>>
>>>> +1
>>>> I'm able to test the new behavior with my real web app.
>>>
>>> Excellent. I've committed my proposed fix. The async unit tests pass
>>> which is generally a good sign. If this works better with your real web
>>> application then we can look to back-port this.
>>
>> I'm seeing now the following exceptions:

In the same run or different runs?

> I back ported the fix to my local 8.5 branch in order to be able to test it
> ...
> 
>>
>> java.lang.IllegalStateException: Calling [asyncComplete()] is not valid
> for a request with Async state [MUST_DISPATCH]

It appears the application called dispatch() from a non-container thread
once the error state had been entered. We could block that but there is
a risk it will break valid use cases. Fundamentally, I don't believe the
app should be doing this.

>> at
> org.apache.coyote.AsyncStateMachine.doComplete(AsyncStateMachine.java:317)
> ~[tomcat-embed-core.jar!/:8.5.16-dev]

<snip/>

>> ==================
>>
>> java.lang.NullPointerException: null

Appears to be the same cause as above, but triggered at a different point.

>> at
> org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
> ~[tomcat-embed-core.jar!/:8.5.16-dev]

<snip/>

This is consistent with what I'd expect. In both of the above cases any
AsyncListener.onError() methods should execute but if the app continues
to do things on the non-container thread then there possibility of
exceptions remains.

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
2017-06-13 13:04 GMT+03:00 Violeta Georgieva <vi...@apache.org>:
>
> Hi,
>
>
> 2017-06-12 21:43 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >
> > On 09/06/17 16:26, Violeta Georgieva wrote:
> > > 2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >
> > <snip/>
> >
> > >> I've spent some time working through the various possible
combinations
> > >> of events and have concluded it is impossible to completely fix this
> > >> without imposing additional requirements on applications that the
> > >> specification doesn't mention.
> > >>
> > >> However, I believe that we can do better than the current
> > >> implementation. What I have on mind would:
> > >>
> > >> - always trigger AsyncListener.onError() for all listeners
> > >> - generally, process the complete() dispatch() call from the
> > >>   AsyncListener rather than any from the non-container thread
> > >> - generally, throw an ISE if complete() or dispatch() is called
> > >>   from the non-container thread after that thread experiences an I/O
> > >>    error
> > >> - leave a small timing window where it was possible that the
complete()
> > >>   or dispatch() from the non-container thread would be used rather
than
> > >>   from the AsyncListener. In that case the AsyncListener would see
the
> > >>   ISE but any remaining AsyncListener instances would still be called
> > >>
> > >> I don't see a way of doing better than this without spec changes /
> > >> clarifications.
> > >>
> > >> WDYT?
> > >
> > > +1
> > > I'm able to test the new behavior with my real web app.
> >
> > Excellent. I've committed my proposed fix. The async unit tests pass
> > which is generally a good sign. If this works better with your real web
> > application then we can look to back-port this.
>
> I'm seeing now the following exceptions:

I back ported the fix to my local 8.5 branch in order to be able to test it
...

>
> java.lang.IllegalStateException: Calling [asyncComplete()] is not valid
for a request with Async state [MUST_DISPATCH]
> at
org.apache.coyote.AsyncStateMachine.doComplete(AsyncStateMachine.java:317)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AsyncStateMachine.asyncComplete(AsyncStateMachine.java:301)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:377)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at org.apache.coyote.Request.action(Request.java:424)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.catalina.core.AsyncContextImpl.complete(AsyncContextImpl.java:96)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:427)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:176)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:861)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1455)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
[na:1.8.0_102]
> at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
[na:1.8.0_102]
> at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at java.lang.Thread.run(Thread.java:745) [na:1.8.0_102]
>
>
> ==================
>
> java.lang.NullPointerException: null
> at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:176)
~[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:861)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1455)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
[na:1.8.0_102]
> at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
[na:1.8.0_102]
> at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
[tomcat-embed-core.jar!/:8.5.16-dev]
> at java.lang.Thread.run(Thread.java:745) [na:1.8.0_102]
>
>
> Thanks,
> Violeta
>
>
> >
> > Mark
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: dev-help@tomcat.apache.org
> >

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi,

2017-06-12 21:43 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 09/06/17 16:26, Violeta Georgieva wrote:
> > 2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> <snip/>
>
> >> I've spent some time working through the various possible combinations
> >> of events and have concluded it is impossible to completely fix this
> >> without imposing additional requirements on applications that the
> >> specification doesn't mention.
> >>
> >> However, I believe that we can do better than the current
> >> implementation. What I have on mind would:
> >>
> >> - always trigger AsyncListener.onError() for all listeners
> >> - generally, process the complete() dispatch() call from the
> >>   AsyncListener rather than any from the non-container thread
> >> - generally, throw an ISE if complete() or dispatch() is called
> >>   from the non-container thread after that thread experiences an I/O
> >>    error
> >> - leave a small timing window where it was possible that the complete()
> >>   or dispatch() from the non-container thread would be used rather than
> >>   from the AsyncListener. In that case the AsyncListener would see the
> >>   ISE but any remaining AsyncListener instances would still be called
> >>
> >> I don't see a way of doing better than this without spec changes /
> >> clarifications.
> >>
> >> WDYT?
> >
> > +1
> > I'm able to test the new behavior with my real web app.
>
> Excellent. I've committed my proposed fix. The async unit tests pass
> which is generally a good sign. If this works better with your real web
> application then we can look to back-port this.

I'm seeing now the following exceptions:

java.lang.IllegalStateException: Calling [asyncComplete()] is not valid for
a request with Async state [MUST_DISPATCH]
at
org.apache.coyote.AsyncStateMachine.doComplete(AsyncStateMachine.java:317)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.coyote.AsyncStateMachine.asyncComplete(AsyncStateMachine.java:301)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:377)
[tomcat-embed-core.jar!/:8.5.16-dev]
at org.apache.coyote.Request.action(Request.java:424)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.catalina.core.AsyncContextImpl.complete(AsyncContextImpl.java:96)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:427)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:176)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:861)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1455)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
[na:1.8.0_102]
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
[na:1.8.0_102]
at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
[tomcat-embed-core.jar!/:8.5.16-dev]
at java.lang.Thread.run(Thread.java:745) [na:1.8.0_102]


==================

java.lang.NullPointerException: null
at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:411)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:176)
~[tomcat-embed-core.jar!/:8.5.16-dev]
at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:861)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1455)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
[tomcat-embed-core.jar!/:8.5.16-dev]
at
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
[na:1.8.0_102]
at
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
[na:1.8.0_102]
at
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
[tomcat-embed-core.jar!/:8.5.16-dev]
at java.lang.Thread.run(Thread.java:745) [na:1.8.0_102]


Thanks,
Violeta


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

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 09/06/17 16:26, Violeta Georgieva wrote:
> 2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:

<snip/>

>> I've spent some time working through the various possible combinations
>> of events and have concluded it is impossible to completely fix this
>> without imposing additional requirements on applications that the
>> specification doesn't mention.
>>
>> However, I believe that we can do better than the current
>> implementation. What I have on mind would:
>>
>> - always trigger AsyncListener.onError() for all listeners
>> - generally, process the complete() dispatch() call from the
>>   AsyncListener rather than any from the non-container thread
>> - generally, throw an ISE if complete() or dispatch() is called
>>   from the non-container thread after that thread experiences an I/O
>>    error
>> - leave a small timing window where it was possible that the complete()
>>   or dispatch() from the non-container thread would be used rather than
>>   from the AsyncListener. In that case the AsyncListener would see the
>>   ISE but any remaining AsyncListener instances would still be called
>>
>> I don't see a way of doing better than this without spec changes /
>> clarifications.
>>
>> WDYT?
> 
> +1
> I'm able to test the new behavior with my real web app.

Excellent. I've committed my proposed fix. The async unit tests pass
which is generally a good sign. If this works better with your real web
application then we can look to back-port this.

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi,

2017-06-09 17:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 09/06/17 12:41, Mark Thomas wrote:
> > On 05/06/17 09:38, Violeta Georgieva wrote:
> >
> > <snip/>
> >
> >> Try with this test
> >>
https://github.com/violetagg/tomcat/commit/53564d4e73496cb3badcaaab81a1314cf2ed750d
> >
> > Thanks. That clarifies things a lot.
> >
> > It boils down to what should happen when an I/O error occurs reading
> > from the request or writing to the response on an non-container thread.
>
> <snip/>
>
> > I'm currently on the fence between calling this an application error
> > (the non-container thread should not call dispatch after an I/O error)
> > and trying to figure out if there is a way we could handle this.
> >
> > I'm currently looking at the code to see what the options might be. Did
> > you have a patch in mind? We also need to be careful that we don't end
> > up with multiple container threads trying to handle the same async
request.
>
> I've spent some time working through the various possible combinations
> of events and have concluded it is impossible to completely fix this
> without imposing additional requirements on applications that the
> specification doesn't mention.
>
> However, I believe that we can do better than the current
> implementation. What I have on mind would:
>
> - always trigger AsyncListener.onError() for all listeners
> - generally, process the complete() dispatch() call from the
>   AsyncListener rather than any from the non-container thread
> - generally, throw an ISE if complete() or dispatch() is called
>   from the non-container thread after that thread experiences an I/O
>    error
> - leave a small timing window where it was possible that the complete()
>   or dispatch() from the non-container thread would be used rather than
>   from the AsyncListener. In that case the AsyncListener would see the
>   ISE but any remaining AsyncListener instances would still be called
>
> I don't see a way of doing better than this without spec changes /
> clarifications.
>
> WDYT?

+1
I'm able to test the new behavior with my real web app.

Thanks, Violeta

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

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 09/06/17 12:41, Mark Thomas wrote:
> On 05/06/17 09:38, Violeta Georgieva wrote:
> 
> <snip/>
> 
>> Try with this test
>> https://github.com/violetagg/tomcat/commit/53564d4e73496cb3badcaaab81a1314cf2ed750d
> 
> Thanks. That clarifies things a lot.
> 
> It boils down to what should happen when an I/O error occurs reading
> from the request or writing to the response on an non-container thread.

<snip/>

> I'm currently on the fence between calling this an application error
> (the non-container thread should not call dispatch after an I/O error)
> and trying to figure out if there is a way we could handle this.
> 
> I'm currently looking at the code to see what the options might be. Did
> you have a patch in mind? We also need to be careful that we don't end
> up with multiple container threads trying to handle the same async request.

I've spent some time working through the various possible combinations
of events and have concluded it is impossible to completely fix this
without imposing additional requirements on applications that the
specification doesn't mention.

However, I believe that we can do better than the current
implementation. What I have on mind would:

- always trigger AsyncListener.onError() for all listeners
- generally, process the complete() dispatch() call from the
  AsyncListener rather than any from the non-container thread
- generally, throw an ISE if complete() or dispatch() is called
  from the non-container thread after that thread experiences an I/O
   error
- leave a small timing window where it was possible that the complete()
  or dispatch() from the non-container thread would be used rather than
  from the AsyncListener. In that case the AsyncListener would see the
  ISE but any remaining AsyncListener instances would still be called

I don't see a way of doing better than this without spec changes /
clarifications.

WDYT?

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 05/06/17 09:38, Violeta Georgieva wrote:

<snip/>

> Try with this test
> https://github.com/violetagg/tomcat/commit/53564d4e73496cb3badcaaab81a1314cf2ed750d

Thanks. That clarifies things a lot.

It boils down to what should happen when an I/O error occurs reading
from the request or writing to the response on an non-container thread.

The Servlet spec is silent on the issue. There are some requirements
starting with the last bullet at the bottom of page 2-16 of the final
3.1 specification but they relate to errors during the call to dispatch.

Tomcat's current behaviour is to:
- rethrow the exception so it is visible to the non-container thread
- expect the non-container thread to exit without further I/O or calls
  to the async API
- invoke the async error handling mechanism that will in turn call
  complete() or dispatch()

Also, the reason this is the way it is is to ensure that when there is
an I/O error, the correct clean-up is performed by the container (e.g.
correctly tracking the current connection count).

The problem in the test case occurs when the non-container thread
catches the error and calls dispatch before the invocation of the async
error handling has taken effect.

The lack of specification in this area is a real problem. If we leave it
up to the non-container thread to do the clean-up, many won't. If we
always have the container do the clean-up, it is likely to conflict with
non-container threads that try to do the same.

I'm currently on the fence between calling this an application error
(the non-container thread should not call dispatch after an I/O error)
and trying to figure out if there is a way we could handle this.

I'm currently looking at the code to see what the options might be. Did
you have a patch in mind? We also need to be careful that we don't end
up with multiple container threads trying to handle the same async request.

Mark

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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi,

2017-06-02 23:44 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 02/06/2017 10:46, Violeta Georgieva wrote:
> > 2017-06-02 12:37 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>
> >> On 02/06/2017 10:23, Violeta Georgieva wrote:
> >>> Hi Mark,
> >>>
> >>> 2017-05-22 14:28 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>
> >> <snip/>
> >>
> >>>> I'm leaning towards thinking this is an application bug.
> >>>>
> >>>> If the application has two non-container threads, A and B, then, as
per
> >>>> section 2.3.3.4 of the Servlet spec, the application is responsible
for
> >>>> accessing the request from those threads in a thread-safe manner.
> >>>
> >>> Yep I agree with the above, but in this scenario we have one
> > non-container
> >>> thread A
> >>> and the other thread B is actually a container thread that tries to
> > send a
> >>> notification for an error.
> >>>
> >>> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
> > while
> >>> processing an asynchronous request
> >>>  java.lang.IllegalStateException: Calling [asyncError()] is not valid
> > for a
> >>> request with Async state [DISPATCHING]
> >>> at org.apache.coyote.AsyncStateMachine.asyncError(
> >>> AsyncStateMachine.java:398)
> >>> at
> > org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> >>> at org.apache.coyote.Request.action(Request.java:390)
> >>> at org.apache.catalina.core.AsyncContextImpl.setErrorState(
> >>> AsyncContextImpl.java:385)
> >>> at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(
> >>> CoyoteAdapter.java:175)
> >>> at
> > org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> >>> at org.apache.coyote.AbstractProcessorLight.process(
> >>> AbstractProcessorLight.java:53)
> >>>
> >>> What do you think?
> >>
> >> I think I still need more clarity on what is going on. Going back to
> >> your original e-mail, the problems start when:
> >>
> >> - At some point thread A makes dispatch() -> Async state is changed
> >>   to DISPATCHING
> >> - At the same time a socket error occurs
> >
> > Client closes the connection while the application is writing the
response.
>
> Sorry - still trying to clarify in my mind what is going on here. Is
> this right?
>
> Application does async write (that does not complete). Container will
> complete this later.
>
> Application calls dispatch.
>
> At the same time the application calls dispatch, the container tries to
> complete the async write and that fails because the client disconnected?
>
> Error handling then fails because of the dispatch?

Try with this test
https://github.com/violetagg/tomcat/commit/53564d4e73496cb3badcaaab81a1314cf2ed750d

In order to be able to reproduce this threading issue, run it with a
debugger and put break points on these two actions:
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AbstractProcessor.java#L382
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AbstractProcessor.java#L392

When the debugger stops you will have two threads - one will be on
ASYNC_DISPATCH the other on ASYNC_ERROR, choose first ASYNC_DISPATCH then
you will be able to see something like:

05-Jun-2017 11:35:53.508 INFO [Thread-4]
org.apache.coyote.AbstractProcessor.setErrorState An error occurred in
processing while on a non-container thread. The connection will be closed
immediately
 java.io.IOException: Broken pipe
at sun.nio.ch.FileDispatcherImpl.write0(Native Method)
at sun.nio.ch.SocketDispatcher.write(SocketDispatcher.java:47)
at sun.nio.ch.IOUtil.writeFromNativeBuffer(IOUtil.java:93)
at sun.nio.ch.IOUtil.write(IOUtil.java:65)
at sun.nio.ch.SocketChannelImpl.write(SocketChannelImpl.java:471)
at org.apache.tomcat.util.net.NioChannel.write(NioChannel.java:134)

....

05-Jun-2017 11:36:16.283 SEVERE [http-nio-127.0.0.1-auto-1-exec-3]
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception while
processing an asynchronous request
 java.lang.IllegalStateException: Calling [asyncError()] is not valid for a
request with Async state [DISPATCHING]
at
org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
at org.apache.coyote.Request.action(Request.java:405)
at
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:386)
at
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:176)
at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)

Regards,
Violeta

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

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 02/06/2017 10:46, Violeta Georgieva wrote:
> 2017-06-02 12:37 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>
>> On 02/06/2017 10:23, Violeta Georgieva wrote:
>>> Hi Mark,
>>>
>>> 2017-05-22 14:28 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>
>> <snip/>
>>
>>>> I'm leaning towards thinking this is an application bug.
>>>>
>>>> If the application has two non-container threads, A and B, then, as per
>>>> section 2.3.3.4 of the Servlet spec, the application is responsible for
>>>> accessing the request from those threads in a thread-safe manner.
>>>
>>> Yep I agree with the above, but in this scenario we have one
> non-container
>>> thread A
>>> and the other thread B is actually a container thread that tries to
> send a
>>> notification for an error.
>>>
>>> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
> while
>>> processing an asynchronous request
>>>  java.lang.IllegalStateException: Calling [asyncError()] is not valid
> for a
>>> request with Async state [DISPATCHING]
>>> at org.apache.coyote.AsyncStateMachine.asyncError(
>>> AsyncStateMachine.java:398)
>>> at
> org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
>>> at org.apache.coyote.Request.action(Request.java:390)
>>> at org.apache.catalina.core.AsyncContextImpl.setErrorState(
>>> AsyncContextImpl.java:385)
>>> at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(
>>> CoyoteAdapter.java:175)
>>> at
> org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
>>> at org.apache.coyote.AbstractProcessorLight.process(
>>> AbstractProcessorLight.java:53)
>>>
>>> What do you think?
>>
>> I think I still need more clarity on what is going on. Going back to
>> your original e-mail, the problems start when:
>>
>> - At some point thread A makes dispatch() -> Async state is changed
>>   to DISPATCHING
>> - At the same time a socket error occurs
> 
> Client closes the connection while the application is writing the response.

Sorry - still trying to clarify in my mind what is going on here. Is
this right?

Application does async write (that does not complete). Container will
complete this later.

Application calls dispatch.

At the same time the application calls dispatch, the container tries to
complete the async write and that fails because the client disconnected?

Error handling then fails because of the dispatch?

Mark


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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
2017-06-02 12:37 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 02/06/2017 10:23, Violeta Georgieva wrote:
> > Hi Mark,
> >
> > 2017-05-22 14:28 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> <snip/>
>
> >> I'm leaning towards thinking this is an application bug.
> >>
> >> If the application has two non-container threads, A and B, then, as per
> >> section 2.3.3.4 of the Servlet spec, the application is responsible for
> >> accessing the request from those threads in a thread-safe manner.
> >
> > Yep I agree with the above, but in this scenario we have one
non-container
> > thread A
> > and the other thread B is actually a container thread that tries to
send a
> > notification for an error.
> >
> > org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
while
> > processing an asynchronous request
> >  java.lang.IllegalStateException: Calling [asyncError()] is not valid
for a
> > request with Async state [DISPATCHING]
> > at org.apache.coyote.AsyncStateMachine.asyncError(
> > AsyncStateMachine.java:398)
> > at
org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> > at org.apache.coyote.Request.action(Request.java:390)
> > at org.apache.catalina.core.AsyncContextImpl.setErrorState(
> > AsyncContextImpl.java:385)
> > at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(
> > CoyoteAdapter.java:175)
> > at
org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> > at org.apache.coyote.AbstractProcessorLight.process(
> > AbstractProcessorLight.java:53)
> >
> > What do you think?
>
> I think I still need more clarity on what is going on. Going back to
> your original e-mail, the problems start when:
>
> - At some point thread A makes dispatch() -> Async state is changed
>   to DISPATCHING
> - At the same time a socket error occurs

Client closes the connection while the application is writing the response.

> The application triggers the dispatch - on its own that is OK. What
> triggers the socket error? That means something is performing a read or
> write. What triggers that read or write?
>
> Mark
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 02/06/2017 10:23, Violeta Georgieva wrote:
> Hi Mark,
> 
> 2017-05-22 14:28 GMT+03:00 Mark Thomas <ma...@apache.org>:

<snip/>

>> I'm leaning towards thinking this is an application bug.
>>
>> If the application has two non-container threads, A and B, then, as per
>> section 2.3.3.4 of the Servlet spec, the application is responsible for
>> accessing the request from those threads in a thread-safe manner.
> 
> Yep I agree with the above, but in this scenario we have one non-container
> thread A
> and the other thread B is actually a container thread that tries to send a
> notification for an error.
> 
> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception while
> processing an asynchronous request
>  java.lang.IllegalStateException: Calling [asyncError()] is not valid for a
> request with Async state [DISPATCHING]
> at org.apache.coyote.AsyncStateMachine.asyncError(
> AsyncStateMachine.java:398)
> at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> at org.apache.coyote.Request.action(Request.java:390)
> at org.apache.catalina.core.AsyncContextImpl.setErrorState(
> AsyncContextImpl.java:385)
> at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(
> CoyoteAdapter.java:175)
> at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> at org.apache.coyote.AbstractProcessorLight.process(
> AbstractProcessorLight.java:53)
> 
> What do you think?

I think I still need more clarity on what is going on. Going back to
your original e-mail, the problems start when:

- At some point thread A makes dispatch() -> Async state is changed
  to DISPATCHING
- At the same time a socket error occurs

The application triggers the dispatch - on its own that is OK. What
triggers the socket error? That means something is performing a read or
write. What triggers that read or write?

Mark


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


Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi Mark,

2017-05-22 14:28 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 10/05/17 20:50, Violeta Georgieva wrote:
> > Hi,
> >
> > 2017-05-02 12:54 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
> >>
> >>
> >> 2017-05-02 12:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >>>
> >>> On 02/05/17 09:52, Violeta Georgieva wrote:
> >>>> Hi,
> >>>>
> >>>> I have a question about Async state DISPATCHING.
> >>>>
> >>>> I have the following scenario
> >>>> - Application invokes startAsync() and then continues processing in a
> >>>> separate non-container thread A.
> >>>> - At some point thread A makes dispatch() -> Async state is changed
to
> >>>> DISPATCHING
> >>>> - At the same time a socket error occurs and thread B tries to change
> >  the
> >>>> Async state to ERROR, as the state was already changed to
> > DISPATCHING, the
> >>>> exception below occurs and the state cannot be changed to ERROR.
> >>>>
> >>>> Isn't that a problem as onError event will not be called?
> >>>> Why don't we treat DISPATCHING as DISPATCHED in asyncError?
> >>>>
> >
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388
> >>>
> >>> It depends. (Async errors and threading issues have been a topic of
> >>> discussion in the Servlet EG recently.)
> >>>
> >>> Could you explain a little more about what triggers this?
> >>>
> >>> My initial thoughts are that the socket error should be visible to the
> >>> application and therefore the application should not call dispatch.
> >>
> >> In the described scenario above yes it sees the socket error but even
> > though it calls dispatch.
> >> Don’t we need to ensure onError is called so that if there are
libraries
> > in between (not only server and application) they will receive onError
> > event?
> >
> > What do you think? Is this an issue that requires a fix?
>
> I'm leaning towards thinking this is an application bug.
>
> If the application has two non-container threads, A and B, then, as per
> section 2.3.3.4 of the Servlet spec, the application is responsible for
> accessing the request from those threads in a thread-safe manner.

Yep I agree with the above, but in this scenario we have one non-container
thread A
and the other thread B is actually a container thread that tries to send a
notification for an error.

org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception while
processing an asynchronous request
 java.lang.IllegalStateException: Calling [asyncError()] is not valid for a
request with Async state [DISPATCHING]
at org.apache.coyote.AsyncStateMachine.asyncError(
AsyncStateMachine.java:398)
at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
at org.apache.coyote.Request.action(Request.java:390)
at org.apache.catalina.core.AsyncContextImpl.setErrorState(
AsyncContextImpl.java:385)
at org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(
CoyoteAdapter.java:175)
at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
at org.apache.coyote.AbstractProcessorLight.process(
AbstractProcessorLight.java:53)

What do you think?

Violeta

> In the scenario above, thread A should only call dispatch once the I/O
> activity in thread B has completed.
>
> Alternatively, once thread A has called dispatch, thread B should not be
> performing any I/O.
>
> Mark
>
>
> >
> > Thanks,
> > Violeta
> >
> >>
> >>
> >>> Mark
> >>>
> >>>
> >>>>
> >>>> Thanks,
> >>>> Violeta
> >>>>
> >>>> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
> > while
> >>>> processing an asynchronous request
> >>>>
> >>>>  java.lang.IllegalStateException: Calling [asyncError()] is not valid
> > for a
> >>>> request with Async state [DISPATCHING]
> >>>>
> >>>> at
> >>>>
> >
org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
> >>>>
> >>>> at
> > org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> >>>>
> >>>> at org.apache.coyote.Request.action(Request.java:390)
> >>>>
> >>>> at
> >>>>
> >
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)
> >>>>
> >>>> at
> >>>>
> >
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)
> >>>>
> >>>> at
> > org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> >>>>
> >>>> at
> >>>>
> >
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
> >>>>
> >>>> at
> >>>>
> >
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)
> >>>>
> >>>> at
> >>>>
> >
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)
> >>>>
> >>>> at
> >>>>
> >
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
> >>>>
> >>>> at
> >>>>
> >
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> >>>>
> >>>> at
> >>>>
> >
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> >>>>
> >>>> at
> >>>>
> >
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >>>>
> >>>> at java.lang.Thread.run(Thread.java:745)
> >>>>
> >>>
> >>>
> >>> ---------------------------------------------------------------------
> >>> 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: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 10/05/17 20:50, Violeta Georgieva wrote:
> Hi,
> 
> 2017-05-02 12:54 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>>
>>
>> 2017-05-02 12:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
>>>
>>> On 02/05/17 09:52, Violeta Georgieva wrote:
>>>> Hi,
>>>>
>>>> I have a question about Async state DISPATCHING.
>>>>
>>>> I have the following scenario
>>>> - Application invokes startAsync() and then continues processing in a
>>>> separate non-container thread A.
>>>> - At some point thread A makes dispatch() -> Async state is changed to
>>>> DISPATCHING
>>>> - At the same time a socket error occurs and thread B tries to change
>  the
>>>> Async state to ERROR, as the state was already changed to
> DISPATCHING, the
>>>> exception below occurs and the state cannot be changed to ERROR.
>>>>
>>>> Isn't that a problem as onError event will not be called?
>>>> Why don't we treat DISPATCHING as DISPATCHED in asyncError?
>>>>
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388
>>>
>>> It depends. (Async errors and threading issues have been a topic of
>>> discussion in the Servlet EG recently.)
>>>
>>> Could you explain a little more about what triggers this?
>>>
>>> My initial thoughts are that the socket error should be visible to the
>>> application and therefore the application should not call dispatch.
>>
>> In the described scenario above yes it sees the socket error but even
> though it calls dispatch.
>> Don’t we need to ensure onError is called so that if there are libraries
> in between (not only server and application) they will receive onError
> event?
> 
> What do you think? Is this an issue that requires a fix?

I'm leaning towards thinking this is an application bug.

If the application has two non-container threads, A and B, then, as per
section 2.3.3.4 of the Servlet spec, the application is responsible for
accessing the request from those threads in a thread-safe manner.

In the scenario above, thread A should only call dispatch once the I/O
activity in thread B has completed.

Alternatively, once thread A has called dispatch, thread B should not be
performing any I/O.

Mark


> 
> Thanks,
> Violeta
> 
>>
>>
>>> Mark
>>>
>>>
>>>>
>>>> Thanks,
>>>> Violeta
>>>>
>>>> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
> while
>>>> processing an asynchronous request
>>>>
>>>>  java.lang.IllegalStateException: Calling [asyncError()] is not valid
> for a
>>>> request with Async state [DISPATCHING]
>>>>
>>>> at
>>>>
> org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
>>>>
>>>> at
> org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
>>>>
>>>> at org.apache.coyote.Request.action(Request.java:390)
>>>>
>>>> at
>>>>
> org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)
>>>>
>>>> at
>>>>
> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)
>>>>
>>>> at
> org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
>>>>
>>>> at
>>>>
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
>>>>
>>>> at
>>>>
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)
>>>>
>>>> at
>>>>
> org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)
>>>>
>>>> at
>>>>
> org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
>>>>
>>>> at
>>>>
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
>>>>
>>>> at
>>>>
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
>>>>
>>>> at
>>>>
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
>>>>
>>>> at java.lang.Thread.run(Thread.java:745)
>>>>
>>>
>>>
>>> ---------------------------------------------------------------------
>>> 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: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <vi...@apache.org>.
Hi,

2017-05-02 12:54 GMT+03:00 Violeta Georgieva <mi...@gmail.com>:
>
>
> 2017-05-02 12:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
> >
> > On 02/05/17 09:52, Violeta Georgieva wrote:
> > > Hi,
> > >
> > > I have a question about Async state DISPATCHING.
> > >
> > > I have the following scenario
> > > - Application invokes startAsync() and then continues processing in a
> > > separate non-container thread A.
> > > - At some point thread A makes dispatch() -> Async state is changed to
> > > DISPATCHING
> > > - At the same time a socket error occurs and thread B tries to change
 the
> > > Async state to ERROR, as the state was already changed to
DISPATCHING, the
> > > exception below occurs and the state cannot be changed to ERROR.
> > >
> > > Isn't that a problem as onError event will not be called?
> > > Why don't we treat DISPATCHING as DISPATCHED in asyncError?
> > >
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388
> >
> > It depends. (Async errors and threading issues have been a topic of
> > discussion in the Servlet EG recently.)
> >
> > Could you explain a little more about what triggers this?
> >
> > My initial thoughts are that the socket error should be visible to the
> > application and therefore the application should not call dispatch.
>
> In the described scenario above yes it sees the socket error but even
though it calls dispatch.
> Don’t we need to ensure onError is called so that if there are libraries
in between (not only server and application) they will receive onError
event?

What do you think? Is this an issue that requires a fix?

Thanks,
Violeta

>
>
> > Mark
> >
> >
> > >
> > > Thanks,
> > > Violeta
> > >
> > > org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
while
> > > processing an asynchronous request
> > >
> > >  java.lang.IllegalStateException: Calling [asyncError()] is not valid
for a
> > > request with Async state [DISPATCHING]
> > >
> > > at
> > >
org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
> > >
> > > at
org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> > >
> > > at org.apache.coyote.Request.action(Request.java:390)
> > >
> > > at
> > >
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)
> > >
> > > at
> > >
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)
> > >
> > > at
org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> > >
> > > at
> > >
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
> > >
> > > at
> > >
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)
> > >
> > > at
> > >
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)
> > >
> > > at
> > >
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
> > >
> > > at
> > >
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> > >
> > > at
> > >
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> > >
> > > at
> > >
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> > >
> > > at java.lang.Thread.run(Thread.java:745)
> > >
> >
> >
> > ---------------------------------------------------------------------
> > To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> > For additional commands, e-mail: dev-help@tomcat.apache.org
> >
>

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Violeta Georgieva <mi...@gmail.com>.
2017-05-02 12:25 GMT+03:00 Mark Thomas <ma...@apache.org>:
>
> On 02/05/17 09:52, Violeta Georgieva wrote:
> > Hi,
> >
> > I have a question about Async state DISPATCHING.
> >
> > I have the following scenario
> > - Application invokes startAsync() and then continues processing in a
> > separate non-container thread A.
> > - At some point thread A makes dispatch() -> Async state is changed to
> > DISPATCHING
> > - At the same time a socket error occurs and thread B tries to change
 the
> > Async state to ERROR, as the state was already changed to DISPATCHING,
the
> > exception below occurs and the state cannot be changed to ERROR.
> >
> > Isn't that a problem as onError event will not be called?
> > Why don't we treat DISPATCHING as DISPATCHED in asyncError?
> >
https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388
>
> It depends. (Async errors and threading issues have been a topic of
> discussion in the Servlet EG recently.)
>
> Could you explain a little more about what triggers this?
>
> My initial thoughts are that the socket error should be visible to the
> application and therefore the application should not call dispatch.

In the described scenario above yes it sees the socket error but even
though it calls dispatch.
Don’t we need to ensure onError is called so that if there are libraries in
between (not only server and application) they will receive onError event?

> Mark
>
>
> >
> > Thanks,
> > Violeta
> >
> > org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception
while
> > processing an asynchronous request
> >
> >  java.lang.IllegalStateException: Calling [asyncError()] is not valid
for a
> > request with Async state [DISPATCHING]
> >
> > at
> >
org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
> >
> > at
org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> >
> > at org.apache.coyote.Request.action(Request.java:390)
> >
> > at
> >
org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)
> >
> > at
> >
org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)
> >
> > at
org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> >
> > at
> >
org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
> >
> > at
> >
org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)
> >
> > at
> >
org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)
> >
> > at
> >
org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
> >
> > at
> >
java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> >
> > at
> >
java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> >
> > at
> >
org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> >
> > at java.lang.Thread.run(Thread.java:745)
> >
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>

Re: asyncError() is not valid while in Async state DISPATCHING

Posted by Mark Thomas <ma...@apache.org>.
On 02/05/17 09:52, Violeta Georgieva wrote:
> Hi,
> 
> I have a question about Async state DISPATCHING.
> 
> I have the following scenario
> - Application invokes startAsync() and then continues processing in a
> separate non-container thread A.
> - At some point thread A makes dispatch() -> Async state is changed to
> DISPATCHING
> - At the same time a socket error occurs and thread B tries to change  the
> Async state to ERROR, as the state was already changed to DISPATCHING, the
> exception below occurs and the state cannot be changed to ERROR.
> 
> Isn't that a problem as onError event will not be called?
> Why don't we treat DISPATCHING as DISPATCHED in asyncError?
> https://github.com/apache/tomcat/blob/trunk/java/org/apache/coyote/AsyncStateMachine.java#L388

It depends. (Async errors and threading issues have been a topic of
discussion in the Servlet EG recently.)

Could you explain a little more about what triggers this?

My initial thoughts are that the socket error should be visible to the
application and therefore the application should not call dispatch.

Mark


> 
> Thanks,
> Violeta
> 
> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch Exception while
> processing an asynchronous request
> 
>  java.lang.IllegalStateException: Calling [asyncError()] is not valid for a
> request with Async state [DISPATCHING]
> 
> at
> org.apache.coyote.AsyncStateMachine.asyncError(AsyncStateMachine.java:398)
> 
> at org.apache.coyote.AbstractProcessor.action(AbstractProcessor.java:393)
> 
> at org.apache.coyote.Request.action(Request.java:390)
> 
> at
> org.apache.catalina.core.AsyncContextImpl.setErrorState(AsyncContextImpl.java:385)
> 
> at
> org.apache.catalina.connector.CoyoteAdapter.asyncDispatch(CoyoteAdapter.java:175)
> 
> at org.apache.coyote.AbstractProcessor.dispatch(AbstractProcessor.java:225)
> 
> at
> org.apache.coyote.AbstractProcessorLight.process(AbstractProcessorLight.java:53)
> 
> at
> org.apache.coyote.AbstractProtocol$ConnectionHandler.process(AbstractProtocol.java:796)
> 
> at
> org.apache.tomcat.util.net.NioEndpoint$SocketProcessor.doRun(NioEndpoint.java:1366)
> 
> at
> org.apache.tomcat.util.net.SocketProcessorBase.run(SocketProcessorBase.java:49)
> 
> at
> java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1142)
> 
> at
> java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:617)
> 
> at
> org.apache.tomcat.util.threads.TaskThread$WrappingRunnable.run(TaskThread.java:61)
> 
> at java.lang.Thread.run(Thread.java:745)
> 


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