tomcat-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ma...@apache.org
Subject svn commit: r1178456 - in /tomcat/tc7.0.x/trunk: ./ java/org/apache/catalina/connector/ java/org/apache/coyote/http11/ test/org/apache/catalina/comet/
Date Mon, 03 Oct 2011 17:00:47 GMT
Author: markt
Date: Mon Oct  3 17:00:47 2011
New Revision: 1178456

URL: http://svn.apache.org/viewvc?rev=1178456&view=rev
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51881
Don't mark processors handling comet requests as non-comet too early.
Before this fix, finishing a comet request was processed as non-comet meaning the comet clean-up
code was not executed which was likely to break processing of the next request on the connection.

Modified:
    tomcat/tc7.0.x/trunk/   (props changed)
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CometEventImpl.java
    tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
    tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java
    tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java

Propchange: tomcat/tc7.0.x/trunk/
------------------------------------------------------------------------------
--- svn:mergeinfo (original)
+++ svn:mergeinfo Mon Oct  3 17:00:47 2011
@@ -1 +1 @@
-/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
 ,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233
+/tomcat/trunk:1156115,1156171,1156276,1156304,1156519,1156530,1156602,1157015,1157018,1157151,1157198,1157204,1157810,1157832,1157834,1157847,1157908,1157939,1158155,1158160,1158176,1158195,1158198-1158199,1158227,1158331,1158334-1158335,1158426,1160347,1160592,1160611,1160619,1160626,1160639,1160652,1160720-1160721,1160772,1160774,1160776,1161303,1161310,1161322,1161339,1161486,1161540,1161549,1161584,1162082,1162149,1162169,1162721,1162769,1162836,1162932,1163630,1164419,1164438,1164469,1164480,1164567,1165234,1165247-1165248,1165253,1165273,1165282,1165309,1165331,1165338,1165347,1165360-1165361,1165367-1165368,1165602,1165608,1165677,1165693,1165721,1165723,1165728,1165730,1165738,1165746,1165765,1165777,1165918,1165921,1166077,1166150-1166151,1166290,1166366,1166620,1166686,1166752,1166757,1167368,1167394,1169447,1170647,1171692,1172233-1172234,1172236,1172269,1172278,1172282,1172610,1172664,1172689,1172711,1173020-1173021,1173082,1173088,1173090,1173096,1173241,1173256
 ,1173288,1173333,1173342,1173461,1173614,1173630,1173659,1173722,1174061,1174239,1174322,1174325,1174329-1174330,1174337-1174339,1174343,1174353,1174799,1174882,1174884,1174983,1175155,1175158,1175167,1175182,1175190,1175201,1175272,1175275,1175283,1175582,1175589-1175590,1175594,1175602,1175613,1175633,1175690,1175713,1175889,1175896,1175907,1176584,1176590,1176799,1177050,1177060,1177125,1177152,1177160,1177245,1177850,1177862,1177978,1178209,1178228,1178233,1178449

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CometEventImpl.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CometEventImpl.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CometEventImpl.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CometEventImpl.java Mon Oct  3
17:00:47 2011
@@ -93,11 +93,11 @@ public class CometEventImpl implements C
         if (request == null) {
             throw new IllegalStateException(sm.getString("cometEvent.nullRequest"));
         }
-        boolean iscomet = request.isComet();
-        request.setComet(false);
         request.finishRequest();
         response.finishResponse();
-        if (iscomet) request.cometClose();
+        if (request.isComet()) {
+            request.cometClose();
+        }
     }
 
     @Override

Modified: tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/Request.java Mon Oct  3 17:00:47
2011
@@ -2518,6 +2518,7 @@ public class Request
     
     public void cometClose() {
         coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent());
