Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Rémy Maucherat
2014-10-29 22:28 GMT+01:00 Mark Thomas :

>
> Thanks. I've just removed it :)
>
> Adding the isCompleting() code and using it in the ErrorReportValve
> appears to have fixed all the issues with the unit tests apart from the
> ISE issues with get[Request|Response]() which are just issues with the
> tests. I'll work on those next.
>
> Very nice !

Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 20:03, Rémy Maucherat wrote:
> 2014-10-29 20:27 GMT+01:00 Mark Thomas :
> 
>> OK. Lets leave it in place until the issues with the tests are fixed.
>>
>> The started flag being true for the MUST_COMPLETE state is causing
>> problems too. It breaks code like this:
>>
>> if (request.isAsyncStarted()) {
>> request.getAsyncContext().complete();
>> }
>>
>> While you could make the "effects of complete have to be delayed..."
>> argument, I don't think it holds in this case as the ability to test if
>> complete() needs to be called is more important.
>>
>> What do you think about reverting that particular change?
>>
>> It's the same thing, the started flag should be true until the end of the
> processing of complete, which means for example for code like:
> ac.complete();
> println("" + ac.isAsyncStarted());
> 
> It should display "true" rather than "false".
> 
> I'll add the strict flag as well for that.

Thanks. I've just removed it :)

Adding the isCompleting() code and using it in the ErrorReportValve
appears to have fixed all the issues with the unit tests apart from the
ISE issues with get[Request|Response]() which are just issues with the
tests. I'll work on those next.

Mark

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



Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Rémy Maucherat
2014-10-29 20:27 GMT+01:00 Mark Thomas :

> OK. Lets leave it in place until the issues with the tests are fixed.
>
> The started flag being true for the MUST_COMPLETE state is causing
> problems too. It breaks code like this:
>
> if (request.isAsyncStarted()) {
> request.getAsyncContext().complete();
> }
>
> While you could make the "effects of complete have to be delayed..."
> argument, I don't think it holds in this case as the ability to test if
> complete() needs to be called is more important.
>
> What do you think about reverting that particular change?
>
> It's the same thing, the started flag should be true until the end of the
processing of complete, which means for example for code like:
ac.complete();
println("" + ac.isAsyncStarted());

It should display "true" rather than "false".

I'll add the strict flag as well for that.

Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 19:27, Mark Thomas wrote:
> On 29/10/2014 19:05, Rémy Maucherat wrote:
>> 2014-10-29 20:00 GMT+01:00 Mark Thomas :
>>
>>> OK. That makes sense.
>>>
>>> My initial reading of the spec was that the ISE was required as soon as
>>> complete() was called. After all, that is what the Javadoc says.
>>>
>>> However...  the Javadoc for complete() says:
>>> 
>>> If this method is called before the container-initiated dispatch that
>>> called startAsync has returned to the container, then the call will not
>>> take effect (and any invocations of AsyncListener.onComplete(AsyncEvent)
>>> will be delayed) until after the container-initiated dispatch has
>>> returned to the container.
>>> 
>>>
>>> It is certainly possible to read the Javadoc for complete() as meaning
>>> that the ISE should not be thrown until after onComplete() finishes
>>> although that isn't how I have been reading it.
>>>
>>> I withdraw my veto (again) and I don't see the need for those
>>> STRICT_SERVLET_COMPLIANCE checks either.
>>>
>>> I'll take a look at the failing unit tests and see what I can do to get
>>> them to pass.
>>>
>> Yes, the javadoc is kinda wrong since it should mention the associated
>> event. Use of the strict flag could remain until the testsuite passes IMO.
> 
> OK. Lets leave it in place until the issues with the tests are fixed.
> 
> The started flag being true for the MUST_COMPLETE state is causing
> problems too. It breaks code like this:
> 
> if (request.isAsyncStarted()) {
> request.getAsyncContext().complete();
> }
> 
> While you could make the "effects of complete have to be delayed..."
> argument, I don't think it holds in this case as the ability to test if
> complete() needs to be called is more important.
> 
> What do you think about reverting that particular change?

For the record, revert that change and all the other test failures go away.

