svn commit: r1798509 - in /tomcat/trunk: conf/logging.properties java/org/apache/coyote/AbstractProcessor.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

Previous Topic Next Topic
 
classic Classic list List threaded Threaded
2 messages Options
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

svn commit: r1798509 - in /tomcat/trunk: conf/logging.properties java/org/apache/coyote/AbstractProcessor.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

markt
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.properties?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
 
 # 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/coyote/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/coyote/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/changelog.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: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

Re: svn commit: r1798509 - in /tomcat/trunk: conf/logging.properties java/org/apache/coyote/AbstractProcessor.java java/org/apache/coyote/AsyncStateMachine.java webapps/docs/changelog.xml

Martin Grigorov
Hi Mark,

On Mon, Jun 12, 2017 at 8:42 PM, <[hidden email]> 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: [hidden email]
> For additional commands, e-mail: [hidden email]
>
>
Loading...