You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2019/10/14 16:27:38 UTC

[tomcat] branch 8.5.x updated: Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors

This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 8.5.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


The following commit(s) were added to refs/heads/8.5.x by this push:
     new 0495f66  Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
0495f66 is described below

commit 0495f664541cf4394875bcc2dfb38b05343f68c4
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Mon Oct 14 16:18:17 2019 +0100

    Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=63816 async errors
    
    Handle I/O errors on a non-container thread after asynchronous
    processing has been started but before the container thread that started
    asynchronous processing has completed processing the current
    request/response.
---
 java/org/apache/coyote/AbstractProcessor.java | 13 +++--------
 java/org/apache/coyote/AsyncStateMachine.java | 32 ++++-----------------------
 webapps/docs/changelog.xml                    |  6 +++++
 3 files changed, 13 insertions(+), 38 deletions(-)

diff --git a/java/org/apache/coyote/AbstractProcessor.java b/java/org/apache/coyote/AbstractProcessor.java
index 357e052..55a350d 100644
--- a/java/org/apache/coyote/AbstractProcessor.java
+++ b/java/org/apache/coyote/AbstractProcessor.java
@@ -113,17 +113,10 @@ public abstract class AbstractProcessor extends AbstractProcessorLight implement
         if (t != null) {
             request.setAttribute(RequestDispatcher.ERROR_EXCEPTION, t);
         }
-        if (blockIo && !ContainerThreadMarker.isContainerThread() && isAsync()) {
-            // The error occurred on a non-container thread during async
-            // processing which means not all of the necessary clean-up will
-            // 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();
-            if (getLog().isDebugEnabled()) {
-                getLog().debug(sm.getString("abstractProcessor.nonContainerThreadError"), t);
+        if (blockIo && isAsync()) {
+            if (asyncStateMachine.asyncError()) {
+                processSocketEvent(SocketEvent.ERROR, true);
             }
-            processSocketEvent(SocketEvent.ERROR, true);
         }
     }
 
diff --git a/java/org/apache/coyote/AsyncStateMachine.java b/java/org/apache/coyote/AsyncStateMachine.java
index cd98f38..81a883b 100644
--- a/java/org/apache/coyote/AsyncStateMachine.java
+++ b/java/org/apache/coyote/AsyncStateMachine.java
@@ -154,7 +154,6 @@ 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;
@@ -434,36 +433,13 @@ 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 boolean asyncError() {
+        clearNonBlockingListeners();
+        state = AsyncState.ERROR;
+        return !ContainerThreadMarker.isContainerThread();
     }
 
 
-    public synchronized void asyncError() {
-        if (state == AsyncState.STARTING ||
-                state == AsyncState.STARTED ||
-                state == AsyncState.DISPATCHED ||
-                state == AsyncState.TIMING_OUT ||
-                state == AsyncState.MUST_COMPLETE ||
-                state == AsyncState.READ_WRITE_OP ||
-                state == AsyncState.COMPLETING ||
-                state == AsyncState.MUST_ERROR) {
-            clearNonBlockingListeners();
-            state = AsyncState.ERROR;
-        } else {
-            throw new IllegalStateException(
-                    sm.getString("asyncStateMachine.invalidAsyncState",
-                            "asyncError()", state));
-        }
-    }
-
     public synchronized void asyncRun(Runnable runnable) {
         if (state == AsyncState.STARTING || state ==  AsyncState.STARTED ||
                 state == AsyncState.READ_WRITE_OP) {
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index ab69483..dbcaef7 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -62,6 +62,12 @@
         <code>AsyncListener.onTimeout()</code> or
         <code>AsyncListener.onError()</code>. (markt)
       </fix>
+      <fix>
+        <bug>63816</bug>: Correctly handle I/O errors on a non-container thread
+        after asynchronous processing has been started but before the container
+        thread that started asynchronous processing has completed processing the
+        current request/response. (markt)
+      </fix>
     </changelog>
   </subsection>
 </section>


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