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 2022/06/28 19:37:52 UTC

[tomcat] branch 10.0.x updated (577d9f0a3d -> 4bd90299ab)

This is an automated email from the ASF dual-hosted git repository.

markt pushed a change to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git


    from 577d9f0a3d Fix potential (not observed) concurrency issues
     new 2426f4881a Fix duplicate Poller registration with HTTP/2, NIO and async IO
     new e3cb239c5d Only need to check socket/network buffers for writes
     new 4bd90299ab Fix logic for sending HTTP/2 pings

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


Summary of changes:
 java/org/apache/coyote/AbstractProtocol.java          |  5 ++++-
 java/org/apache/coyote/http2/Http2UpgradeHandler.java | 16 +++++++++++-----
 java/org/apache/tomcat/util/net/AbstractEndpoint.java |  2 +-
 java/org/apache/tomcat/util/net/NioEndpoint.java      |  4 ++--
 webapps/docs/changelog.xml                            |  4 ++++
 5 files changed, 22 insertions(+), 9 deletions(-)


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


[tomcat] 03/03: Fix logic for sending HTTP/2 pings

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 4bd90299ab2a6aa4a73d1fb9904b942e3a3f9e5c
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 28 19:52:30 2022 +0100

    Fix logic for sending HTTP/2 pings
---
 java/org/apache/coyote/http2/Http2UpgradeHandler.java | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index fed4383972..de33bfded6 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -327,14 +327,13 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
         // Might not be necessary. init() will handle that.
         init(null);
 
