Skip to content

netty: Unconditionally disable adaptive cumulator #12390

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 1, 2025
Merged

netty: Unconditionally disable adaptive cumulator #12390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion netty/shaded/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,11 @@ class NettyResourceTransformer implements Transformer {

@Override
boolean canTransformResource(FileTreeElement fileTreeElement) {
fileTreeElement.name.startsWith("META-INF/native-image/io.netty")
// io.netty.versions.properties can't actually be shaded successfully,
// as io.netty.util.Version still looks for the unshaded name. But we
// keep the file for manual inspection.
fileTreeElement.name.startsWith("META-INF/native-image/io.netty") ||
fileTreeElement.name.startsWith("META-INF/io.netty.versions.properties")
}

@Override
Expand Down
40 changes: 0 additions & 40 deletions netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@

import static com.google.common.base.Preconditions.checkState;

import com.google.common.annotations.VisibleForTesting;
import io.grpc.Attributes;
import io.grpc.ChannelLogger;
import io.grpc.Internal;
Expand All @@ -28,45 +27,16 @@
import io.netty.handler.codec.http2.Http2ConnectionEncoder;
import io.netty.handler.codec.http2.Http2ConnectionHandler;
import io.netty.handler.codec.http2.Http2Settings;
import io.netty.util.Version;
import javax.annotation.Nullable;

/**
* gRPC wrapper for {@link Http2ConnectionHandler}.
*/
@Internal
public abstract class GrpcHttp2ConnectionHandler extends Http2ConnectionHandler {
static final int ADAPTIVE_CUMULATOR_COMPOSE_MIN_SIZE_DEFAULT = 1024;
static final Cumulator ADAPTIVE_CUMULATOR =
new NettyAdaptiveCumulator(ADAPTIVE_CUMULATOR_COMPOSE_MIN_SIZE_DEFAULT);

@Nullable
protected final ChannelPromise channelUnused;
private final ChannelLogger negotiationLogger;
private static final boolean usingPre4_1_111_Netty;

static {
// Netty 4.1.111 introduced a change in the behavior of duplicate() method
// that breaks the assumption of the cumulator. We need to detect this version
// and adjust the behavior accordingly.

boolean identifiedOldVersion = false;
try {
Version version = Version.identify().get("netty-buffer");
if (version != null) {
String[] split = version.artifactVersion().split("\\.");
if (split.length >= 3
&& Integer.parseInt(split[0]) == 4
&& Integer.parseInt(split[1]) <= 1
&& Integer.parseInt(split[2]) < 111) {
identifiedOldVersion = true;
}
}
} catch (Exception e) {
// Ignore, we'll assume it's a new version.
}
usingPre4_1_111_Netty = identifiedOldVersion;
}

@SuppressWarnings("this-escape")
protected GrpcHttp2ConnectionHandler(
Expand All @@ -78,16 +48,6 @@ protected GrpcHttp2ConnectionHandler(
super(decoder, encoder, initialSettings);
this.channelUnused = channelUnused;
this.negotiationLogger = negotiationLogger;
if (usingPre4_1_111_Netty()) {
// We need to use the adaptive cumulator only if we're using a version of Netty that
// doesn't have the behavior that breaks it.
setCumulator(ADAPTIVE_CUMULATOR);
}
}

@VisibleForTesting
static boolean usingPre4_1_111_Netty() {
return usingPre4_1_111_Netty;
}

/**
Expand Down
13 changes: 7 additions & 6 deletions netty/src/test/java/io/grpc/netty/NettyAdaptiveCumulatorTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@

@RunWith(Enclosed.class)
public class NettyAdaptiveCumulatorTest {
private static boolean usingPre4_1_111_Netty() {
return false; // Disabled detection because it was unreliable
}

private static Collection<Object[]> cartesianProductParams(List<?>... lists) {
return Lists.transform(Lists.cartesianProduct(lists), List::toArray);
Expand Down Expand Up @@ -385,9 +388,8 @@ public void mergeWithCompositeTail_tailExpandable_reallocateInMemory() {
}

private void assertTailExpanded(String expectedTailReadableData, int expectedNewTailCapacity) {
if (!GrpcHttp2ConnectionHandler.usingPre4_1_111_Netty()) {
return; // Netty 4.1.111 doesn't work with NettyAdaptiveCumulator
}
assume().withMessage("Netty 4.1.111 doesn't work with NettyAdaptiveCumulator")
.that(usingPre4_1_111_Netty()).isTrue();
int originalNumComponents = composite.numComponents();

// Handle the case when reader index is beyond all readable bytes of the cumulation.
Expand Down Expand Up @@ -628,9 +630,8 @@ public void mergeWithCompositeTail_outOfSyncComposite() {
alloc.compositeBuffer(8).addFlattenedComponents(true, composite1);
assertThat(composite2.toString(US_ASCII)).isEqualTo("01234");

if (!GrpcHttp2ConnectionHandler.usingPre4_1_111_Netty()) {
return; // Netty 4.1.111 doesn't work with NettyAdaptiveCumulator
}
assume().withMessage("Netty 4.1.111 doesn't work with NettyAdaptiveCumulator")
.that(usingPre4_1_111_Netty()).isTrue();

// The previous operation does not adjust the read indexes of the underlying buffers,
// only the internal Component offsets. When the cumulator attempts to append the input to
Expand Down