From bdc767f118c617e64f6ec1492ebe68fcdcb22754 Mon Sep 17 00:00:00 2001
From: Michael Nitschinger <mnitschinger@apple.com>
Date: Tue, 14 Jan 2025 15:50:10 +0100
Subject: [PATCH] Improve discovery of (and fix some) netty-releated test
 buffer leaks

This changeset improves disovery of netty-related buffer leaks if
the --warn flag is enabled on the command line when running the
gradle tests. Paranoid is enabled all the time now, but the overhead
of doing so is very minimal and does not need to be enabled conditionally.
--warn is needed since only in that mode stdout is visible on the
command line.

Running it showed a couple of leaks in tests where two of them are
already fixed as part of this commit.
---
 .../internal/ServiceTalkLibraryPlugin.groovy      | 15 +++++++++++++++
 .../netty/H2PriorKnowledgeFeatureParityTest.java  |  2 +-
 .../http/netty/ServerRespondsOnClosingTest.java   |  3 ++-
 3 files changed, 18 insertions(+), 2 deletions(-)

diff --git a/servicetalk-gradle-plugin-internal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy b/servicetalk-gradle-plugin-internal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy
index 523d445918..49a6898981 100644
--- a/servicetalk-gradle-plugin-internal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy
+++ b/servicetalk-gradle-plugin-internal/src/main/groovy/io/servicetalk/gradle/plugin/internal/ServiceTalkLibraryPlugin.groovy
@@ -224,6 +224,16 @@ final class ServiceTalkLibraryPlugin extends ServiceTalkCorePlugin {
           }
         }
 
+        filter {
+          //includeTestsMatching "ContextMapTest"
+          failOnNoMatchingTests = false
+        }
+
+        reports {
+          html.required = true
+        }
+
+
         // if property is defined and true allow tests to continue running after first fail
         ignoreFailures = Boolean.getBoolean("servicetalk.test.ignoreFailures")
 
@@ -233,6 +243,11 @@ final class ServiceTalkLibraryPlugin extends ServiceTalkCorePlugin {
         // Always require native libraries for running tests. This helps to make sure transport-level state machine
         // receives all expected network events from Netty.
         systemProperty "io.servicetalk.transport.netty.requireNativeLibs", "true"
+
+        // Make sure we enable netty leak detection for our tests. By default these are not visible in the logs,
+        // to see them you must at --warn to your ./gradlew test run in order to see the "showStandardStreams".
+        systemProperty "io.netty.leakDetection.level", "PARANOID"
+        systemProperty "io.netty.leakDetection.targetRecords", "25"
       }
 
       dependencies {
diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/H2PriorKnowledgeFeatureParityTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/H2PriorKnowledgeFeatureParityTest.java
index 7a44f1b6d0..20f12e5073 100644
--- a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/H2PriorKnowledgeFeatureParityTest.java
+++ b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/H2PriorKnowledgeFeatureParityTest.java
@@ -2070,7 +2070,7 @@ public void channelReadComplete(ChannelHandlerContext ctx) {
         }
 
         private static void onDataRead(ChannelHandlerContext ctx, Http2DataFrame data) {
-            ctx.write(new DefaultHttp2DataFrame(data.content().retainedDuplicate(), data.isEndStream()));
+            ctx.write(new DefaultHttp2DataFrame(data.content(), data.isEndStream()));
         }
 
         private void onHeadersRead(ChannelHandlerContext ctx, Http2HeadersFrame headers) {
diff --git a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ServerRespondsOnClosingTest.java b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ServerRespondsOnClosingTest.java
index c04a81faf4..2582d069ce 100644
--- a/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ServerRespondsOnClosingTest.java
+++ b/servicetalk-http-netty/src/test/java/io/servicetalk/http/netty/ServerRespondsOnClosingTest.java
@@ -62,7 +62,6 @@
 import static org.hamcrest.Matchers.is;
 
 class ServerRespondsOnClosingTest {
-
     private static final HttpResponseFactory RESPONSE_FACTORY = new DefaultHttpResponseFactory(
             DefaultHttpHeadersFactory.INSTANCE, DEFAULT_ALLOCATOR, HTTP_1_1);
     private static final String RESPONSE_PAYLOAD_BODY = "Hello World";
@@ -252,6 +251,8 @@ private void verifyResponse(String requestPath) {
         assertThat("Unexpected response meta-data", metaData.toString(US_ASCII), containsString(requestPath));
         ByteBuf payloadBody = channel.readOutbound();
         assertThat("Unexpected response payload body", payloadBody.toString(US_ASCII), equalTo(RESPONSE_PAYLOAD_BODY));
+        metaData.release();
+        payloadBody.release();
     }
 
     private void respondWithFIN() throws Exception {