From: markt Date: Mon, 3 Oct 2011 16:42:19 +0000 (+0000) Subject: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=51881 X-Git-Url: https://git.internetallee.de/?a=commitdiff_plain;h=fb2fb4880f348d1711adef435fdd2f1bf0936ced;p=tomcat7.0 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. git-svn-id: https://svn.apache.org/repos/asf/tomcat/trunk@1178449 13f79535-47bb-0310-9956-ffa450edef68 --- diff --git a/java/org/apache/catalina/connector/CometEventImpl.java b/java/org/apache/catalina/connector/CometEventImpl.java index 34e9f20cb..6b35139c4 100644 --- a/java/org/apache/catalina/connector/CometEventImpl.java +++ b/java/org/apache/catalina/connector/CometEventImpl.java @@ -93,11 +93,11 @@ public class CometEventImpl implements CometEvent { 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 diff --git a/java/org/apache/catalina/connector/Request.java b/java/org/apache/catalina/connector/Request.java index 87ce69009..5be7ef421 100644 --- a/java/org/apache/catalina/connector/Request.java +++ b/java/org/apache/catalina/connector/Request.java @@ -2514,6 +2514,7 @@ public class Request public void cometClose() { coyoteRequest.action(ActionCode.COMET_CLOSE,getEvent()); + setComet(false); } public void setCometTimeout(long timeout) { diff --git a/java/org/apache/coyote/http11/AbstractHttp11Processor.java b/java/org/apache/coyote/http11/AbstractHttp11Processor.java index cc7a8231e..a5def0a74 100644 --- a/java/org/apache/coyote/http11/AbstractHttp11Processor.java +++ b/java/org/apache/coyote/http11/AbstractHttp11Processor.java @@ -723,7 +723,17 @@ public abstract class AbstractHttp11Processor extends AbstractProcessor { @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()) diff --git a/java/org/apache/coyote/http11/Http11AprProcessor.java b/java/org/apache/coyote/http11/Http11AprProcessor.java index 9918980b0..c8815b451 100644 --- a/java/org/apache/coyote/http11/Http11AprProcessor.java +++ b/java/org/apache/coyote/http11/Http11AprProcessor.java @@ -273,21 +273,7 @@ public class Http11AprProcessor extends AbstractHttp11Processor { 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)) { diff --git a/java/org/apache/coyote/http11/Http11NioProcessor.java b/java/org/apache/coyote/http11/Http11NioProcessor.java index 3678d5596..f645f14ed 100644 --- a/java/org/apache/coyote/http11/Http11NioProcessor.java +++ b/java/org/apache/coyote/http11/Http11NioProcessor.java @@ -95,14 +95,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { */ 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 AbstractHttp11Processor { @Override public void recycleInternal() { socket = null; - cometClose = false; comet = false; sendfileData = null; } @@ -307,22 +299,7 @@ public class Http11NioProcessor extends AbstractHttp11Processor { @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 AbstractHttp11Processor { 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; diff --git a/java/org/apache/coyote/http11/Http11Processor.java b/java/org/apache/coyote/http11/Http11Processor.java index e1572847d..dfbbac2d3 100644 --- a/java/org/apache/coyote/http11/Http11Processor.java +++ b/java/org/apache/coyote/http11/Http11Processor.java @@ -244,19 +244,7 @@ public class Http11Processor extends AbstractHttp11Processor { @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) { diff --git a/test/org/apache/catalina/comet/TestCometProcessor.java b/test/org/apache/catalina/comet/TestCometProcessor.java index c6c6105ea..baa4fd0ce 100644 --- a/test/org/apache/catalina/comet/TestCometProcessor.java +++ b/test/org/apache/catalina/comet/TestCometProcessor.java @@ -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 TomcatBaseTest { } } } + + 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(); + } + } + } }