You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by fh...@apache.org on 2012/05/18 21:23:13 UTC

svn commit: r1340215 - /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java

Author: fhanik
Date: Fri May 18 19:23:12 2012
New Revision: 1340215

URL: http://svn.apache.org/viewvc?rev=1340215&view=rev
Log:
https://issues.apache.org/bugzilla/show_bug.cgi?id=52858
https://issues.apache.org/bugzilla/show_bug.cgi?id=53138
Fix broken NIO sendfile download
Fix high CPU usage due to registered OP_READ for sendfile processes


Modified:
    tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java

Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1340215&r1=1340214&r2=1340215&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Fri May 18 19:23:12 2012
@@ -1278,9 +1278,13 @@ public class NioEndpoint extends Abstrac
 
         public boolean processSendfile(SelectionKey sk, KeyAttachment attachment, boolean reg, boolean event) {
             NioChannel sc = null;
+            if (log.isTraceEnabled()) {
+                log.trace("["+new java.sql.Date(System.currentTimeMillis()).toGMTString()+"] Processing send file. ["+sk+"] ");
+            }
             try {
                 unreg(sk, attachment, sk.readyOps());
                 SendfileData sd = attachment.getSendfileData();
+                //setup the file channel
                 if ( sd.fchannel == null ) {
                     File f = new File(sd.fileName);
                     if ( !f.exists() ) {
@@ -1289,10 +1293,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 = ((sc instanceof SecureNioChannel)?sc:sc.getIOChannel());
 
+                //we still have data in the buffer
                 if (sc.getOutboundRemaining()>0) {
                     if (sc.flushOutbound()) {
                         attachment.access();
@@ -1322,7 +1330,6 @@ public class NioEndpoint extends Abstrac
                     } catch (Exception ignore) {
                     }
                     if ( sd.keepAlive ) {
-                        if (reg) {
                             if (log.isDebugEnabled()) {
                                 log.debug("Connection is keep alive, registering back for OP_READ");
                             }
@@ -1331,7 +1338,6 @@ public class NioEndpoint extends Abstrac
                             } else {
                                 reg(sk,attachment,SelectionKey.OP_READ);
                             }
-                        }
                     } else {
                         if (log.isDebugEnabled()) {
                             log.debug("Send file connection is being closed");
@@ -1339,7 +1345,7 @@ public class NioEndpoint extends Abstrac
                         cancelledKey(sk,SocketStatus.STOP);
                         return false;
                     }
-                } else if ( attachment.interestOps() == 0 && reg ) {
+                } else { //if ( attachment.interestOps() == 0 && reg ) {
                     if (log.isDebugEnabled()) {
                         log.debug("OP_WRITE for sendilfe:"+sd.fileName);
                     }



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1340215 - /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java

Posted by Konstantin Kolinko <kn...@gmail.com>.
2012/5/21 Filip Hanik Mailing Lists <de...@hanik.com>:
>
>
> ----- Mensaje original -----
>> De: "Rainer Jung" <ra...@kippdata.de>
>>>    +            if (log.isTraceEnabled()) {
>>>    +                log.trace("["+new java.sql.Date(System.currentTimeMillis()).toGMTString()+"] Processing send file. ["+sk+"] ");
>>>    +            }
>>
>> Was that intentional? I'd say the timestamp should be provided by the
>> log framework and not by java.sql.Date. But maybe the whole message
>> is
>> just a leftover from debugging the issue.
>>
>> The same for the TC 7 backport.
>>
>
> hi Rainer, no, not intentional, it is left over from debugging. I will remove the Date object
>

Fixed by
http://svn.apache.org/viewvc?view=revision&revision=1342468

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1340215 - /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java

Posted by Filip Hanik Mailing Lists <de...@hanik.com>.

----- Mensaje original -----
> De: "Rainer Jung" <ra...@kippdata.de>
            }
> 
> Was that intentional? I'd say the timestamp should be provided by the
> log framework and not by java.sql.Date. But maybe the whole message
> is
> just a leftover from debugging the issue.
> 
> The same for the TC 7 backport.
> 

hi Rainer, no, not intentional, it is left over from debugging. I will remove the Date object

Filip


> >               try {
> >                   unreg(sk, attachment, sk.readyOps());
> >                   SendfileData sd = attachment.getSendfileData();
> > +                //setup the file channel
> >                   if ( sd.fchannel == null ) {
> >                       File f = new File(sd.fileName);
> >                       if ( !f.exists() ) {
> 
> ...
> 
> Regards,
> 
> Rainer
> 
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
> 
> 

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1340215 - /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Filip,

On 18.05.2012 21:23, fhanik@apache.org wrote:
> Author: fhanik
> Date: Fri May 18 19:23:12 2012
> New Revision: 1340215
>
> URL: http://svn.apache.org/viewvc?rev=1340215&view=rev
> Log:
> https://issues.apache.org/bugzilla/show_bug.cgi?id=52858
> https://issues.apache.org/bugzilla/show_bug.cgi?id=53138
> Fix broken NIO sendfile download
> Fix high CPU usage due to registered OP_READ for sendfile processes
>
>
> Modified:
>      tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
>
> Modified: tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java?rev=1340215&r1=1340214&r2=1340215&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
> +++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Fri May 18 19:23:12 2012
> @@ -1278,9 +1278,13 @@ public class NioEndpoint extends Abstrac
>
>           public boolean processSendfile(SelectionKey sk, KeyAttachment attachment, boolean reg, boolean event) {
>               NioChannel sc = null;
> +            if (log.isTraceEnabled()) {
> +                log.trace("["+new java.sql.Date(System.currentTimeMillis()).toGMTString()+"] Processing send file. ["+sk+"] ");
> +            }

Was that intentional? I'd say the timestamp should be provided by the 
log framework and not by java.sql.Date. But maybe the whole message is 
just a leftover from debugging the issue.

The same for the TC 7 backport.

>               try {
>                   unreg(sk, attachment, sk.readyOps());
>                   SendfileData sd = attachment.getSendfileData();
> +                //setup the file channel
>                   if ( sd.fchannel == null ) {
>                       File f = new File(sd.fileName);
>                       if ( !f.exists() ) {

...

Regards,

Rainer

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org