Mark


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



Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 19:05, Rémy Maucherat wrote:
> 2014-10-29 20:00 GMT+01:00 Mark Thomas :
> 
>> OK. That makes sense.
>>
>> My initial reading of the spec was that the ISE was required as soon as
>> complete() was called. After all, that is what the Javadoc says.
>>
>> However...  the Javadoc for complete() says:
>> 
>> If this method is called before the container-initiated dispatch that
>> called startAsync has returned to the container, then the call will not
>> take effect (and any invocations of AsyncListener.onComplete(AsyncEvent)
>> will be delayed) until after the container-initiated dispatch has
>> returned to the container.
>> 
>>
>> It is certainly possible to read the Javadoc for complete() as meaning
>> that the ISE should not be thrown until after onComplete() finishes
>> although that isn't how I have been reading it.
>>
>> I withdraw my veto (again) and I don't see the need for those
>> STRICT_SERVLET_COMPLIANCE checks either.
>>
>> I'll take a look at the failing unit tests and see what I can do to get
>> them to pass.
>>
> Yes, the javadoc is kinda wrong since it should mention the associated
> event. Use of the strict flag could remain until the testsuite passes IMO.

OK. Lets leave it in place until the issues with the tests are fixed.

The started flag being true for the MUST_COMPLETE state is causing
problems too. It breaks code like this:

if (request.isAsyncStarted()) {
request.getAsyncContext().complete();
}

While you could make the "effects of complete have to be delayed..."
argument, I don't think it holds in this case as the ability to test if
complete() needs to be called is more important.

What do you think about reverting that particular change?

Mark

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



Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Rémy Maucherat
2014-10-29 20:00 GMT+01:00 Mark Thomas :

> OK. That makes sense.
>
> My initial reading of the spec was that the ISE was required as soon as
> complete() was called. After all, that is what the Javadoc says.
>
> However...  the Javadoc for complete() says:
> 
> If this method is called before the container-initiated dispatch that
> called startAsync has returned to the container, then the call will not
> take effect (and any invocations of AsyncListener.onComplete(AsyncEvent)
> will be delayed) until after the container-initiated dispatch has
> returned to the container.
> 
>
> It is certainly possible to read the Javadoc for complete() as meaning
> that the ISE should not be thrown until after onComplete() finishes
> although that isn't how I have been reading it.
>
> I withdraw my veto (again) and I don't see the need for those
> STRICT_SERVLET_COMPLIANCE checks either.
>
> I'll take a look at the failing unit tests and see what I can do to get
> them to pass.
>
> Yes, the javadoc is kinda wrong since it should mention the associated
event. Use of the strict flag could remain until the testsuite passes IMO.

Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 18:28, Rémy Maucherat wrote:
> 2014-10-29 19:22 GMT+01:00 Mark Thomas :
> 
>> OK. Try again :)
>>
>> -1 veto.
>>
>> This breaks a requirement of the Servlet 3.1 spec that an ISE should be
>> thrown if get[Request|Response]() is called after complete() is called.
>>
>> This commit breaks the unit tests that check this (the calls do not
>> thrown any exception).
>>
>> The commit did not include any test cases. What problem are you trying
>> to solve? And where in the spec does it mandate the behaviour you are
>> trying to achieve?
>>
> 
> The problem I am trying to solve is that the listener onComplete should be
> able to call getResponse and write things. Basically complete is not done
> until onComplete is done.

OK. That makes sense.

My initial reading of the spec was that the ISE was required as soon as
complete() was called. After all, that is what the Javadoc says.

However...  the Javadoc for complete() says:

If this method is called before the container-initiated dispatch that
called startAsync has returned to the container, then the call will not
take effect (and any invocations of AsyncListener.onComplete(AsyncEvent)
will be delayed) until after the container-initiated dispatch has
returned to the container.


It is certainly possible to read the Javadoc for complete() as meaning
that the ISE should not be thrown until after onComplete() finishes
although that isn't how I have been reading it.

I withdraw my veto (again) and I don't see the need for those
STRICT_SERVLET_COMPLIANCE checks either.

