From e84edfeb8a2f01091656c91652bdbfb7a3630dc9 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Wed, 15 Jun 2022 07:43:34 -0700 Subject: [PATCH 1/3] Update Netty 4.1.77 -> 4.1.78 --- gradle.properties | 2 +- .../http/netty/OptimizedHttp2FrameCodecBuilder.java | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/gradle.properties b/gradle.properties index 0c6c66fccd..0621be453b 100644 --- a/gradle.properties +++ b/gradle.properties @@ -29,7 +29,7 @@ issueManagementUrl=https://github.com/apple/servicetalk/issues ciManagementUrl=https://github.com/apple/servicetalk/actions # dependency versions -nettyVersion=4.1.77.Final +nettyVersion=4.1.78.Final nettyIoUringVersion=0.0.14.Final jsr305Version=3.0.2 diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java index 7aa42cbc6f..9a8155becc 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java @@ -37,6 +37,10 @@ final class OptimizedHttp2FrameCodecBuilder extends Http2FrameCodecBuilder { */ OptimizedHttp2FrameCodecBuilder(final boolean server) { this.server = server; + // We manage flushes at ST level and don't want netty to flush the preface & settings only. Instead, we write + // headers or entire message and flush them all together. Netty changed the default flush behavior starting from + // 4.1.78.Final. For context, see https://github.com/netty/netty/pull/12349. + flushPreface(false); } @Override From 03a7bc1074b3f6c93ac0ea4d17204ba830c3cc5d Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Wed, 15 Jun 2022 14:07:08 -0700 Subject: [PATCH 2/3] Use MethodHandle to avoid hard dependency on newer netty version --- .../OptimizedHttp2FrameCodecBuilder.java | 65 +++++++++++++++++-- 1 file changed, 60 insertions(+), 5 deletions(-) diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java index 9a8155becc..9f0153ce4b 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java @@ -1,5 +1,5 @@ /* - * Copyright © 2021 Apple Inc. and the ServiceTalk project authors + * Copyright © 2021-2022 Apple Inc. and the ServiceTalk project authors * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -21,6 +21,15 @@ import io.netty.handler.codec.http2.Http2FrameCodecBuilder; import io.netty.handler.codec.http2.Http2RemoteFlowController; import io.netty.handler.codec.http2.UniformStreamByteDistributor; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; + +import java.lang.invoke.MethodHandle; +import java.lang.invoke.MethodHandles; +import javax.annotation.Nullable; + +import static io.servicetalk.utils.internal.PlatformDependent.throwException; +import static java.lang.invoke.MethodType.methodType; /** * Optimized variant of {@link Http2FrameCodecBuilder} that allows us to use {@link UniformStreamByteDistributor} @@ -28,6 +37,29 @@ */ final class OptimizedHttp2FrameCodecBuilder extends Http2FrameCodecBuilder { + private static final Logger LOGGER = LoggerFactory.getLogger(OptimizedHttp2FrameCodecBuilder.class); + + @Nullable + private static final MethodHandle FLUSH_PREFACE; + + static { + MethodHandle flushPreface; + try { + // Find a new method that exists only in Netty starting from 4.1.78.Final: + flushPreface = MethodHandles.publicLookup() + .findVirtual(Http2FrameCodecBuilder.class, "flushPreface", + methodType(Http2FrameCodecBuilder.class, boolean.class)); + // Verify the method is working as expected: + disableFlushPreface(flushPreface, Http2FrameCodecBuilder.forClient()); + } catch (Throwable cause) { + LOGGER.debug("Http2FrameCodecBuilder#flushPreface(boolean) is available only starting from " + + "Netty 4.1.78.Final. Detected Netty version: {}", + Http2FrameCodecBuilder.class.getPackage().getImplementationVersion(), cause); + flushPreface = null; + } + FLUSH_PREFACE = flushPreface; + } + private final boolean server; /** @@ -37,10 +69,7 @@ final class OptimizedHttp2FrameCodecBuilder extends Http2FrameCodecBuilder { */ OptimizedHttp2FrameCodecBuilder(final boolean server) { this.server = server; - // We manage flushes at ST level and don't want netty to flush the preface & settings only. Instead, we write - // headers or entire message and flush them all together. Netty changed the default flush behavior starting from - // 4.1.78.Final. For context, see https://github.com/netty/netty/pull/12349. - flushPreface(false); + disableFlushPreface(FLUSH_PREFACE, this); } @Override @@ -56,4 +85,30 @@ public Http2FrameCodec build() { connection(connection); return super.build(); } + + /** + * We manage flushes at ST level and don't want netty to flush the preface & settings only. Instead, we write + * headers or entire message and flush them all together. Netty changed the default flush behavior starting from + * 4.1.78.Final. To avoid a strict dependency on Netty 4.1.78.Final in the classpath, we use {@link MethodHandle} to + * check if the new method is available or not. + * + * @param flushPrefaceMethod {@link MethodHandle} for {@link Http2FrameCodecBuilder#flushPreface(boolean)} + * @param builderInstance an instance of {@link Http2FrameCodecBuilder} where the flush behavior should be disabled + * @return {@link Http2FrameCodecBuilder} or {@code null} if {@code flushPrefaceMethod == null} + * @see Netty PR#12349 + */ + @Nullable + private static Http2FrameCodecBuilder disableFlushPreface(@Nullable final MethodHandle flushPrefaceMethod, + final Http2FrameCodecBuilder builderInstance) { + if (flushPrefaceMethod == null) { + return null; + } + try { + // invokeExact requires return type cast to match the type signature + return (Http2FrameCodecBuilder) flushPrefaceMethod.invokeExact(builderInstance, false); + } catch (Throwable t) { + throwException(t); + return null; + } + } } From 5b3a078ed55abfe6ac5f8d16d7f4cd4ee1e446f0 Mon Sep 17 00:00:00 2001 From: Idel Pivnitskiy Date: Wed, 15 Jun 2022 14:18:48 -0700 Subject: [PATCH 3/3] Return `builderInstance` instead of `null` --- .../http/netty/OptimizedHttp2FrameCodecBuilder.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java index 9f0153ce4b..193c7ada6f 100644 --- a/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java +++ b/servicetalk-http-netty/src/main/java/io/servicetalk/http/netty/OptimizedHttp2FrameCodecBuilder.java @@ -97,18 +97,17 @@ public Http2FrameCodec build() { * @return {@link Http2FrameCodecBuilder} or {@code null} if {@code flushPrefaceMethod == null} * @see Netty PR#12349 */ - @Nullable private static Http2FrameCodecBuilder disableFlushPreface(@Nullable final MethodHandle flushPrefaceMethod, final Http2FrameCodecBuilder builderInstance) { if (flushPrefaceMethod == null) { - return null; + return builderInstance; } try { // invokeExact requires return type cast to match the type signature return (Http2FrameCodecBuilder) flushPrefaceMethod.invokeExact(builderInstance, false); } catch (Throwable t) { throwException(t); - return null; + return builderInstance; } } }