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