You are viewing a plain text version of this content. The canonical link for it is here.

Posted to dev@tomcat.apache.org by pe...@apache.org on 2010/07/04 00:48:16 UTC

svn commit: r960283 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java test/org/apache/catalina/core/TestAsyncContextImpl.java webapps/docs/changelog.xml

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-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org

Re: svn commit: r960283 - in /tomcat/trunk: java/org/apache/catalina/core/AsyncContextImpl.java test/org/apache/catalina/core/TestAsyncContextImpl.java webapps/docs/changelog.xml

Posted by Peter Roßbach <pr...@objektpark.de>.

Hi Rainer,

source of AsyncContextImpl has a lot of open checkstyle warnings. I  
check my eclipse
but all formatter use space instead tabs. Very strange...

Peter

Am 04.07.2010 um 01:17 schrieb Rainer Jung:

> Hi Peter,
>
> On 04.07.2010 00:48, pero@apache.org wrote:
>> 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!");
>
> Style nitpicks:
>
> - indentation wrong, maybe tabs involved
> - spacing around "if"
>
>>              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!");
>
> - spacing around "if"
>
>>              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);
>
> - spacing around "if"
>
>>                  }catch (Exception x) {
>> -                    log.debug("Exception during onError.",x);
>> +                    if(log.isDebugEnabled())
>> +                        log.debug("Exception during onError.",x);
>
> - spacing around "if"
>
>>                  }
>>                  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
>
>> +    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()) {
>
> - spacing around "&&"
>
>> 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>
>
> - indentation
> - no need for exclamation mark
>
> Regards,
>
> Rainer
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>


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