-
         SocketState result = SocketState.CLOSED;
 
         try {
             switch(status) {
             case OPEN_READ:
                 synchronized (socketWrapper) {
-                    if (!socketWrapper.canWrite()) {
+                    if (socketWrapper.canWrite()) {
                         // Only send a ping if there is no other data waiting to be sent.
                         // Ping manager will ensure they aren't sent too frequently.
                         pingManager.sendPing(false);


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


[tomcat] 01/03: Fix duplicate Poller registration with HTTP/2, NIO and async IO

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit 2426f4881a7de32e50ceaf8ec97f8c232ae6e597
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 28 20:31:41 2022 +0100

    Fix duplicate Poller registration with HTTP/2, NIO and async IO
    
    This could cause HTTP/2 connections to unexpectedly fail
---
 java/org/apache/coyote/AbstractProtocol.java          |  5 ++++-
 java/org/apache/coyote/http2/Http2UpgradeHandler.java | 13 ++++++++++---
 java/org/apache/tomcat/util/net/AbstractEndpoint.java |  2 +-
 webapps/docs/changelog.xml                            |  4 ++++
 4 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/java/org/apache/coyote/AbstractProtocol.java b/java/org/apache/coyote/AbstractProtocol.java
index a3331c175a..09367766d6 100644
--- a/java/org/apache/coyote/AbstractProtocol.java
+++ b/java/org/apache/coyote/AbstractProtocol.java
@@ -919,7 +919,7 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
                             if (httpUpgradeHandler instanceof InternalHttpUpgradeHandler) {
                                 if (((InternalHttpUpgradeHandler) httpUpgradeHandler).hasAsyncIO()) {
                                     // The handler will initiate all further I/O
-                                    state = SocketState.UPGRADED;
+                                    state = SocketState.ASYNC_IO;
                                 }
                             }
                         }
@@ -955,6 +955,9 @@ public abstract class AbstractProtocol<S> implements ProtocolHandler,
                         longPoll(wrapper, processor);
                         getProtocol().addWaitingProcessor(processor);
                     }
+                } else if (state == SocketState.ASYNC_IO) {
+                    // Don't add sockets back to the poller.
+                    // The handler will initiate all further I/O
                 } else if (state == SocketState.SUSPENDED) {
                     // Don't add sockets back to the poller.
                     // The resumeProcessing() method will add this socket
diff --git a/java/org/apache/coyote/http2/Http2UpgradeHandler.java b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
index 8b7ec85cc5..fed4383972 100644
--- a/java/org/apache/coyote/http2/Http2UpgradeHandler.java
+++ b/java/org/apache/coyote/http2/Http2UpgradeHandler.java
@@ -388,14 +388,21 @@ class Http2UpgradeHandler extends AbstractStream implements InternalHttpUpgradeH
                 }
 
                 if (connectionState.get() != ConnectionState.CLOSED) {
-                    result = SocketState.UPGRADED;
+                    if (socketWrapper.hasAsyncIO()) {
+                        result = SocketState.ASYNC_IO;
+                    } else {
+                        result = SocketState.UPGRADED;
+                    }
                 }
                 break;
 
             case OPEN_WRITE:
                 processWrites();
-
-                result = SocketState.UPGRADED;
+                if (socketWrapper.hasAsyncIO()) {
+                    result = SocketState.ASYNC_IO;
+                } else {
+                    result = SocketState.UPGRADED;
+                }
                 break;
 
             case TIMEOUT:
diff --git a/java/org/apache/tomcat/util/net/AbstractEndpoint.java b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
index 572901a9fb..d058703953 100644
--- a/java/org/apache/tomcat/util/net/AbstractEndpoint.java
+++ b/java/org/apache/tomcat/util/net/AbstractEndpoint.java
@@ -77,7 +77,7 @@ public abstract class AbstractEndpoint<S,U> {
         public enum SocketState {
             // TODO Add a new state to the AsyncStateMachine and remove
             //      ASYNC_END (if possible)
-            OPEN, CLOSED, LONG, ASYNC_END, SENDFILE, UPGRADING, UPGRADED, SUSPENDED
+            OPEN, CLOSED, LONG, ASYNC_END, SENDFILE, UPGRADING, UPGRADED, ASYNC_IO, SUSPENDED
         }
 
 
diff --git a/webapps/docs/changelog.xml b/webapps/docs/changelog.xml
index 03eca51763..3943cd54ff 100644
--- a/webapps/docs/changelog.xml
+++ b/webapps/docs/changelog.xml
@@ -132,6 +132,10 @@
         default value for the <code>jmvRoute</code> attribute of an Engine.
         (markt)
       </scode>
+      <fix>
+        Fix duplicate Poller registration with HTTP/2, NIO and async IO that
+        could cause HTTP/2 connections to unexpectedly fail. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Jasper">


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


[tomcat] 02/03: Only need to check socket/network buffers for writes

Posted by ma...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

markt pushed a commit to branch 10.0.x
in repository https://gitbox.apache.org/repos/asf/tomcat.git

commit e3cb239c5d669475dbc94dc23e9c74467a88be04
Author: Mark Thomas <ma...@apache.org>
AuthorDate: Tue Jun 28 19:51:28 2022 +0100

    Only need to check socket/network buffers for writes
---
 java/org/apache/tomcat/util/net/NioEndpoint.java | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/java/org/apache/tomcat/util/net/NioEndpoint.java b/java/org/apache/tomcat/util/net/NioEndpoint.java
index 041f2d4be1..ddcdcd39ab 100644
--- a/java/org/apache/tomcat/util/net/NioEndpoint.java
+++ b/java/org/apache/tomcat/util/net/NioEndpoint.java
@@ -1661,7 +1661,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                                 }
                             }
                             if (nBytes != 0 || (!buffersArrayHasRemaining(buffers, offset, length) &&
-                                    !socketOrNetworkBufferHasDataLeft())) {
+                                    (read || !socketOrNetworkBufferHasDataLeft()))) {
                                 completionDone = false;
                             }
                         }
@@ -1670,7 +1670,7 @@ public class NioEndpoint extends AbstractJsseEndpoint<NioChannel,SocketChannel>
                     }
                 }
                 if (nBytes > 0 || (nBytes == 0 && !buffersArrayHasRemaining(buffers, offset, length) &&
-                        !socketOrNetworkBufferHasDataLeft())) {
+                        (read || !socketOrNetworkBufferHasDataLeft()))) {
                     // The bytes processed are only updated in the completion handler
                     completion.completed(Long.valueOf(nBytes), this);
                 } else if (nBytes < 0 || getError() != null) {


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