I'll take a look at the failing unit tests and see what I can do to get
them to pass.

Mark

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



Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Violeta Georgieva
Hi,

2014-10-29 20:28 GMT+02:00 Rémy Maucherat :
>
> 2014-10-29 19:22 GMT+01:00 Mark Thomas :
>
> > OK. Try again :)
> >
> > -1 veto.
> >
> > This breaks a requirement of the Servlet 3.1 spec that an ISE should be
> > thrown if get[Request|Response]() is called after complete() is called.
> >
> > This commit breaks the unit tests that check this (the calls do not
> > thrown any exception).
> >
> > The commit did not include any test cases. What problem are you trying
> > to solve? And where in the spec does it mandate the behaviour you are
> > trying to achieve?
> >
>
> The problem I am trying to solve is that the listener onComplete should be
> able to call getResponse and write things. Basically complete is not done
> until onComplete is done.

If I remember correctly the change for onComplete was introduced because of
https://issues.apache.org/bugzilla/show_bug.cgi?id=56190


Regards,
Violeta

>
> Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Rémy Maucherat
2014-10-29 19:22 GMT+01:00 Mark Thomas :

> OK. Try again :)
>
> -1 veto.
>
> This breaks a requirement of the Servlet 3.1 spec that an ISE should be
> thrown if get[Request|Response]() is called after complete() is called.
>
> This commit breaks the unit tests that check this (the calls do not
> thrown any exception).
>
> The commit did not include any test cases. What problem are you trying
> to solve? And where in the spec does it mandate the behaviour you are
> trying to achieve?
>

The problem I am trying to solve is that the listener onComplete should be
able to call getResponse and write things. Basically complete is not done
until onComplete is done.

Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Rémy Maucherat
2014-10-29 19:14 GMT+01:00 Mark Thomas :

>
> Hmm. I withdraw that veto. This change has no impact on the failing
> test. I need to investigate further. (It is odd though how we are now
> seeing a ServletException where there should be an ISE).
>
> If there are tests which contradict these fixes, they'll need to be fixed
IMO. I haven't run the TC testsuite yet with them, sorry.

Two of the changes should be rather logical though:
- getResponse should ISE after complete, but the listener onComplete call
should be part of that process
- the started flag being still true for MUST_COMPLETE is rather gratuitous,
but ...
- the javadoc says Throws: ServletException - if the given clazz fails to
be instantiated (so most exceptions should be wrapped)

Could 15 wait for next week ? I could have more edgy fixes on the way.

Rémy


Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 17:29, r...@apache.org wrote:
> Author: remm
> Date: Wed Oct 29 17:29:19 2014
> New Revision: 1635215
> 
> URL: http://svn.apache.org/r1635215
> Log:
> Fix three edgy async context bugs.

OK. Try again :)

-1 veto.

This breaks a requirement of the Servlet 3.1 spec that an ISE should be
thrown if get[Request|Response]() is called after complete() is called.

This commit breaks the unit tests that check this (the calls do not
thrown any exception).

The commit did not include any test cases. What problem are you trying
to solve? And where in the spec does it mandate the behaviour you are
trying to achieve?

I'd really like to get this resolved quickly so it doesn't delay the
8.0.15 tag.

Note that there are now multiple Async unit tests failures and I have
not yet checked them all to see if they have the same root cause.

Mark

