Http/2 revamp (#9520)

9273 Http/2 revamp * Add h2spec test * Consuming request trailers * Larger frame splitting fix * Flow control update timeout * Streamed payload larger than content length discovery Signed-off-by: Daniel Kec <[email protected]>
helidon-io · Dec 16, 2024 · 3244b5a · 3244b5a
1 parent 309ed37
commit 3244b5a
Show file tree

Hide file tree

Showing 19 changed files with 1,076 additions and 126 deletions.
diff --git a/http/http2/src/main/java/io/helidon/http/http2/Http2ConnectionWriter.java b/http/http2/src/main/java/io/helidon/http/http2/Http2ConnectionWriter.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -30,7 +30,6 @@
 public class Http2ConnectionWriter implements Http2StreamWriter {
     private final DataWriter writer;
 
-    // todo replace with prioritized lock (stream priority + connection writes have highest prio)
     private final Lock streamLock = new ReentrantLock(true);
     private final SocketContext ctx;
     private final Http2FrameListener listener;
@@ -143,24 +142,14 @@ public int writeHeaders(Http2Headers headers,
                             Http2Flag.HeaderFlags flags,
                             Http2FrameData dataFrame,
                             FlowControl.Outbound flowControl) {
-        // this is executing in the thread of the stream
-        // we must enforce parallelism of exactly 1, to make sure the dynamic table is updated
-        // and then immediately written
-
-        lock();
-        try {
-            int bytesWritten = 0;
-
-            bytesWritten += writeHeaders(headers, streamId, flags, flowControl);
-
-            writeData(dataFrame, flowControl);
-            bytesWritten += Http2FrameHeader.LENGTH;
-            bytesWritten += dataFrame.header().length();
-
-            return bytesWritten;
-        } finally {
-            streamLock.unlock();
-        }
+        // Executed on stream thread
+        int bytesWritten = 0;
+        bytesWritten += writeHeaders(headers, streamId, flags, flowControl);
+        writeData(dataFrame, flowControl);
+        bytesWritten += Http2FrameHeader.LENGTH;
+        bytesWritten += dataFrame.header().length();
+
+        return bytesWritten;
     }
 
     /**
@@ -227,32 +216,10 @@ private void splitAndWrite(Http2FrameData frame, FlowControl.Outbound flowContro
             } else if (splitFrames.length == 2) {
                 // write send-able part and block until window update with the rest
                 lockedWrite(splitFrames[0]);
-                flowControl.decrementWindowSize(currFrame.header().length());
+                flowControl.decrementWindowSize(splitFrames[0].header().length());
                 flowControl.blockTillUpdate();
                 currFrame = splitFrames[1];
             }
         }
     }
-
-    // TODO use for fastpath
-    //    private void noLockWrite(Http2FrameData... frames) {
-    //        List<BufferData> toWrite = new LinkedList<>();
-    //
-    //        for (Http2FrameData frame : frames) {
-    //            BufferData headerData = frame.header().write();
-    //
-    //            listener.frameHeader(ctx, frame.header());
-    //            listener.frameHeader(ctx, headerData);
-    //
-    //            toWrite.add(headerData);
-    //
-    //            BufferData data = frame.data();
-    //
-    //            if (data.available() != 0) {
-    //                toWrite.add(data);
-    //            }
-    //        }
-    //
-    //        writer.write(toWrite.toArray(new BufferData[0]));
-    //    }
 }
diff --git a/http/http2/src/main/java/io/helidon/http/http2/WindowSizeImpl.java b/http/http2/src/main/java/io/helidon/http/http2/WindowSizeImpl.java
@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+ * Copyright (c) 2022, 2024 Oracle and/or its affiliates.
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -15,12 +15,9 @@
  */
 package io.helidon.http.http2;
 
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Semaphore;
 import java.util.concurrent.TimeUnit;
-import java.util.concurrent.TimeoutException;
 import java.util.concurrent.atomic.AtomicInteger;
-import java.util.concurrent.atomic.AtomicReference;
 import java.util.function.BiConsumer;
 
 import static java.lang.System.Logger.Level.DEBUG;
@@ -124,7 +121,9 @@ public long incrementWindowSize(int increment) {
      */
     static final class Outbound extends WindowSizeImpl implements WindowSize.Outbound {
 
-        private final AtomicReference<CompletableFuture<Void>> updated = new AtomicReference<>(new CompletableFuture<>());
+        private static final int BACKOFF_MIN = 50;
+        private static final int BACKOFF_MAX = 5000;
+        private final Semaphore updatedSemaphore = new Semaphore(1);
         private final ConnectionFlowControl.Type type;
         private final int streamId;
         private final long timeoutMillis;
@@ -146,21 +145,52 @@ public long incrementWindowSize(int increment) {
             return remaining;
         }
 
+        @Override
+        public void resetWindowSize(int size) {
+            super.resetWindowSize(size);
+            triggerUpdate();
+        }
+
+        @Override
+        public int decrementWindowSize(int decrement) {
+            int n = super.decrementWindowSize(decrement);
+            triggerUpdate();
+            return n;
+        }
+
         @Override
         public void triggerUpdate() {
-            updated.getAndSet(new CompletableFuture<>()).complete(null);
+            updatedSemaphore.release();
         }
 
         @Override
         public void blockTillUpdate() {
+            var startTime = System.currentTimeMillis();
+            int backoff = BACKOFF_MIN;
             while (getRemainingWindowSize() < 1) {
                 try {
-                    updated.get().get(timeoutMillis, TimeUnit.MILLISECONDS);
-                } catch (InterruptedException | ExecutionException | TimeoutException e) {
-                    if (LOGGER_OUTBOUND.isLoggable(DEBUG)) {
-                        LOGGER_OUTBOUND.log(DEBUG,
-                                            String.format("%s OFC STR %d: Window depleted, waiting for update.", type, streamId));
-                    }
+                    updatedSemaphore.drainPermits();
+                    var ignored = updatedSemaphore.tryAcquire(backoff, TimeUnit.MILLISECONDS);
+                    // linear deterministic backoff
+                    backoff = Math.min(backoff * 2, BACKOFF_MAX);
+                } catch (InterruptedException e) {
+                    debugLog("%s OFC STR %d: Window depleted, waiting for update interrupted.", e);
+                    throw new Http2Exception(Http2ErrorCode.FLOW_CONTROL, "Flow control update wait interrupted.");
+                }
+                if (System.currentTimeMillis() - startTime > timeoutMillis) {
+                    debugLog("%s OFC STR %d: Window depleted, waiting for update time-out.", null);
+                    throw new Http2Exception(Http2ErrorCode.FLOW_CONTROL, "Flow control update wait time-out.");
+                }
+                debugLog("%s OFC STR %d: Window depleted, waiting for update.", null);
+            }
+        }
+
+        private void debugLog(String message, Exception e) {
+            if (LOGGER_OUTBOUND.isLoggable(DEBUG)) {
+                if (e != null) {
+                    LOGGER_OUTBOUND.log(DEBUG, String.format(message, type, streamId), e);
+                } else {
+                    LOGGER_OUTBOUND.log(DEBUG, String.format(message, type, streamId));
                 }
             }
         }

diff --git a/http/tests/media/multipart/pom.xml b/http/tests/media/multipart/pom.xml
@@ -36,6 +36,10 @@
             <groupId>io.helidon.http.media</groupId>
             <artifactId>helidon-http-media-multipart</artifactId>
         </dependency>
+        <dependency>
+            <groupId>io.helidon.logging</groupId>
+            <artifactId>helidon-logging-jul</artifactId>
+        </dependency>
         <dependency>
             <groupId>io.helidon.webserver.testing.junit5</groupId>
             <artifactId>helidon-webserver-testing-junit5</artifactId>

diff --git a/http/tests/media/multipart/src/test/resources/logging-test.properties b/http/tests/media/multipart/src/test/resources/logging-test.properties
@@ -1,5 +1,5 @@
 #
-# Copyright (c) 2022, 2023 Oracle and/or its affiliates.
+# Copyright (c) 2022, 2024 Oracle and/or its affiliates.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -13,7 +13,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 #
-handlers=java.util.logging.ConsoleHandler
+handlers=io.helidon.logging.jul.HelidonConsoleHandler
 java.util.logging.ConsoleHandler.level=FINEST
 java.util.logging.ConsoleHandler.formatter=java.util.logging.SimpleFormatter
 java.util.logging.SimpleFormatter.format=%1$tH:%1$tM:%1$tS %4$s %3$s %5$s%6$s%n

diff --git a/tests/integration/h2spec/Dockerfile b/tests/integration/h2spec/Dockerfile
@@ -0,0 +1,39 @@
+#
+# Copyright (c) 2024 Oracle and/or its affiliates.
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#     http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+#
+
+FROM container-registry.oracle.com/os/oraclelinux:9-slim AS build
+ENV GO111MODULE=on
+ENV GOPROXY=https://proxy.golang.org
+ENV CGO_ENABLED=0
+ENV VERSION=2.6.1-SNAPSHOT
+ENV COMMIT=af83a65f0b6273ef38bf778d400d98892e7653d8
+
+RUN microdnf install go-toolset git -y
+
+WORKDIR /workspace
+RUN git clone https://github.com/summerwind/h2spec.git
+
+WORKDIR /workspace/h2spec
+RUN git checkout ${COMMIT}
+RUN go build -ldflags "-X main.VERSION=${VERSION} -X main.COMMIT=${COMMIT}" ./cmd/h2spec
+
+FROM container-registry.oracle.com/os/oraclelinux:9-slim
+ARG PORT=8080
+ARG HOST=localhost
+ENV PORT=${PORT}
+ENV HOST=${HOST}
+COPY --from=build /workspace/h2spec/h2spec /usr/local/bin/h2spec
+CMD ["/usr/local/bin/h2spec", "-h", "${HOST}", "-p", "${PORT}"]
diff --git a/tests/integration/h2spec/pom.xml b/tests/integration/h2spec/pom.xml
@@ -0,0 +1,133 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+    Copyright (c) 2024 Oracle and/or its affiliates.
+
+    Licensed under the Apache License, Version 2.0 (the "License");
+    you may not use this file except in compliance with the License.
+    You may obtain a copy of the License at
+
+        http://www.apache.org/licenses/LICENSE-2.0
+
+    Unless required by applicable law or agreed to in writing, software
+    distributed under the License is distributed on an "AS IS" BASIS,
+    WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+    See the License for the specific language governing permissions and
+    limitations under the License.
+-->
+
+<project xmlns="http://maven.apache.org/POM/4.0.0"
+        xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+        xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
+    <modelVersion>4.0.0</modelVersion>
+    <parent>
+        <groupId>io.helidon.applications</groupId>
+        <artifactId>helidon-mp</artifactId>
+        <version>4.2.0-SNAPSHOT</version>
+        <relativePath>../../../applications/mp/pom.xml</relativePath>
+    </parent>
+    <groupId>io.helidon.tests.integration.h2spec</groupId>
+    <artifactId>helidon-tests-integration-h2spec</artifactId>
+    <name>Helidon Tests Integration Http/2 h2spec</name>
+
+    <properties>
+        <mainClass>io.helidon.webserver.h2spec.Main</mainClass>
+        <redirectTestOutputToFile>true</redirectTestOutputToFile>
+    </properties>
+
+    <dependencies>
+        <dependency>
+            <groupId>io.helidon.webserver</groupId>
+            <artifactId>helidon-webserver</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.webserver</groupId>
+            <artifactId>helidon-webserver-http2</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.config</groupId>
+            <artifactId>helidon-config-yaml</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.logging</groupId>
+            <artifactId>helidon-logging-jul</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-jdk14</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-api</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.junit.jupiter</groupId>
+            <artifactId>junit-jupiter-params</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.hamcrest</groupId>
+            <artifactId>hamcrest-all</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <!--suppress VulnerableLibrariesLocal -->
+        <dependency>
+            <groupId>org.testcontainers</groupId>
+            <artifactId>junit-jupiter</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>io.helidon.webserver.testing.junit5</groupId>
+            <artifactId>helidon-webserver-testing-junit5</artifactId>
+            <scope>test</scope>
+        </dependency>
+    </dependencies>
+
+    <build>
+        <plugins>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-dependency-plugin</artifactId>
+                <executions>
+                    <execution>
+                        <id>copy-libs</id>
+                    </execution>
+                </executions>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-surefire-plugin</artifactId>
+                <configuration>
+                    <excludes>
+                        <exclude>**/*IT</exclude>
+                    </excludes>
+                    <systemPropertyVariables>
+                        <java.util.logging.config.file>
+                            ${project.build.outputDirectory}/logging.properties
+                        </java.util.logging.config.file>
+                    </systemPropertyVariables>
+                </configuration>
+            </plugin>
+            <plugin>
+                <groupId>org.apache.maven.plugins</groupId>
+                <artifactId>maven-failsafe-plugin</artifactId>
+                <configuration>
+                    <systemPropertyVariables>
+                        <java.util.logging.config.file>
+                            ${project.build.outputDirectory}/logging.properties
+                        </java.util.logging.config.file>
+                    </systemPropertyVariables>
+                    <redirectTestOutputToFile>${redirectTestOutputToFile}</redirectTestOutputToFile>
+                </configuration>
+                <executions>
+                    <execution>
+                        <goals>
+                            <goal>integration-test</goal>
+                            <goal>verify</goal>
+                        </goals>
+                    </execution>
+                </executions>
+            </plugin>
+        </plugins>
+    </build>
+</project>