You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by ma...@apache.org on 2011/10/03 18:42:20 UTC
svn commit: r1178449 - in /tomcat/trunk: java/org/apache/catalina/connector/
java/org/apache/coyote/http11/ test/org/apache/catalina/comet/
Author: markt
Date: Mon Oct 3 16:42:19 2011
New Revision: 1178449
URL: http://svn.apache.org/viewvc?rev=1178449&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/trunk/java/org/apache/catalina/connector/CometEventImpl.java
tomcat/trunk/java/org/apache/catalina/connector/Request.java
tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java
tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
Modified: tomcat/trunk/java/org/apache/catalina/connector/CometEventImpl.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/CometEventImpl.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/CometEventImpl.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/CometEventImpl.java Mon Oct 3 16:42:19 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/trunk/java/org/apache/catalina/connector/Request.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/Request.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/Request.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/Request.java Mon Oct 3 16:42:19 2011
@@ -2514,6 +2514,7 @@ public class Request
public void cometClose() {
coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent());
+ setComet(false);
}
public void setCometTimeout(long timeout) {
Modified: tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/AbstractHttp11Processor.java Mon Oct 3 16:42:19 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/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11AprProcessor.java Mon Oct 3 16:42:19 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/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11NioProcessor.java Mon Oct 3 16:42:19 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,22 +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;
-
- 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)) {
@@ -459,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/trunk/java/org/apache/coyote/http11/Http11Processor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http11/Http11Processor.java Mon Oct 3 16:42:19 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/trunk/test/org/apache/catalina/comet/TestCometProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java?rev=1178449&r1=1178448&r2=1178449&view=diff
==============================================================================
--- tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java (original)
+++ tomcat/trunk/test/org/apache/catalina/comet/TestCometProcessor.java Mon Oct 3 16:42:19 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