Skip to content

Commit

Permalink
Improve discovery of (and fix some) netty-releated test buffer leaks
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
daschl committed Jan 20, 2025
1 parent 13197de commit 4e38926
Show file tree
Hide file tree
Showing 3 changed files with 8 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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 {
Expand Down

0 comments on commit 4e38926

Please sign in to comment.