tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1509155 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/coyote/AsyncStateMachine.java test/org/apache/catalina/core/TestAsyncContextImpl.java webapps/docs/changelog.xml
Date Thu, 01 Aug 2013 10:08:29 GMT
Author: markt
Date: Thu Aug  1 10:08:29 2013
New Revision: 1509155

URL: http://svn.apache.org/r1509155
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55331
A dispatch to an async servlet during the onTimeout method of an async listener should not
trigger an ISE.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
    tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
  Merged /tomcat/trunk:r1507872,1509128,1509151

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java?rev=1509155&r1=1509154&r2=1509155&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/AsyncStateMachine.java Thu Aug  1 10:08:29
2013
@@ -200,6 +200,10 @@ public class AsyncStateMachine<S> {
         } else if (state == AsyncState.DISPATCHING) {
             state = AsyncState.DISPATCHED;
             return SocketState.ASYNC_END;
+        } else if (state == AsyncState.STARTED) {
+            // This can occur if an async listener does a dispatch to an async
+            // servlet during onTimeout
+            return SocketState.LONG;
         } else {
             throw new IllegalStateException(
                     sm.getString("asyncStateMachine.invalidAsyncState",

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java?rev=1509155&r1=1509154&r2=1509155&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Thu Aug 
1 10:08:29 2013
@@ -46,8 +46,6 @@ import javax.servlet.http.HttpServletRes
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertTrue;
-import static org.junit.Assert.fail;
-
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -398,15 +396,28 @@ public class TestAsyncContextImpl extend
     }
 
     @Test
-    public void testTimeoutListenerCompleteDispatch() throws Exception {
+    public void testTimeoutListenerCompleteNonAsyncDispatch() throws Exception {
+        // Should trigger an error - can't do both
+        doTestTimeout(Boolean.TRUE, Boolean.FALSE);
+    }
+
+    @Test
+    public void testTimeoutListenerNoCompleteNonAsyncDispatch()
+            throws Exception {
+        // Should work
+        doTestTimeout(Boolean.FALSE, Boolean.FALSE);
+    }
+
+    @Test
+    public void testTimeoutListenerCompleteAsyncDispatch() throws Exception {
         // Should trigger an error - can't do both
-        doTestTimeout(Boolean.TRUE, "/nonasync");
+        doTestTimeout(Boolean.TRUE, Boolean.TRUE);
     }
 
     @Test
-    public void testTimeoutListenerNoCompleteDispatch() throws Exception {
+    public void testTimeoutListenerNoCompleteAsyncDispatch() throws Exception {
         // Should work
-        doTestTimeout(Boolean.FALSE, "/nonasync");
+        doTestTimeout(Boolean.FALSE, Boolean.TRUE);
     }
 
     @Test
@@ -415,21 +426,24 @@ public class TestAsyncContextImpl extend
         doTestTimeout(null, null);
     }
 
-    private void doTestTimeout(Boolean completeOnTimeout, String dispatchUrl)
-    throws Exception {
+    private void doTestTimeout(Boolean completeOnTimeout, Boolean asyncDispatch)
+            throws Exception {
+
+        String dispatchUrl = null;
+        if (asyncDispatch != null) {
+            if (asyncDispatch.booleanValue()) {
+                dispatchUrl = "/async";
+            } else {
+                dispatchUrl = "/nonasync";
+            }
+        }
+
         // Setup Tomcat instance
         Tomcat tomcat = getTomcatInstance();
 
         // Must have a real docBase - just use temp
         File docBase = new File(System.getProperty("java.io.tmpdir"));
 
-        // Create the folder that will trigger the redirect
-        File foo = new File(docBase, "async");
-        addDeleteOnTearDown(foo);
-        if (!foo.mkdirs() && !foo.isDirectory()) {
-            fail("Unable to create async directory in docBase");
-        }
-
         Context ctx = tomcat.addContext("", docBase.getAbsolutePath());
 
         TimeoutServlet timeout =
@@ -437,13 +451,22 @@ public class TestAsyncContextImpl extend
 
         Wrapper wrapper = Tomcat.addServlet(ctx, "time", timeout);
         wrapper.setAsyncSupported(true);
-        ctx.addServletMapping("/async", "time");
+        ctx.addServletMapping("/start", "time");
 
-        if (dispatchUrl != null) {
-            NonAsyncServlet nonAsync = new NonAsyncServlet();
-            Tomcat.addServlet(ctx, "nonasync", nonAsync);
-            ctx.addServletMapping(dispatchUrl, "nonasync");
-        }
+        if (asyncDispatch != null) {
+            if (asyncDispatch.booleanValue()) {
+                AsyncStartRunnable asyncStartRunnable =
+                        new AsyncStartRunnable();
+                Wrapper async =
+                        Tomcat.addServlet(ctx, "async", asyncStartRunnable);
+                async.setAsyncSupported(true);
+                ctx.addServletMapping(dispatchUrl, "async");
+            } else {
+                NonAsyncServlet nonAsync = new NonAsyncServlet();
+                Tomcat.addServlet(ctx, "nonasync", nonAsync);
+                ctx.addServletMapping(dispatchUrl, "nonasync");
+            }
+         }
 
         ctx.addApplicationListener(new ApplicationListener(
                 TrackingRequestListener.class.getName(), false));
@@ -456,7 +479,7 @@ public class TestAsyncContextImpl extend
         tomcat.start();
         ByteChunk res = new ByteChunk();
         try {
-            getUrl("http://localhost:" + getPort() + "/async", res, null);
+            getUrl("http://localhost:" + getPort() + "/start", res, null);
         } catch (IOException ioe) {
             // Ignore - expected for some error conditions
         }
@@ -470,8 +493,12 @@ public class TestAsyncContextImpl extend
             expected.append("requestDestroyed");
         } else {
             expected.append("onTimeout-");
-            if (dispatchUrl != null) {
-                expected.append("NonAsyncServletGet-");
+            if (asyncDispatch != null) {
+                if (asyncDispatch.booleanValue()) {
+                    expected.append("onStartAsync-Runnable-");
+                } else {
+                    expected.append("NonAsyncServletGet-");
+                }
             }
             expected.append("onComplete-");
             expected.append("requestDestroyed");
@@ -487,12 +514,16 @@ public class TestAsyncContextImpl extend
                     TimeoutServlet.ASYNC_TIMEOUT + TIMEOUT_MARGIN +
                     REQUEST_TIME);
         } else {
-            alvGlobal.validateAccessLog(1, 200, TimeoutServlet.ASYNC_TIMEOUT,
-                    TimeoutServlet.ASYNC_TIMEOUT + TIMEOUT_MARGIN +
-                    REQUEST_TIME);
-            alv.validateAccessLog(1, 200, TimeoutServlet.ASYNC_TIMEOUT,
-                    TimeoutServlet.ASYNC_TIMEOUT + TIMEOUT_MARGIN +
-                    REQUEST_TIME);
+            long timeoutDelay = TimeoutServlet.ASYNC_TIMEOUT;
+            if (asyncDispatch != null && asyncDispatch.booleanValue() &&
+                    !completeOnTimeout.booleanValue()) {
+                // Extra timeout in this case
+                timeoutDelay += TimeoutServlet.ASYNC_TIMEOUT;
+            }
+            alvGlobal.validateAccessLog(1, 200, timeoutDelay,
+                    timeoutDelay + TIMEOUT_MARGIN + REQUEST_TIME);
+            alv.validateAccessLog(1, 200, timeoutDelay,
+                    timeoutDelay + TIMEOUT_MARGIN + REQUEST_TIME);
         }
     }
 

Modified: tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml?rev=1509155&r1=1509154&r2=1509155&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml Thu Aug  1 10:08:29 2013
@@ -92,6 +92,11 @@
         refers to the value of the <code>contextClass</code> attribute of Host.
         (kfujino)
       </fix>
+      <fix>
+        <bug>55331</bug>: Dispatching to an asychronous servlet from
+        <code>AsyncListener.onTimeout()</code> should not trigger an
+        <code>IllegalStateException</code>. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Coyote">



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


Mime
View raw message