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/03/08 11:58:01 UTC

svn commit: r1826209 - in /tomcat/trunk: java/org/apache/coyote/http2/Http2AsyncParser.java java/org/apache/coyote/http2/Http2Parser.java java/org/apache/coyote/http2/Http2UpgradeHandler.java webapps/docs/changelog.xml

Author: markt
Date: Thu Mar  8 11:58:01 2018
New Revision: 1826209

URL: http://svn.apache.org/viewvc?rev=1826209&view=rev
Log:
Refactor the check for a paused connector to consistently prevent new streams from being created after the connector has been paused.
This appears to fix the HTTP/2 NIO2 unit test failure. At least it does on the machine on which I could reproduce it.

Modified:
    tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncParser.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
    tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
    tomcat/trunk/webapps/docs/changelog.xml

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncParser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncParser.java?rev=1826209&r1=1826208&r2=1826209&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncParser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2AsyncParser.java Thu Mar  8 11:58:01 2018
@@ -133,11 +133,6 @@ class Http2AsyncParser extends Http2Pars
             }
 
             if (buffers[1].position() < handler.payloadSize) {
-                try {
-                    upgradeHandler.checkPauseState();
-                } catch (IOException e) {
-                    error = e;
-                }
                 return CompletionHandlerCall.CONTINUE;
             }
 

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java?rev=1826209&r1=1826208&r2=1826209&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2Parser.java Thu Mar  8 11:58:01 2018
@@ -718,7 +718,8 @@ class Http2Parser {
         void swallowedPadding(int streamId, int paddingLength) throws ConnectionException, IOException;
 
         // Header frames
-        HeaderEmitter headersStart(int streamId, boolean headersEndStream) throws Http2Exception;
+        HeaderEmitter headersStart(int streamId, boolean headersEndStream)
+                throws Http2Exception, IOException;
         void headersEnd(int streamId) throws ConnectionException;
 
         // Priority frames (also headers)

Modified: tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java
URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java?rev=1826209&r1=1826208&r2=1826209&view=diff
==============================================================================
--- tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java (original)
+++ tomcat/trunk/java/org/apache/coyote/http2/Http2UpgradeHandler.java Thu Mar  8 11:58:01 2018
@@ -305,8 +305,6 @@ class Http2UpgradeHandler extends Abstra
         try {
             pingManager.sendPing(false);
 
-            checkPauseState();
-
             switch(status) {
             case OPEN_READ:
                 try {
@@ -1234,7 +1232,13 @@ class Http2UpgradeHandler extends Abstra
 
 
     @Override
-    public HeaderEmitter headersStart(int streamId, boolean headersEndStream) throws Http2Exception {
+    public HeaderEmitter headersStart(int streamId, boolean headersEndStream)
+            throws Http2Exception, IOException {
+
+        // Check the pause state before processing headers since the pause state
+        // determines if a new stream is created or if this stream is ignored.
+        checkPauseState();
+
         if (connectionState.get().isNewStreamAllowed()) {
             Stream stream = getStream(streamId, false);
             if (stream == null) {

Modified: tomcat/trunk/webapps/docs/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1826209&r1=1826208&r2=1826209&view=diff
==============================================================================
--- tomcat/trunk/webapps/docs/changelog.xml (original)
+++ tomcat/trunk/webapps/docs/changelog.xml Thu Mar  8 11:58:01 2018
@@ -63,6 +63,10 @@
         Ensure streams that are received but not processed are excluded from the
         tracking of maximum ID of processed streams. (markt)
       </fix>
+      <fix>
+        Refactor the check for a paused connector to consistently prevent new
+        streams from being created after the connector has been paused. (markt)
+      </fix>
     </changelog>
   </subsection>
   <subsection name="Other">



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


Re: svn commit: r1826209 - in /tomcat/trunk: java/org/apache/coyote/http2/Http2AsyncParser.java java/org/apache/coyote/http2/Http2Parser.java java/org/apache/coyote/http2/Http2UpgradeHandler.java webapps/docs/changelog.xml

Posted by Rémy Maucherat <re...@apache.org>.
On Thu, Mar 8, 2018 at 12:58 PM, <ma...@apache.org> wrote:

> Author: markt
> Date: Thu Mar  8 11:58:01 2018
> New Revision: 1826209
>
> URL: http://svn.apache.org/viewvc?rev=1826209&view=rev
> Log:
> Refactor the check for a paused connector to consistently prevent new
> streams from being created after the connector has been paused.
> This appears to fix the HTTP/2 NIO2 unit test failure. At least it does on
> the machine on which I could reproduce it.
>
> First run worked at least, so that new location for the check may be
better and it makes the parser even smaller.

Rémy