netty: Unconditionally disable adaptive cumulator (grpc#12390)

io.netty.util.Version is unreliable, so we stop using it. grpc-netty and
grpc-netty-shaded have their version.properties mix, and you can't tell
which is which.

Changed the tests to use assume, so it is clear in the results that they
weren't run.

Author: ejona86

netty/shaded/build.gradle

@@ -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

netty/src/main/java/io/grpc/netty/GrpcHttp2ConnectionHandler.java

@@ -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;
@@ -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(
@@ -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;
   }
 
   /**

netty/src/test/java/io/grpc/netty/NettyAdaptiveCumulatorTest.java

@@ -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);
@@ -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.
@@ -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