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 2016/04/24 18:15:34 UTC

svn commit: r1740707 - in /tomcat/trunk: java/org/apache/catalina/connector/ java/org/apache/catalina/valves/ java/org/apache/coyote/ java/org/apache/coyote/ajp/ java/org/apache/coyote/http11/ java/org/apache/coyote/http2/ test/org/apache/catalina/core/

Author: markt
Date: Sun Apr 24 16:15:34 2016
New Revision: 1740707

URL: http://svn.apache.org/viewvc?rev=1740707&view=rev
Log:
Third pass at fixing https://bz.apache.org/bugzilla/show_bug.cgi?id=59219
Includes a test case based on a patch by Abdessamed MANSOURI.

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
    tomcat/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
    tomcat/trunk/java/org/apache/coyote/ActionCode.java
    tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
    tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
    tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
    tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
    tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java

Modified: tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java Sun Apr 24 16:15:34 2016
@@ -364,6 +364,16 @@ public class CoyoteAdapter implements Ad
                         request.getContext().unbind(false, oldCL);
                     }
                 }
+
+                Throwable throwable =
+                        (Throwable) request.getAttribute(RequestDispatcher.ERROR_EXCEPTION);
+
+                // If an async request was started, is not going to end once
+                // this container thread finishes and an error occurred, trigger
+                // the async error process
+                if (!request.isAsyncCompleting() && throwable != null) {
+                    request.getAsyncContextInternal().setErrorState(throwable, true);
+                }
             } else {
                 request.finishRequest();
                 response.finishResponse();

Modified: tomcat/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Sun Apr 24 16:15:34 2016
@@ -1645,16 +1645,6 @@ public class Request implements HttpServ
         return asyncContext;
     }
 
-    public boolean isAsyncStarting() {
-        if (asyncContext == null) {
-            return false;
-        }
-
-        AtomicBoolean result = new AtomicBoolean(false);
-        coyoteRequest.action(ActionCode.ASYNC_IS_STARTING, result);
-        return result.get();
-    }
-
     @Override
     public boolean isAsyncStarted() {
         if (asyncContext == null) {

Modified: tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java (original)
+++ tomcat/trunk/java/org/apache/catalina/valves/ErrorReportValve.java Sun Apr 24 16:15:34 2016
@@ -100,13 +100,6 @@ public class ErrorReportValve extends Va
         // If an async request is in progress and is not going to end once this
         // container thread finishes, do not process any error page here.
         if (request.isAsync() && !request.isAsyncCompleting()) {
-            // If an async dispatch is in progress the error handling in the
-            // CoyoteAdapter will trigger the necessary processing. It is only
-            // necessary to trigger it here if async is starting (i.e. this is
-            // the post-processing of the service() method)
-            if (throwable != null && request.isAsyncStarting()) {
-                request.getAsyncContextInternal().setErrorState(throwable, true);
-            }
             return;
         }
 

Modified: tomcat/trunk/java/org/apache/coyote/ActionCode.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ActionCode.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ActionCode.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ActionCode.java Sun Apr 24 16:15:34 2016
@@ -164,11 +164,6 @@ public enum ActionCode {
     ASYNC_IS_ASYNC,
 
     /**
-     * Callback to determine if async processing is starting.
-     */
-    ASYNC_IS_STARTING,
-
-    /**
      * Callback to determine if async dispatch is in progress.
      */
     ASYNC_IS_STARTED,

Modified: tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] (original)
+++ tomcat/trunk/java/org/apache/coyote/AsyncStateMachine.java [UTF-8] Sun Apr 24 16:15:34 2016
@@ -179,10 +179,6 @@ public class AsyncStateMachine {
         return state.isDispatching();
     }
 
-    public boolean isAsyncStarting() {
-        return state == AsyncState.STARTING;
-    }
-
     public boolean isAsyncStarted() {
         return state.isStarted();
     }

Modified: tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/ajp/AjpProcessor.java Sun Apr 24 16:15:34 2016
@@ -571,10 +571,6 @@ public class AjpProcessor extends Abstra
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncError());
             break;
         }
-        case ASYNC_IS_STARTING: {
-            ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarting());
-            break;
-        }
         case ASYNC_IS_STARTED: {
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarted());
             break;

Modified: tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Sun Apr 24 16:15:34 2016
@@ -876,10 +876,6 @@ public class Http11Processor extends Abs
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncError());
             break;
         }
-        case ASYNC_IS_STARTING: {
-            ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarting());
-            break;
-        }
         case ASYNC_IS_STARTED: {
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarted());
             break;

Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Sun Apr 24 16:15:34 2016
@@ -291,10 +291,6 @@ public class StreamProcessor extends Abs
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncError());
             break;
         }
-        case ASYNC_IS_STARTING: {
-            ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarting());
-            break;
-        }
         case ASYNC_IS_STARTED: {
             ((AtomicBoolean) param).set(asyncStateMachine.isAsyncStarted());
             break;

Modified: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1740707&r1=1740706&r2=1740707&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java (original)
+++ tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Sun Apr 24 16:15:34 2016
@@ -1821,6 +1821,76 @@ public class TestAsyncContextImpl extend
         }
     }
 
+
+    @Test
+    public void testBug59219a() throws Exception{
+        doTestBug59219("", "doGet-onError-onComplete-");
+    }
+
+
+    @Test
+    public void testBug59219b() throws Exception{
+        doTestBug59219("?loops=3", "doGet-doGet-onStartAsync-doGet-onStartAsync-onError-onComplete-");
+    }
+
+
+    private void doTestBug59219(String queryString, String expectedTrack) throws Exception {
+        resetTracker();
+        Tomcat tomcat = getTomcatInstance();
+
+        Context ctx = tomcat.addContext("", null);
+        Wrapper w = tomcat.addServlet("", "async", new Bug59219Servlet());
+        w.setAsyncSupported(true);
+        ctx.addServletMapping("/async", "async");
+
+        tomcat.start();
+
+        getUrl("http://localhost:" + getPort() + "/async" + queryString);
+
+        // Wait up to 5s for the response
+        int count = 0;
+        while(!expectedTrack.equals(getTrack()) && count < 100) {
+            Thread.sleep(50);
+            count++;
+        }
+
+        Assert.assertEquals(expectedTrack, getTrack());
+    }
+
+
+    private static class Bug59219Servlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+
+        @Override
+        protected void doGet(HttpServletRequest req, HttpServletResponse resp)
+                throws ServletException, IOException {
+
+            track("doGet-");
+            AsyncContext ctx = req.startAsync();
+            ctx.setTimeout(3000);
+            ctx.addListener(new TrackingListener(true, false, "/async"));
+
+            String loopsParam = req.getParameter("loops");
+            Integer loopsAttr = (Integer) req.getAttribute("loops");
+
+            int loops = 0;
+            if (loopsAttr != null) {
+                loops = loopsAttr.intValue();
+            } else if (loopsParam != null) {
+                loops = Integer.parseInt(loopsParam);
+            }
+
+            if (loops > 1) {
+                loops--;
+                req.setAttribute("loops", Integer.valueOf(loops));
+                ctx.dispatch();
+            } else
+                throw new ServletException();
+        }
+
+    }
+
     @Test
     public void testForbiddenDispatching() throws Exception {
         resetTracker();



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