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 2013/10/07 22:44:04 UTC

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

Author: markt
Date: Mon Oct  7 20:44:04 2013
New Revision: 1530057

URL: http://svn.apache.org/r1530057
Log:
Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55633 for NIO.
The Comet code that ensured that multiple threads didn't process the same socket when the selector indicated that a socket was ready for read and write pre-dated r1001698 where syncs where added to the SocketProcessor to achieve the same aim for Servlet 3.0 asyncs processing.
The Comet code was re-used to handle upgraded connections.
The upgrade code did not handle the case where a socket was registered for read and write but only a write event occurred. In this case the read registration was lost. This is the root cause of the lack of responsiveness observed in bug 55633.
With the changes in r1001698, a simpler solution can be used for both
Comet and HTTP upgrade. The new approach unregisters the socket operations the selector has reported ready for and then triggers a read and/or write as appropriate. For Comet the syncs will ensure that read and write aren't processed in parallel. For HTTP upgrade such parallel processing is permitted.

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=1530057&r1=1530056&r2=1530057&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java (original)
+++ tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java Mon Oct  7 20:44:04 2013
@@ -1155,44 +1155,19 @@ public class NioEndpoint extends Abstrac
                     if (sk.isReadable() || sk.isWritable() ) {
                         if ( attachment.getSendfileData() != null ) {
                             processSendfile(sk,attachment, false);
-                        } else if ( attachment.isComet() ) {
-                            //check if thread is available
-                            if ( isWorkerAvailable() ) {
-                                //set interest ops to 0 so we don't get multiple
-                                //Invocations for both read and write on separate threads
-                                reg(sk, attachment, 0);
-                                //read goes before write
-                                if (sk.isReadable()) {
-                                    //read notification
-                                    if (!processSocket(channel, SocketStatus.OPEN_READ, true))
-                                        processSocket(channel, SocketStatus.DISCONNECT, true);
-                                } else {
-                                    //future placement of a WRITE notif
-                                    if (!processSocket(channel, SocketStatus.OPEN_WRITE, true))
-                                        processSocket(channel, SocketStatus.DISCONNECT, true);
-                                }
-                            } else {
-                                result = false;
-                            }
                         } else {
-                            //later on, improve latch behavior
                             if ( isWorkerAvailable() ) {
-
-                                boolean readAndWrite = sk.isReadable() && sk.isWritable();
-                                reg(sk, attachment, 0);
-                                if (attachment.isAsync() && readAndWrite) {
-                                    //remember the that we want to know about write too
-                                    attachment.interestOps(SelectionKey.OP_WRITE);
-                                }
-                                //read goes before write
+                                unreg(sk, attachment, sk.readyOps());
+                                // Read goes before write
                                 if (sk.isReadable()) {
-                                    //read notification
-                                    if (!processSocket(channel, SocketStatus.OPEN_READ, true))
+                                    if (!processSocket(channel, SocketStatus.OPEN_READ, true)) {
                                         close = true;
-                                } else {
-                                    //future placement of a WRITE notif
-                                    if (!processSocket(channel, SocketStatus.OPEN_WRITE, true))
+                                    }
+                                }
+                                if (!close && sk.isWritable()) {
+                                    if (!processSocket(channel, SocketStatus.OPEN_WRITE, true)) {
                                         close = true;
+                                    }
                                 }
                                 if (close) {
                                     cancelledKey(sk,SocketStatus.DISCONNECT);



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


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

Posted by Mark Thomas <ma...@apache.org>.
On 07/10/2013 22:29, Konstantin Preißer wrote:
> Mark,
> 
>> -----Original Message-----
>> From: markt@apache.org [mailto:markt@apache.org]
>> Sent: Monday, October 7, 2013 10:44 PM
>> To: dev@tomcat.apache.org
>> Subject: svn commit: r1530057 -
>> /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
>>
>> Author: markt
>> Date: Mon Oct  7 20:44:04 2013
>> New Revision: 1530057
>>
>> URL: http://svn.apache.org/r1530057
>> Log:
>> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55633 for NIO.
>> The Comet code that ensured that multiple threads didn't process the same
>> socket when the selector indicated that a socket was ready for read and
>> write pre-dated r1001698 where syncs where added to the SocketProcessor
>> to achieve the same aim for Servlet 3.0 asyncs processing.
>> The Comet code was re-used to handle upgraded connections.
>> The upgrade code did not handle the case where a socket was registered for
>> read and write but only a write event occurred. In this case the read
>> registration was lost. This is the root cause of the lack of responsiveness
>> observed in bug 55633.
>> With the changes in r1001698, a simpler solution can be used for both
>> Comet and HTTP upgrade. The new approach unregisters the socket
>> operations the selector has reported ready for and then triggers a read
>> and/or write as appropriate. For Comet the syncs will ensure that read and
>> write aren't processed in parallel. For HTTP upgrade such parallel processing
>> is permitted.
> 
> Thank you very much. I can confirm that for NIO the behavior described in the report is fixed - Tomcat will correctly call the OnMessage and OnClose methods.
> 
> I noticed that there still seems to be an issue when the connection is aborted before the data is read:
> 1) Follow steps 1-4 from bugzilla 55633
> 2) Instead of resuming the firefox.exe, kill the process.
> 3) On IE/other browser, the snakes will stand still for ~ 15 seconds, then continue to move - but the snake from the killed firefox is still visible.

This should be fixed too now.

Mark


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


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

Posted by Konstantin Preißer <kp...@apache.org>.
Mark,

> -----Original Message-----
> From: markt@apache.org [mailto:markt@apache.org]
> Sent: Monday, October 7, 2013 10:44 PM
> To: dev@tomcat.apache.org
> Subject: svn commit: r1530057 -
> /tomcat/trunk/java/org/apache/tomcat/util/net/NioEndpoint.java
> 
> Author: markt
> Date: Mon Oct  7 20:44:04 2013
> New Revision: 1530057
> 
> URL: http://svn.apache.org/r1530057
> Log:
> Fix https://issues.apache.org/bugzilla/show_bug.cgi?id=55633 for NIO.
> The Comet code that ensured that multiple threads didn't process the same
> socket when the selector indicated that a socket was ready for read and
> write pre-dated r1001698 where syncs where added to the SocketProcessor
> to achieve the same aim for Servlet 3.0 asyncs processing.
> The Comet code was re-used to handle upgraded connections.
> The upgrade code did not handle the case where a socket was registered for
> read and write but only a write event occurred. In this case the read
> registration was lost. This is the root cause of the lack of responsiveness
> observed in bug 55633.
> With the changes in r1001698, a simpler solution can be used for both
> Comet and HTTP upgrade. The new approach unregisters the socket
> operations the selector has reported ready for and then triggers a read
> and/or write as appropriate. For Comet the syncs will ensure that read and
> write aren't processed in parallel. For HTTP upgrade such parallel processing
> is permitted.

Thank you very much. I can confirm that for NIO the behavior described in the report is fixed - Tomcat will correctly call the OnMessage and OnClose methods.

I noticed that there still seems to be an issue when the connection is aborted before the data is read:
1) Follow steps 1-4 from bugzilla 55633
2) Instead of resuming the firefox.exe, kill the process.
3) On IE/other browser, the snakes will stand still for ~ 15 seconds, then continue to move - but the snake from the killed firefox is still visible.


Regards,
Konstantin Preißer


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