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 2018/11/13 23:02:31 UTC

svn commit: r1846551 - in /tomcat/trunk: java/org/apache/catalina/connector/ java/org/apache/coyote/ java/org/apache/coyote/http2/ webapps/docs/

Author: markt
Date: Tue Nov 13 23:02:31 2018
New Revision: 1846551

URL: http://svn.apache.org/viewvc?rev=1846551&view=rev
Log:
Fix https://bz.apache.org/bugzilla/show_bug.cgi?id=62899
Prevent the incorrect timing out of connections when Servlet non-blocking I/O is used to read a request body over an HTTP/2 stream.

Modified:
    tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
    tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
    tomcat/trunk/java/org/apache/coyote/http2/Stream.java
    tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java?rev=1846551&r1=1846550&r2=1846551&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java (original)
+++ tomcat/trunk/java/org/apache/catalina/connector/InputBuffer.java Tue Nov 13 23:02:31 2018
@@ -27,6 +27,7 @@ import java.security.PrivilegedActionExc
 import java.security.PrivilegedExceptionAction;
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicBoolean;
 
 import javax.servlet.ReadListener;
 
@@ -214,17 +215,23 @@ public class InputBuffer extends Reader
 
 
     public int available() {
+        int available = availableInThisBuffer();
+        if (available == 0) {
+            coyoteRequest.action(ActionCode.AVAILABLE,
+                    Boolean.valueOf(coyoteRequest.getReadListener() != null));
+            available = (coyoteRequest.getAvailable() > 0) ? 1 : 0;
+        }
+        return available;
+    }
+
+
+    private int availableInThisBuffer() {
         int available = 0;
         if (state == BYTE_STATE) {
             available = bb.remaining();
         } else if (state == CHAR_STATE) {
             available = cb.remaining();
         }
-        if (available == 0) {
-            coyoteRequest.action(ActionCode.AVAILABLE,
-                    Boolean.valueOf(coyoteRequest.getReadListener() != null));
-            available = (coyoteRequest.getAvailable() > 0) ? 1 : 0;
-        }
         return available;
     }
 
@@ -282,11 +289,21 @@ public class InputBuffer extends Reader
             }
             return false;
         }
-        boolean result = available() > 0;
-        if (!result) {
-            coyoteRequest.action(ActionCode.NB_READ_INTEREST, null);
+        // Checking for available data at the network level and registering for
+        // read can be done sequentially for HTTP/1.x and AJP as there is only
+        // ever a single thread processing the socket at any one time. However,
+        // for HTTP/2 there is one thread processing the connection and separate
+        // threads for each stream. For HTTP/2 the two operations have to be
+        // performed atomically else it is possible for the connection thread to
+        // read more data in to the buffer after the stream thread checks for
+        // available network data but before it registers for read.
+        if (availableInThisBuffer() > 0) {
+            return true;
         }
-        return result;
+
+        AtomicBoolean result = new AtomicBoolean();
+        coyoteRequest.action(ActionCode.NB_READ_INTEREST, result);
+        return result.get();
     }
 
 

Modified: tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java?rev=1846551&r1=1846550&r2=1846551&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/AbstractProcessor.java Tue Nov 13 23:02:31 2018
@@ -565,9 +565,8 @@ public abstract class AbstractProcessor
             break;
         }
         case NB_READ_INTEREST: {
-            if (!isRequestBodyFullyRead()) {
-                registerReadInterest();
-            }
+            AtomicBoolean isReady = (AtomicBoolean)param;
+            isReady.set(isReadyForRead());
             break;
         }
         case NB_WRITE_INTEREST: {
@@ -781,6 +780,19 @@ public abstract class AbstractProcessor
     }
 
 
+    protected boolean isReadyForRead() {
+        if (available(true) > 0) {
+            return true;
+        }
+
+        if (!isRequestBodyFullyRead()) {
+            registerReadInterest();
+        }
+
+        return false;
+    }
+
+
     protected abstract boolean isRequestBodyFullyRead();
 
 

Modified: tomcat/trunk/java/org/apache/coyote/http2/Stream.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Stream.java?rev=1846551&r1=1846550&r2=1846551&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Stream.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Stream.java Tue Nov 13 23:02:31 2018
@@ -988,15 +988,22 @@ class Stream extends AbstractStream impl
         }
 
 
-        final void registerReadInterest() {
+        final boolean isReadyForRead() {
             ensureBuffersExist();
 
-            synchronized (inBuffer) {
-                readInterest = true;
+            synchronized (this) {
+                if (available() > 0) {
+                    return true;
+                }
+
+                if (!isRequestBodyFullyRead()) {
+                    readInterest = true;
+                }
+
+                return false;
             }
         }
 
-
         final synchronized boolean isRequestBodyFullyRead() {
             return (inBuffer == null || inBuffer.position() == 0) && isInputFinished();
         }

Modified: tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java?rev=1846551&r1=1846550&r2=1846551&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/StreamProcessor.java Tue Nov 13 23:02:31 2018
@@ -238,6 +238,12 @@ class StreamProcessor extends AbstractPr
 
 
     @Override
+    protected final boolean isReadyForRead() {
+        return stream.getInputBuffer().isReadyForRead();
+    }
+
+
+    @Override
     protected final boolean isRequestBodyFullyRead() {
         return stream.getInputBuffer().isRequestBodyFullyRead();
     }
@@ -245,7 +251,9 @@ class StreamProcessor extends AbstractPr
 
     @Override
     protected final void registerReadInterest() {
-        stream.getInputBuffer().registerReadInterest();
+        // Should never be called for StreamProcessor as isReadyForRead() is
+        // overridden
+        throw new UnsupportedOperationException();
     }
 
 

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1846551&r1=1846550&r2=1846551&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Tue Nov 13 23:02:31 2018
@@ -112,6 +112,11 @@
         Load SSL configuration resources for JSSE using the ConfigurationSource
         API. OpenSSL use requires actual files. (remm)
       </update>
+      <fix>
+        <bug>62899</bug>: Prevent the incorrect timing out of connections when
+        Servlet non-blocking I/O is used to read a request body over an HTTP/2
+        stream. (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