Author: kkolinko Date: Sat Aug 11 21:59:31 2012 New Revision: 1372035 URL: http://svn.apache.org/viewvc?rev=1372035&view=rev Log: Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52858 Fix high CPU load with SSL, NIO and sendfile when client breaks the connection before reading all the requested data. Avoids BZ 53138. Backport of r1300948, r1340218, r1342468
Modified: tomcat/tc6.0.x/trunk/STATUS.txt tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Modified: tomcat/tc6.0.x/trunk/STATUS.txt URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/STATUS.txt?rev=1372035&r1=1372034&r2=1372035&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/STATUS.txt (original) +++ tomcat/tc6.0.x/trunk/STATUS.txt Sat Aug 11 21:59:31 2012 @@ -80,25 +80,6 @@ PATCHES PROPOSED TO BACKPORT: request parsing, so only broken webapps may break, not broken clients. -1: -* Fix bug https://issues.apache.org/bugzilla/show_bug.cgi?id=52858 - http://svn.apache.org/viewvc?rev=1300948&view=rev - +1: fhanik, markt, rjung - -1: - rjung: Isn't this only fixing the regression introduced by 52858 (BZ 53138) - but 52858 will be again unfixed? - -1: kkolinko: unless r1340218 is backported as well (I agree with rjung's - concern). Proposed below. - - Additional patch: - kkolinko: Regarding r1340218: Note, that "reg" argument in all existing - calls to processSendfile() is always true. The actual change in this - revision is registering for OP_WRITE regardless of the value of - attachment.interestOps() - http://svn.apache.org/viewvc?view=revision&revision=1340218 (fix BZ 53138) - http://svn.apache.org/viewvc?view=revision&revision=1342468 (cleanup) - +1: kkolinko, markt, schultz - -1: - * Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=52918 Add WebSocket support to Tomcat 6 +1: fhanik Modified: tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1372035&r1=1372034&r2=1372035&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original) +++ tomcat/tc6.0.x/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Sat Aug 11 21:59:31 2012 @@ -1713,8 +1713,14 @@ public class NioEndpoint extends Abstrac public boolean processSendfile(SelectionKey sk, KeyAttachment attachment, boolean reg, boolean event) { NioChannel sc = null; try { - //unreg(sk,attachment);//only do this if we do process send file on a separate thread + unreg(sk, attachment, sk.readyOps()); SendfileData sd = attachment.getSendfileData(); + + if (log.isTraceEnabled()) { + log.trace("Processing send file for: " + sd.fileName); + } + + //setup the file channel if ( sd.fchannel == null ) { File f = new File(sd.fileName); if ( !f.exists() ) { @@ -1723,10 +1729,14 @@ public class NioEndpoint extends Abstrac } sd.fchannel = new FileInputStream(f).getChannel(); } + + //configure output channel sc = attachment.getChannel(); sc.setSendFile(true); + //ssl channel is slightly different WritableByteChannel wc =(WritableByteChannel) ((sc instanceof SecureNioChannel)?sc:sc.getIOChannel()); - + + //we still have data in the buffer if (sc.getOutboundRemaining()>0) { if (sc.flushOutbound()) { attachment.access(); @@ -1753,15 +1763,13 @@ public class NioEndpoint extends Abstrac attachment.setSendfileData(null); try {sd.fchannel.close();}catch(Exception ignore){} if ( sd.keepAlive ) { - if (reg) { - if (log.isDebugEnabled()) { - log.debug("Connection is keep alive, registering back for OP_READ"); - } - if (event) { - this.add(attachment.getChannel(),SelectionKey.OP_READ); - } else { - reg(sk,attachment,SelectionKey.OP_READ); - } + if (log.isDebugEnabled()) { + log.debug("Connection is keep alive, registering back for OP_READ"); + } + if (event) { + this.add(attachment.getChannel(),SelectionKey.OP_READ); + } else { + reg(sk,attachment,SelectionKey.OP_READ); } } else { if (log.isDebugEnabled()) { @@ -1770,9 +1778,9 @@ public class NioEndpoint extends Abstrac cancelledKey(sk,SocketStatus.STOP,false); return false; } - } else if ( attachment.interestOps() == 0 && reg ) { + } else { if (log.isDebugEnabled()) { - log.debug("OP_WRITE for sendilfe:"+sd.fileName); + log.debug("OP_WRITE for sendfile:" + sd.fileName); } if (event) { add(attachment.getChannel(),SelectionKey.OP_WRITE); Modified: tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml URL: http://svn.apache.org/viewvc/tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml?rev=1372035&r1=1372034&r2=1372035&view=diff ============================================================================== --- tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml (original) +++ tomcat/tc6.0.x/trunk/webapps/docs/changelog.xml Sat Aug 11 21:59:31 2012 @@ -198,6 +198,11 @@ AJP. (markt) </fix> <fix> + <bug>52858</bug>: Fix high CPU load with SSL, NIO and sendfile when + client breaks the connection before reading all the requested data. + (fhanik/kkolinko) + </fix> + <fix> <bug>53119</bug>: Prevent buffer overflow errors being reported when a client disconnects before the response has been fully written from an AJP connection using the APR/native connector. (kkolinko) --------------------------------------------------------------------- To unsubscribe, e-mail: dev-unsubscr...@tomcat.apache.org For additional commands, e-mail: dev-h...@tomcat.apache.org