Author: pero
Date: Sat Jul  3 22:48:16 2010
New Revision: 960283

URL: http://svn.apache.org/viewvc?rev=960283&view=rev
Log:
Fix Bug 49528 - HttpServletRequest.isAsyncStarted() returns false when a 
Runnable is started
Add isDebugEnabled statements.

Todo: Why we doesn't start directly a container thread with 
asyncContext.start()?
Currently we start user runnable as an internal forward.

Added:
    tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java   
(with props)
Modified:
    tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java?rev=960283&r1=960282&r2=960283&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/core/AsyncContextImpl.java Sat Jul  3 
22:48:16 2010
@@ -253,7 +253,7 @@ public class AsyncContextImpl implements
     }
 
     public boolean isStarted() {
-        return (state.get() == AsyncState.STARTED || state.get() == 
AsyncState.DISPATCHING);
+        return (state.get() == AsyncState.STARTED || state.get() == 
AsyncState.DISPATCHING || state.get() == AsyncState.DISPATCHED);
     }
 
     public void setStarted(Context context) {
@@ -292,7 +292,8 @@ public class AsyncContextImpl implements
     
     public void doInternalDispatch() throws ServletException, IOException {
         if (this.state.compareAndSet(AsyncState.TIMING_OUT, 
AsyncState.COMPLETING)) {
-            log.debug("TIMING OUT!");
+               if( log.isDebugEnabled())
+                   log.debug("TIMING OUT!");
             boolean listenerInvoked = false;
             for (AsyncListenerWrapper listener : listeners) {
                 listener.fireOnTimeout(event);
@@ -303,15 +304,18 @@ public class AsyncContextImpl implements
             }
             doInternalComplete(true);
         } else if (this.state.compareAndSet(AsyncState.ERROR_DISPATCHING, 
AsyncState.COMPLETING)) {
-            log.debug("ON ERROR!");
+            if( log.isDebugEnabled())
+                log.debug("ON ERROR!");
             boolean listenerInvoked = false;
             for (AsyncListenerWrapper listener : listeners) {
                 try {
                     listener.fireOnError(event);
                 }catch (IllegalStateException x) {
-                    log.debug("Listener invoked invalid state.",x);
+                    if( log.isDebugEnabled())
+                        log.debug("Listener invoked invalid state.",x);
                 }catch (Exception x) {
-                    log.debug("Exception during onError.",x);
+                    if(log.isDebugEnabled())
+                        log.debug("Exception during onError.",x);
                 }
                 listenerInvoked = true;
             }

Added: 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=960283&view=auto
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java (added)
+++ tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java Sat 
Jul  3 22:48:16 2010
@@ -0,0 +1,120 @@
+/*
+ *  Licensed to the Apache Software Foundation (ASF) under one or more
+ *  contributor license agreements.  See the NOTICE file distributed with
+ *  this work for additional information regarding copyright ownership.
+ *  The ASF licenses this file to You under the Apache License, Version 2.0
+ *  (the "License"); you may not use this file except in compliance with
+ *  the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+package org.apache.catalina.core;
+
+import java.io.File;
+import java.io.IOException;
+
+import javax.servlet.ServletException;
+
+import javax.servlet.http.HttpServlet;
+import javax.servlet.http.HttpServletRequest;
+import javax.servlet.http.HttpServletResponse;
+
+import org.apache.catalina.Context;
+import org.apache.catalina.Wrapper;
+import org.apache.catalina.startup.Tomcat;
+import org.apache.catalina.startup.TomcatBaseTest;
+
+/**
+ * Simulate Bug 49528.
+ * 
+ * @author Peter Rossbach
+ * @version $Revision$
+ */
+public class TestAsyncContextImpl  extends TomcatBaseTest {
+
+    public void testIsAsyncStarted() throws Exception {
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();        
+
+        BUG49528Servlet servlet = createTestApp(tomcat);
+        tomcat.start();
+        getUrl("http://localhost:"; + getPort() + "/async");
+        // currently the bug show that we don't start a thread. All message 
log the same thread id!
+        assertTrue(servlet.asyncStartBeforeDispatched);
+        assertTrue(servlet.asyncStartRun);
+        assertFalse(servlet.asyncStartAfterComplete);
+    }
+    
+    private BUG49528Servlet createTestApp(Tomcat tomcat) {
+        // 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");
+        if (!foo.exists() && !foo.mkdirs()) {
+            fail("Unable to create async directory in docBase");
+        }
+        
+        Context ctx = tomcat.addContext("/", docBase.getAbsolutePath());
+
+        BUG49528Servlet servlet = new BUG49528Servlet();
+        Wrapper wrapper = Tomcat.addServlet(ctx, "test", servlet);
+        wrapper.setAsyncSupported(true);
+        ctx.addServletMapping("/async", "test");
+        return servlet;
+    }
+    
+    // see BUG 49528 report TestCase servlet
+    private static class BUG49528Servlet extends HttpServlet {
+
+        private static final long serialVersionUID = 1L;
+ 
+        protected volatile boolean asyncStartBeforeDispatched = false;
+        
+        protected volatile boolean asyncStartRun = false;
+        
+        protected volatile boolean asyncStartAfterComplete = true;
+
+        @Override
+        protected void doGet(final HttpServletRequest request, 
HttpServletResponse response)
+                throws ServletException, IOException {
+
+            long rtid = Thread.currentThread().getId();
+            log(rtid + " Start async()");
+            request.startAsync();
+            log(rtid + " Dispatching start()");
+            log(rtid + " request.isAsyncStarted()1" + 
request.isAsyncStarted());
+            asyncStartBeforeDispatched = request.isAsyncStarted();
+            request.getAsyncContext().start(new Runnable() {
+                @Override
+                public void run() {
+                    try {
+                        long tid = Thread.currentThread().getId();
+                        asyncStartRun = request.isAsyncStarted();
+                        log(tid + " request.isAsyncStarted()2" + 
request.isAsyncStarted());
+                        log(tid + " Before sleep()");
+                        Thread.sleep(500);
+                        log(tid + " After sleep()");
+                        log(tid + " request.isAsyncStarted()3" + 
request.isAsyncStarted());
+                        request.getAsyncContext().complete();
+                        asyncStartAfterComplete = request.isAsyncStarted();
+                        log(tid + " Returning from run()");
+                        log(tid + " request.isAsyncStarted()4" + 
request.isAsyncStarted());
+                    } catch (InterruptedException e) {
+                        // TODO Auto-generated catch block
+                        e.printStackTrace();
+                    }
+                }
+            });
+            log(rtid + " Returning from doGet()");
+        }
+        
+    }
+
+}

Propchange: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
------------------------------------------------------------------------------
    svn:eol-style = native

Propchange: tomcat/trunk/test/org/apache/catalina/core/TestAsyncContextImpl.java
------------------------------------------------------------------------------
    svn:keywords = Author Date Id Revision

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: 
http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=960283&r1=960282&r2=960283&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Sat Jul  3 22:48:16 2010
@@ -38,6 +38,10 @@
   <subsection name="Catalina">
     <changelog>
       <fix>
+        <bug>49528</bug>: HttpServletRequest.isAsyncStarted() now returns true 
when a Runnable is started! 
+        Reported by Pieter Libin (pero)
+      </fix>
+      <fix>
         GSOC 2010. Continue work to align MBean descriptors with reality. Patch
         provided by Chamith Buddhika. (markt)
       </fix>



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

Reply via email to