+        setComet(false);
     }
     
     public void setCometTimeout(long timeout) {

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Mon Oct
 3 17:00:47 2011
@@ -723,7 +723,17 @@ public abstract class AbstractHttp11Proc
     @Override
     public final void action(ActionCode actionCode, Object param) {
         
-        if (actionCode == ActionCode.COMMIT) {
+        if (actionCode == ActionCode.CLOSE) {
+            // End the processing of the current request
+
+            try {
+                getOutputBuffer().endRequest();
+            } catch (IOException e) {
+                // Set error flag
+                error = true;
+            }
+
+        } else if (actionCode == ActionCode.COMMIT) {
             // Commit current response
 
             if (response.isCommitted())

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Mon Oct  3
17:00:47 2011
@@ -273,21 +273,7 @@ public class Http11AprProcessor extends 
 
         long socketRef = socket.getSocket().longValue();
         
-        if (actionCode == ActionCode.CLOSE) {
-            // Close
-
-            // End the processing of the current request, and stop any further
-            // transactions with the client
-
-            comet = false;
-            try {
-                outputBuffer.endRequest();
-            } catch (IOException e) {
-                // Set error flag
-                error = true;
-            }
-
-        } else if (actionCode == ActionCode.REQ_HOST_ADDR_ATTRIBUTE) {
+        if (actionCode == ActionCode.REQ_HOST_ADDR_ATTRIBUTE) {
 
             // Get remote host address
             if (remoteAddr == null && (socketRef != 0)) {

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Mon Oct  3
17:00:47 2011
@@ -95,14 +95,7 @@ public class Http11NioProcessor extends 
      */
     protected NioEndpoint.SendfileData sendfileData = null;
 
-    /**
-     * Closed flag, a Comet async thread can 
-     * signal for this Nio processor to be closed and recycled instead
-     * of waiting for a timeout.
-     * Closed by HttpServletResponse.getWriter().close()
-     */
-    protected boolean cometClose = false;
-    
+
     /**
      * Socket associated with the current connection.
      */
@@ -289,7 +282,6 @@ public class Http11NioProcessor extends 
     @Override
     public void recycleInternal() {
         socket = null;
-        cometClose = false;
         comet = false;
         sendfileData = null;
     }
@@ -307,33 +299,7 @@ public class Http11NioProcessor extends 
     @Override
     public void actionInternal(ActionCode actionCode, Object param) {
 
-        if (actionCode == ActionCode.CLOSE) {
-            // Close
-            // End the processing of the current request, and stop any further
-            // transactions with the client
-
-            comet = false;
-            cometClose = true;
-            SelectionKey key = socket.getSocket().getIOChannel().keyFor(socket.getSocket().getPoller().getSelector());
-            if ( key != null ) {
-                NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment) key.attachment();
-                if ( attach!=null && attach.getComet()) {
-                    //if this is a comet connection
-                    //then execute the connection closure at the next selector loop
-                    //request.getAttributes().remove("org.apache.tomcat.comet.timeout");
-                    //attach.setTimeout(5000); //force a cleanup in 5 seconds
-                    //attach.setError(true); //this has caused concurrency errors
-                }
-            }
-
-            try {
-                outputBuffer.endRequest();
-            } catch (IOException e) {
-                // Set error flag
-                error = true;
-            }
-
-        } else if (actionCode == ActionCode.REQ_HOST_ADDR_ATTRIBUTE) {
+        if (actionCode == ActionCode.REQ_HOST_ADDR_ATTRIBUTE) {
 
             // Get remote host address
             if ((remoteAddr == null) && (socket != null)) {
@@ -470,10 +436,13 @@ public class Http11NioProcessor extends 
             if (socket==null || socket.getSocket().getAttachment(false)==null) return;
             NioEndpoint.KeyAttachment attach = (NioEndpoint.KeyAttachment)socket.getSocket().getAttachment(false);
             attach.setCometOps(NioEndpoint.OP_CALLBACK);
-            //notify poller if not on a tomcat thread
             RequestInfo rp = request.getRequestProcessor();
-            if ( rp.getStage() != org.apache.coyote.Constants.STAGE_SERVICE ) //async handling
+            if (rp.getStage() != org.apache.coyote.Constants.STAGE_SERVICE) {
+                // Close event for this processor triggered by request
+                // processing in another processor, a non-Tomcat thread (i.e.
+                // an application controlled thread) or similar.
                 socket.getSocket().getPoller().add(socket.getSocket());
+            }
         } else if (actionCode == ActionCode.COMET_SETTIMEOUT) {
             if (param==null) return;
             if (socket==null || socket.getSocket().getAttachment(false)==null) return;

Modified: tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/Http11Processor.java Mon Oct  3 17:00:47
2011
@@ -244,19 +244,7 @@ public class Http11Processor extends Abs
     @Override
     public void actionInternal(ActionCode actionCode, Object param) {
 
-        if (actionCode == ActionCode.CLOSE) {
-            // Close
-            // End the processing of the current request, and stop any further
-            // transactions with the client
-
-            try {
-                outputBuffer.endRequest();
-            } catch (IOException e) {
-                // Set error flag
-                error = true;
-            }
-
-        } else if (actionCode == ActionCode.REQ_SSL_ATTRIBUTE ) {
+        if (actionCode == ActionCode.REQ_SSL_ATTRIBUTE ) {
 
             try {
                 if (sslSupport != null) {

Modified: tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java?rev=1178456&r1=1178455&r2=1178456&view=diff
==============================================================================
--- tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java (original)
+++ tomcat/tc7.0.x/trunk/test/org/apache/catalina/comet/TestCometProcessor.java Mon Oct  3
17:00:47 2011
@@ -31,17 +31,90 @@ import javax.servlet.http.HttpSession;
 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.Test;
 
 import org.apache.catalina.Context;
 import org.apache.catalina.comet.CometEvent.EventType;
+import org.apache.catalina.connector.CometEventImpl;
+import org.apache.catalina.connector.Request;
+import org.apache.catalina.connector.Response;
 import org.apache.catalina.startup.Tomcat;
 import org.apache.catalina.startup.TomcatBaseTest;
+import org.apache.catalina.valves.ValveBase;
 
 public class TestCometProcessor extends TomcatBaseTest {
 
     @Test
+    public void testAsyncClose() throws Exception {
+        
+        if (!isCometSupported()) {
+            return;
+        }
+
+        // Setup Tomcat instance
+        Tomcat tomcat = getTomcatInstance();
+        Context root = tomcat.addContext("", TEMP_DIR);
+        Tomcat.addServlet(root, "comet", new SimpleCometServlet());
+        root.addServletMapping("/comet", "comet");
+        Tomcat.addServlet(root, "hello", new HelloWorldServlet());
+        root.addServletMapping("/hello", "hello");
+        root.getPipeline().addValve(new AsyncCometCloseValve());
+        tomcat.getConnector().setProperty("connectionTimeout", "5000");
+        tomcat.start();
+        
+        // Create connection to Comet servlet
+        final Socket socket =
+            SocketFactory.getDefault().createSocket("localhost", getPort());
+        socket.setSoTimeout(5000);
+        
+        final OutputStream os = socket.getOutputStream();
+        String requestLine = "POST http://localhost:" + getPort() +
+                "/comet HTTP/1.1\r\n";
+        os.write(requestLine.getBytes());
+        os.write("transfer-encoding: chunked\r\n".getBytes());
+        os.write("\r\n".getBytes());
+
+        InputStream is = socket.getInputStream();
+        ResponseReaderThread readThread = new ResponseReaderThread(is);
+        readThread.start();
+        
+        // Wait for the comet request/response to finish
+        int count = 0;
+        while (count < 10 && !readThread.getResponse().endsWith("0\r\n\r\n"))
{
+            Thread.sleep(500);
+            count++;
+        }
+        
+        if (count == 10) {
+            fail("Comet request did not complete");
+        }
+        
+        // Send a standard HTTP request on the same connection
+        requestLine = "GET http://localhost:" + getPort() +
+                "/hello HTTP/1.1\r\n";
+        os.write(requestLine.getBytes());
+        os.write("\r\n".getBytes());
+        
+        // Check for the expected response
+        count = 0;
+        while (count < 10 && !readThread.getResponse().contains(
+                HelloWorldServlet.RESPONSE_TEXT)) {
+            Thread.sleep(500);
+            count++;
+        }
+
+        if (count == 10) {
+            fail("Non-comet request did not complete");
+        }
+
+        readThread.join();
+        os.close();
+        is.close();
+    }
+
+    @Test
     public void testSimpleCometClient() throws Exception {
         
         if (!isCometSupported()) {
@@ -286,4 +359,41 @@ public class TestCometProcessor extends 
             }
         }
     }
+
+    private static class AsyncCometCloseValve extends ValveBase {
+
+        @Override
+        public void invoke(Request request, Response response)
+                throws IOException, ServletException {
+            
+            CometEventImpl event = new CometEventImpl(request, response);
+            
+            getNext().invoke(request, response);
+            
+            if (request.isComet()) {
+                Thread t = new AsyncCometCloseThread(event);
+                t.start();
+            }
+        }
+    }
+    
+    private static class AsyncCometCloseThread extends Thread {
+
+        private CometEvent event;
+        
+        public AsyncCometCloseThread(CometEvent event) {
+            this.event = event;
+        }
+        
+        @Override
+        public void run() {
+            try {
+                Thread.sleep(2000);
+                event.close();
+            } catch (Exception e) {
+                // Test should fail. Report what went wrong.
+                e.printStackTrace();
+            }
+        }
+    }
 }



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


Mime
View raw message