Hi Mark, On Mon, Jun 12, 2017 at 8:42 PM, <ma...@apache.org> wrote:
> Author: markt > Date: Mon Jun 12 18:42:32 2017 > New Revision: 1798509 > > URL: http://svn.apache.org/viewvc?rev=1798509&view=rev > Log: > Make asynchronous error handling more robust. In particular ensure that > onError() is called for any registered AsyncListeners after an I/O error on > a non-container thread. > > Modified: > tomcat/trunk/conf/logging.properties > tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java > tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java > tomcat/trunk/webapps/docs/changelog.xml > > Modified: tomcat/trunk/conf/logging.properties > URL: http://svn.apache.org/viewvc/tomcat/trunk/conf/logging.prope > rties?rev=1798509&r1=1798508&r2=1798509&view=diff > ============================================================ > ================== > --- tomcat/trunk/conf/logging.properties (original) > +++ tomcat/trunk/conf/logging.properties Mon Jun 12 18:42:32 2017 > @@ -65,6 +65,8 @@ org.apache.catalina.core.ContainerBase.[ > > # To see debug messages for HTTP/2 handling, uncomment the following line: > #org.apache.coyote.http2.level = FINE > +org.apache.coyote.level = FINEST > +org.apache.catalina.level = FINEST > Look like debug leftovers. > > # To see debug messages for WebSocket handling, uncomment the following > line: > #org.apache.tomcat.websocket.level = FINE > > Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/co > yote/AbstractProcessor.java?rev=1798509&r1=1798508&r2=1798509&view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original) > +++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Mon Jun 12 > 18:42:32 2017 > @@ -95,6 +95,7 @@ public abstract class AbstractProcessor > // have been completed. Dispatch to a container thread to do > the > // clean-up. Need to do it this way to ensure that all the > necessary > // clean-up is performed. > + asyncStateMachine.asyncMustError(); > > getLog().info(sm.getString("abstractProcessor.nonContainerThreadError"), > t); > processSocketEvent(SocketEvent.ERROR, true); > } > > Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java > URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/co > yote/AsyncStateMachine.java?rev=1798509&r1=1798508&r2=1798509&view=diff > ============================================================ > ================== > --- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] > (original) > +++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] > Mon Jun 12 18:42:32 2017 > @@ -66,38 +66,49 @@ import org.apache.tomcat.util.security.P > * differences required to avoid race conditions > during error > * handling. > * DISPATCHING - The dispatch is being processed. > + * MUST_ERROR - ServletRequest.startAsync() has been called > followed by an > + * I/O error on a non-container thread. The main > purpose of > + * this state is to prevent additional async actions > + * (complete(), dispatch() etc.) on the non-container > thread. > + * The container will perform the necessary error > handling, > + * including ensuring that the AsyncLister.onError() > method > + * is called. > * ERROR - Something went wrong. > * > - * |-----------------»------| > - * | \|/ /---«------------------------- > ------«------------------------------| > - * | |----------«-----E R R O R--«-----------------------«-- > -----------------------------| | > - * | | complete() /|\/|\\ \-«--------------------------------«-------| > | | > - * | | | | \ > | | | > - * | | |-----»-------| | \-----------»----------| > | | | > - * | | | | |dispatch() > | | | > - * | | | | \|/ > ^ | | > - * | | | | |--|timeout() | > | | | > - * | | | post() | | \|/ | post() > | | | > - * | | | |---------- | --»DISPATCHED«---------- | > --------------COMPLETING«-----| | | > - * | | | | | /|\/|\ | | > | /|\ /|\ | | | > - * | | | | |---»- | ---| | |startAsync() | > timeout()|--| | | | | > - * | | ^ ^ | | | | | > | | ^ | > - * | | | | | |-- \ -----| | complete() | > |post() | | | > - * | | | | | | \ | /--»----- | > ---COMPLETE_PENDING-»-| ^ | | > - * | | | | | | \ | / | > | | | > - * | | | | | ^ \ | / | > | | | > - * | \|/ | | | | \ \|/ / post() | > complete() | | | > - * | MUST_COMPLETE-«- | - | --«----STARTING--»--------- | ------------| > /---»-----| | | > - * | /|\ /|\ | | complete() | \ | | > / | ^ > - * | | | | | | \ | post() | > / error() | | > - * | | | ^ | dispatch()| \ | |-----| | > //------»-------| | > - * | | | | | | \ | | | | > // | > - * | | | | | \|/ \ | | \|/\|/ > // post() | > + * |-----«---------------------- > ---------«------------------------------| > + * | > | > + * | error() > | > + * |-----------------»---| | |--«--------MUST_ERROR-------- > -------«------------------------| | > + * | \|/ \|/\|/ > | | > + * | |----------«-----E R R O R--«-----------------------«-- > -----------------------------| | | > + * | | complete() /|\/|\\ \-«--------------------------------«-------| > | | | > + * | | | | \ > | | | | > + * | | |-----»-------| | \-----------»----------| > | | | | > + * | | | | |dispatch() > | | ^ | > + * | | | | \|/ > ^ | | | > + * | | | | |--|timeout() | > | | | | > + * | | | post() | | \|/ | post() > | | | | > + * | | | |---------- | --»DISPATCHED«---------- | > --------------COMPLETING«-----| | | | > + * | | | | | /|\/|\ | | > | /|\ /|\ | | | | > + * | | | | |---»- | ---| | |startAsync() | > timeout()|--| | | | | | > + * | | ^ ^ | | | | | > | | ^ | | > + * | | | | | |-- \ -----| | complete() | > |post() | | | | > + * | | | | | | \ | /--»----- | > ---COMPLETE_PENDING-»-| ^ | | | > + * | | | | | | \ | / | > | | | | > + * | | | | | ^ \ | / | > complete() | | | | > + * | \|/ | | | | \ \|/ / post() | > /---»-----| | ^ | > + * | MUST_COMPLETE-«- | - | --«----STARTING--»--------- | ------------| > / | | | > + * | /|\ /|\ | | complete() | \ | | > / error() | | ^ > + * | | | | | | \ | | > //---»----------| | | > + * | | | ^ | dispatch()| \ | post() | > // | | > + * | | | | | | \ | |-----| | > // nct-io-error | | > + * | | | | | | \ | | | | > ///---»---------------| | > + * | | | | | \|/ \ | | \|/\| > ||| | > * | | | | |--«--MUST_DISPATCH-----«-----| > |--«--STARTED«---------«---------| | > - * | | | | dispatched() /|\ | \ / | > | | | > - * | | | | | | \ / | > | | | > - * | | | | | | \ / | > | | | > - * | | | | | |post() \ | | > | ^ | > + * | | | | dispatched() /|\ | \ / | > | post() | | > + * | | | | | | \ / | > | | | > + * | | | | | | \ / | > | | | > + * | | | | | |post() | | | > | ^ | > * ^ | ^ | | | \|/ | | > |asyncOperation() | | > * | | | ^ | | DISPATCH_PENDING | | > | | | > * | | | | | | |post() | | > | | | > @@ -142,6 +153,7 @@ public class AsyncStateMachine { > DISPATCH_PENDING(true, true, false, false), > DISPATCHING (true, false, false, true), > READ_WRITE_OP (true, true, false, false), > + MUST_ERROR (true, true, false, false), > ERROR (true, true, false, false); > > private final boolean isAsync; > @@ -384,6 +396,18 @@ public class AsyncStateMachine { > } > > > + public synchronized void asyncMustError() { > + if (state == AsyncState.STARTED) { > + clearNonBlockingListeners(); > + state = AsyncState.MUST_ERROR; > + } else { > + throw new IllegalStateException( > + sm.getString("asyncStateMachine.invalidAsyncState", > + "asyncMustError()", state)); > + } > + } > + > + > public synchronized void asyncError() { > if (state == AsyncState.STARTING || > state == AsyncState.STARTED || > @@ -391,7 +415,8 @@ public class AsyncStateMachine { > state == AsyncState.TIMING_OUT || > state == AsyncState.MUST_COMPLETE || > state == AsyncState.READ_WRITE_OP || > - state == AsyncState.COMPLETING) { > + state == AsyncState.COMPLETING || > + state == AsyncState.MUST_ERROR) { > clearNonBlockingListeners(); > state = AsyncState.ERROR; > } else { > > Modified: tomcat/trunk/webapps/docs/changelog.xml > URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/chang > elog.xml?rev=1798509&r1=1798508&r2=1798509&view=diff > ============================================================ > ================== > --- tomcat/trunk/webapps/docs/changelog.xml (original) > +++ tomcat/trunk/webapps/docs/changelog.xml Mon Jun 12 18:42:32 2017 > @@ -117,6 +117,12 @@ > changed the status code recorded in the access log when the client > dropped the connection from 200 to 500. (markt) > </fix> > + <fix> > + Make asynchronous error handling more robust. In particular > ensure that > + <code>onError()</code> is called for any registered > + <code>AsyncListener</code>s after an I/O error on a non-container > + thread. (markt) > + </fix> > </changelog> > </subsection> > <subsection name="Jasper"> > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org > For additional commands, e-mail: dev-h...@tomcat.apache.org > >