> 
> Modified:
> tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
> tomcat/trunk/webapps/docs/changelog.xml
> 
> Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Wed Oct 
> 29 17:29:19 2014
> @@ -84,7 +84,6 @@ public class AsyncContextImpl implements
>  }
>  check();
>  request.getCoyoteRequest().action(ActionCode.ASYNC_COMPLETE, null);
> -clearServletRequestResponse();
>  }
>  
>  @Override
> @@ -104,6 +103,7 @@ public class AsyncContextImpl implements
>  }
>  }
>  } finally {
> +clearServletRequestResponse();
>  context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
>  }
>  
> @@ -290,6 +290,10 @@ public class AsyncContextImpl implements
>  } catch (ClassNotFoundException e) {
>  ServletException se = new ServletException(e);
>  throw se;
> +} catch (Exception e) {
> +ExceptionUtils.handleThrowable(e.getCause());
> +ServletException se = new ServletException(e);
> +throw se;
>  }
>  return listener;
>  }
> 
> Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] 
> (original)
> +++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] Wed 
> Oct 29 17:29:19 2014
> @@ -109,7 +109,7 @@ public class AsyncStateMachine {
>  DISPATCHED(false, false, false),
>  STARTING(true, true, false),
>  STARTED(true, true, false),
> -MUST_COMPLETE(true, false, false),
> +MUST_COMPLETE(true, true, false),
>  COMPLETING(true, false, false),
>  TIMING_OUT(true, false, false),
>  MUST_DISPATCH(true, true, true),
> 
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Wed Oct 29 17:29:19 2014
> @@ -183,6 +183,13 @@
>  full class path, ensure that class path entries added directly to the
>  web application class loader are scanned. (markt)
>
> +  
> +AsyncContext should remain usable until fireOnComplete is called. 
> (remm)
> +  
> +  
> +AsyncContext createListener should wrap any instantiation exception
> +using a ServletException. (remm)
> +  
>  
>
>
> @@ -219,6 +226,9 @@
>  AsyncContext.start(Runnable) during non-blocking IO 
> reads
>  and writes. (markt)
>
> +  
> +Async state MUST_COMPLETE should still be started. (remm)
> +  
>  
>
>
> 
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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



Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 18:11, Mark Thomas wrote:
> On 29/10/2014 17:29, r...@apache.org wrote:
>> Author: remm
>> Date: Wed Oct 29 17:29:19 2014
>> New Revision: 1635215
>>
>> URL: http://svn.apache.org/r1635215
>> Log:
>> Fix three edgy async context bugs.
>>
>> Modified:
>> tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
>> tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
>> tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1635215&r1=1635214&r2=1635215&view=diff
>> ==
>> --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java 
>> (original)
>> +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Wed Oct 
>> 29 17:29:19 2014
>> @@ -84,7 +84,6 @@ public class AsyncContextImpl implements
>>  }
>>  check();
>>  request.getCoyoteRequest().action(ActionCode.ASYNC_COMPLETE, null);
>> -clearServletRequestResponse();
>>  }
>>  
>>  @Override
>> @@ -104,6 +103,7 @@ public class AsyncContextImpl implements
>>  }
>>  }
>>  } finally {
>> +clearServletRequestResponse();
>>  context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
>>  }
>>  
>> @@ -290,6 +290,10 @@ public class AsyncContextImpl implements
>>  } catch (ClassNotFoundException e) {
>>  ServletException se = new ServletException(e);
>>  throw se;
>> +} catch (Exception e) {
>> +ExceptionUtils.handleThrowable(e.getCause());
>> +ServletException se = new ServletException(e);
>> +throw se;
> 
> -1 (veto)
> 
> The Servlet 3.1 specification requires that an IllegalStateException is
> thrown if get[Request|Response]() is called after complete().
> 
> There are unit tests for this that now fail.
> 
> Please revert this ASAP before it starts to block the 8.0.15 tag.

Hmm. I withdraw that veto. This change has no impact on the failing
test. I need to investigate further. (It is odd though how we are now
seeing a ServletException where there should be an ISE).

Mark


> Mark
> 
>>  }
>>  return listener;
>>  }
>>
>> Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1635215&r1=1635214&r2=1635215&view=diff
>> ==
>> --- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] 
>> (original)
>> +++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] Wed 
>> Oct 29 17:29:19 2014
>> @@ -109,7 +109,7 @@ public class AsyncStateMachine {
>>  DISPATCHED(false, false, false),
>>  STARTING(true, true, false),
>>  STARTED(true, true, false),
>> -MUST_COMPLETE(true, false, false),
>> +MUST_COMPLETE(true, true, false),
>>  COMPLETING(true, false, false),
>>  TIMING_OUT(true, false, false),
>>  MUST_DISPATCH(true, true, true),
>>
>> Modified: tomcat/trunk/webapps/docs/changelog.xml
>> URL: 
>> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1635215&r1=1635214&r2=1635215&view=diff
>> ==
>> --- tomcat/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/trunk/webapps/docs/changelog.xml Wed Oct 29 17:29:19 2014
>> @@ -183,6 +183,13 @@
>>  full class path, ensure that class path entries added directly to 
>> the
>>  web application class loader are scanned. (markt)
>>
>> +  
>> +AsyncContext should remain usable until fireOnComplete is called. 
>> (remm)
>> +  
>> +  
>> +AsyncContext createListener should wrap any instantiation exception
>> +using a ServletException. (remm)
>> +  
>>  
>>
>>
>> @@ -219,6 +226,9 @@
>>  AsyncContext.start(Runnable) during non-blocking IO 
>> reads
>>  and writes. (markt)
>>
>> +  
>> +Async state MUST_COMPLETE should still be started. (remm)
>> +  
>>  
>>
>>
>>
>>
>>
>> -
>> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
>> For additional commands, e-mail: dev-h...@tomcat.apache.org
>>
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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

Re: svn commit: r1635215 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

2014-10-29 Thread Mark Thomas
On 29/10/2014 17:29, r...@apache.org wrote:
> Author: remm
> Date: Wed Oct 29 17:29:19 2014
> New Revision: 1635215
> 
> URL: http://svn.apache.org/r1635215
> Log:
> Fix three edgy async context bugs.
> 
> Modified:
> tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
> tomcat/trunk/webapps/docs/changelog.xml
> 
> Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java 
> (original)
> +++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Wed Oct 
> 29 17:29:19 2014
> @@ -84,7 +84,6 @@ public class AsyncContextImpl implements
>  }
>  check();
>  request.getCoyoteRequest().action(ActionCode.ASYNC_COMPLETE, null);
> -clearServletRequestResponse();
>  }
>  
>  @Override
> @@ -104,6 +103,7 @@ public class AsyncContextImpl implements
>  }
>  }
>  } finally {
> +clearServletRequestResponse();
>  context.unbind(Globals.IS_SECURITY_ENABLED, oldCL);
>  }
>  
> @@ -290,6 +290,10 @@ public class AsyncContextImpl implements
>  } catch (ClassNotFoundException e) {
>  ServletException se = new ServletException(e);
>  throw se;
> +} catch (Exception e) {
> +ExceptionUtils.handleThrowable(e.getCause());
> +ServletException se = new ServletException(e);
> +throw se;

-1 (veto)

The Servlet 3.1 specification requires that an IllegalStateException is
thrown if get[Request|Response]() is called after complete().

There are unit tests for this that now fail.

Please revert this ASAP before it starts to block the 8.0.15 tag.

Mark

>  }
>  return listener;
>  }
> 
> Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] 
> (original)
> +++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] Wed 
> Oct 29 17:29:19 2014
> @@ -109,7 +109,7 @@ public class AsyncStateMachine {
>  DISPATCHED(false, false, false),
>  STARTING(true, true, false),
>  STARTED(true, true, false),
> -MUST_COMPLETE(true, false, false),
> +MUST_COMPLETE(true, true, false),
>  COMPLETING(true, false, false),
>  TIMING_OUT(true, false, false),
>  MUST_DISPATCH(true, true, true),
> 
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: 
> http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1635215&r1=1635214&r2=1635215&view=diff
> ==
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Wed Oct 29 17:29:19 2014
> @@ -183,6 +183,13 @@
>  full class path, ensure that class path entries added directly to the
>  web application class loader are scanned. (markt)
>
> +  
> +AsyncContext should remain usable until fireOnComplete is called. 
> (remm)
> +  
> +  
> +AsyncContext createListener should wrap any instantiation exception
> +using a ServletException. (remm)
> +  
>  
>
>
> @@ -219,6 +226,9 @@
>  AsyncContext.start(Runnable) during non-blocking IO 
> reads
>  and writes. (markt)
>
> +  
> +Async state MUST_COMPLETE should still be started. (remm)
> +  
>  
>
>
> 
> 
> 
> -
> To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org
> For additional commands, e-mail: dev-h...@tomcat.apache.org
> 


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