2014-06-14 0:21 GMT+04:00 <ma...@apache.org>: > Author: markt > Date: Fri Jun 13 20:21:32 2014 > New Revision: 1602510 > > URL: http://svn.apache.org/r1602510 > Log: > Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=56518 > Do not attempt an NIO write if a thread has been interrupted as it can lead > to a connection limit leak > > Modified: > tomcat/tc7.0.x/trunk/ (props changed) > tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/AbstractProcessor.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/Adapter.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/LocalStrings.properties > tomcat/tc7.0.x/trunk/java/org/apache/coyote/Processor.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AbstractAjpProcessor.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpAprProcessor.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpNioProcessor.java > tomcat/tc7.0.x/trunk/java/org/apache/coyote/ajp/AjpProcessor.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/upgrade/AbstractProcessor.java > > tomcat/tc7.0.x/trunk/java/org/apache/coyote/http11/upgrade/UpgradeProcessor.java > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/AbstractEndpoint.java > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioChannel.java > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SecureNioChannel.java > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/SocketStatus.java > > tomcat/tc7.0.x/trunk/java/org/apache/tomcat/util/net/res/LocalStrings.properties > tomcat/tc7.0.x/trunk/webapps/docs/changelog.xml > > Propchange: tomcat/tc7.0.x/trunk/ > ------------------------------------------------------------------------------ > Merged /tomcat/trunk:r1602198 > > Modified: > tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > URL: > http://svn.apache.org/viewvc/tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java?rev=1602510&r1=1602509&r2=1602510&view=diff > ============================================================================== > --- > tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > (original) > +++ > tomcat/tc7.0.x/trunk/java/org/apache/catalina/connector/CoyoteAdapter.java > Fri Jun 13 20:21:32 2014 > @@ -469,6 +469,34 @@ public class CoyoteAdapter implements Ad > > > @Override > + public void errorDispatch(org.apache.coyote.Request req, > + org.apache.coyote.Response res) { > + Request request = (Request) req.getNote(ADAPTER_NOTES); > + Response response = (Response) res.getNote(ADAPTER_NOTES); > + > + if (request != null && request.getMappingData().context != null) { > + ((Context) request.getMappingData().context).logAccess( > + request, response, > + System.currentTimeMillis() - req.getStartTime(), > + false); > + } else { > + log(req, res, System.currentTimeMillis() - req.getStartTime()); > + } > + > + if (request != null) { > + request.recycle(); > + } > + > + if (response != null) { > + response.recycle(); > + } > + > + res.recycle(); > + res.recycle();
I fixed the typo by r1602852. > + } In short, my summary on how this works (on example of NIO) on Tomcat 7 codebase is as following In AbstractProcessor.setErrorState it now does [[[ getEndpoint().processSocketAsync(socketWrapper, SocketStatus.CLOSE_NOW); ]]] That goes (for example) into NioEndpoint.processSocketAsync(...) and into NioEndpoint.processSocket(...) where it creates a new SocketProcessor(socket, status) and submits it to an Executor. NioEndpoint$SocketProcessor.run() obtains monitor on the socket (by "synchronized (socket)"), calls processor (by "handler.process(ka, status)"), receives SocketState.CLOSED from the call and counts down the connection (done in getPoller().cancelledKey()). The method "handler.process()" that is called there is AbstractProtocol$AbstractConnectionHandler.process(SocketWrapper, SocketStatus). In Tomcat 8 it gets a processor for this socket (if there is one associated with it), calls "processor.errorDispatch(...);" (the new "CoyoteAdapter. errorDispatch(...)" method above) and recycles the processor. I have several questions. 1) The AbstractProtocol$AbstractConnectionHandler.process() mentioned above is from Tomcat 8. The code for handling SocketStatus.CLOSE_NOW there is missing in backport to Tomcat 7. In Tomcat 7 the SocketStatus.CLOSE_NOW enum value is used only in 1 place, where in Tomcat 8 it is used in 2 places.. 2) The submission of SocketProcessor to the Executor may end with RejectedExecutionException. - in NioEndpoint#processSocket(NioChannel, SocketStatus, boolean) In that case it just logs a warning and does nothing else. How likely is this rejected execution? My understanding/answer is that the rejection is unlikely. The executor here is o.a.c.core.StandardThreadExecutor. Its execute() method is unlikely to reject tasks. There is a queue that accepts tasks. Tasks will be rejected if either a) Executor is not running. b) Queue is full. The maximum queue length is Integer.MAX_VALUE by default. Though its size is configurable via StandardThreadExecutor.setMaxQueueSize(). Will it result in failing to decrement the connection counter? Will it result in failing to obtain and to recycle the processor associated with the socket? I suspect that per this BZ 56518 it will fail to decrement the counter. 3) In case if there are several executor threads that process the socket at the same time, what happens? My understanding/answer is that "synchronized (socket)" in NioEndpoint$SocketProcessor.run() handles that. In this case I see that NioEndpoint$SocketProcessor.doRun() does "nioChannels.offer(socket)" That is it offers the "socket" for reuse. Thus "synchronized (socket)" is broken, as the "socket" can be recycled and reused by another thread while the second one waits to obtain the monitor on the "socket". 4) There are no tests for this new feature. Best regards, Konstantin Kolinko --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org
