Skip to content
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.

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

NETWORKING: Fix Netty Leaks by upgrading to 4.1.28 #32511

Merged
merged 1 commit into from
Aug 1, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 8 additions & 9 deletions modules/transport-netty4/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -34,13 +34,13 @@ compileTestJava.options.compilerArgs << "-Xlint:-cast,-deprecation,-rawtypes,-tr

dependencies {
// network stack
compile "io.netty:netty-buffer:4.1.16.Final"
compile "io.netty:netty-codec:4.1.16.Final"
compile "io.netty:netty-codec-http:4.1.16.Final"
compile "io.netty:netty-common:4.1.16.Final"
compile "io.netty:netty-handler:4.1.16.Final"
compile "io.netty:netty-resolver:4.1.16.Final"
compile "io.netty:netty-transport:4.1.16.Final"
compile "io.netty:netty-buffer:4.1.28.Final"
compile "io.netty:netty-codec:4.1.28.Final"
compile "io.netty:netty-codec-http:4.1.28.Final"
compile "io.netty:netty-common:4.1.28.Final"
compile "io.netty:netty-handler:4.1.28.Final"
compile "io.netty:netty-resolver:4.1.28.Final"
compile "io.netty:netty-transport:4.1.28.Final"
}

dependencyLicenses {
Expand Down Expand Up @@ -134,7 +134,6 @@ thirdPartyAudit.excludes = [
'net.jpountz.xxhash.StreamingXXHash32',
'net.jpountz.xxhash.XXHashFactory',
'io.netty.internal.tcnative.CertificateRequestedCallback',
'io.netty.internal.tcnative.CertificateRequestedCallback$KeyMaterial',
'io.netty.internal.tcnative.CertificateVerifier',
'io.netty.internal.tcnative.SessionTicketKey',
'io.netty.internal.tcnative.SniHostNameMatcher',
Expand All @@ -161,6 +160,6 @@ thirdPartyAudit.excludes = [

'org.conscrypt.AllocatedBuffer',
'org.conscrypt.BufferAllocator',
'org.conscrypt.Conscrypt$Engines',
'org.conscrypt.Conscrypt',
'org.conscrypt.HandshakeListener'
]

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d6c2d13492778009d33f60e05ed90bcb535d1fd1

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a38361d893900947524f8a9da980555950e73d6a

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
897100c1022c780b0a436b9349e507e8fa9800dc

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
df69ce8bb9b544a71e7bbee290253cf7c93e6bad

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a035784682da0126bc25f10713dac732b5082a6d

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f33557dcb31fa20da075ac05e4808115e32ef9b7

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d2ef28f49d726737f0ffe84bf66529b3bf6e0c0d
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ grant codeBase "${codebase.netty-common}" {

// netty makes and accepts socket connections
permission java.net.SocketPermission "*", "accept,connect";

// Netty sets custom classloader for some of its internal threads
permission java.lang.RuntimePermission "*", "setContextClassLoader";
};

grant codeBase "${codebase.netty-transport}" {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.http.netty4;

import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.util.ReferenceCounted;
import org.elasticsearch.ElasticsearchException;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -92,15 +93,19 @@ public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadC
try (Netty4HttpClient nettyHttpClient = new Netty4HttpClient()) {
final Collection<FullHttpResponse> responses =
nettyHttpClient.get(transportAddress.address(), "/_cluster/settings?pretty=%");
assertThat(responses, hasSize(1));
assertThat(responses.iterator().next().status().code(), equalTo(400));
final Collection<String> responseBodies = Netty4HttpClient.returnHttpResponseBodies(responses);
assertThat(responseBodies, hasSize(1));
assertThat(responseBodies.iterator().next(), containsString("\"type\":\"bad_parameter_exception\""));
assertThat(
try {
assertThat(responses, hasSize(1));
assertThat(responses.iterator().next().status().code(), equalTo(400));
final Collection<String> responseBodies = Netty4HttpClient.returnHttpResponseBodies(responses);
assertThat(responseBodies, hasSize(1));
assertThat(responseBodies.iterator().next(), containsString("\"type\":\"bad_parameter_exception\""));
assertThat(
responseBodies.iterator().next(),
containsString(
"\"reason\":\"java.lang.IllegalArgumentException: unterminated escape sequence at end of string: %\""));
"\"reason\":\"java.lang.IllegalArgumentException: unterminated escape sequence at end of string: %\""));
} finally {
responses.forEach(ReferenceCounted::release);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.http.netty4;

import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.util.ReferenceCounted;
import org.elasticsearch.ESNetty4IntegTestCase;
import org.elasticsearch.common.collect.Tuple;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -88,12 +89,20 @@ public void testLimitsInFlightRequests() throws Exception {

try (Netty4HttpClient nettyHttpClient = new Netty4HttpClient()) {
Collection<FullHttpResponse> singleResponse = nettyHttpClient.post(transportAddress.address(), requests[0]);
assertThat(singleResponse, hasSize(1));
assertAtLeastOnceExpectedStatus(singleResponse, HttpResponseStatus.OK);

Collection<FullHttpResponse> multipleResponses = nettyHttpClient.post(transportAddress.address(), requests);
assertThat(multipleResponses, hasSize(requests.length));
assertAtLeastOnceExpectedStatus(multipleResponses, HttpResponseStatus.SERVICE_UNAVAILABLE);
try {
assertThat(singleResponse, hasSize(1));
assertAtLeastOnceExpectedStatus(singleResponse, HttpResponseStatus.OK);

Collection<FullHttpResponse> multipleResponses = nettyHttpClient.post(transportAddress.address(), requests);
try {
assertThat(multipleResponses, hasSize(requests.length));
assertAtLeastOnceExpectedStatus(multipleResponses, HttpResponseStatus.SERVICE_UNAVAILABLE);
} finally {
multipleResponses.forEach(ReferenceCounted::release);
}
} finally {
singleResponse.forEach(ReferenceCounted::release);
}
}
}

Expand All @@ -113,8 +122,12 @@ public void testDoesNotLimitExcludedRequests() throws Exception {

try (Netty4HttpClient nettyHttpClient = new Netty4HttpClient()) {
Collection<FullHttpResponse> responses = nettyHttpClient.put(transportAddress.address(), requestUris);
assertThat(responses, hasSize(requestUris.length));
assertAllInExpectedStatus(responses, HttpResponseStatus.OK);
try {
assertThat(responses, hasSize(requestUris.length));
assertAllInExpectedStatus(responses, HttpResponseStatus.OK);
} finally {
responses.forEach(ReferenceCounted::release);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import io.netty.handler.codec.http.FullHttpRequest;
import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.handler.codec.http.HttpHeaderNames;
import io.netty.util.ReferenceCounted;
import org.elasticsearch.common.bytes.BytesArray;
import org.elasticsearch.common.network.NetworkService;
import org.elasticsearch.common.settings.Settings;
Expand Down Expand Up @@ -98,8 +99,12 @@ public void testThatHttpPipeliningWorks() throws Exception {

try (Netty4HttpClient nettyHttpClient = new Netty4HttpClient()) {
Collection<FullHttpResponse> responses = nettyHttpClient.get(transportAddress.address(), requests.toArray(new String[]{}));
Collection<String> responseBodies = Netty4HttpClient.returnHttpResponseBodies(responses);
assertThat(responseBodies, contains(requests.toArray()));
try {
Collection<String> responseBodies = Netty4HttpClient.returnHttpResponseBodies(responses);
assertThat(responseBodies, contains(requests.toArray()));
} finally {
responses.forEach(ReferenceCounted::release);
}
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -207,14 +207,23 @@ public void dispatchBadRequest(RestRequest request, RestChannel channel, ThreadC
HttpUtil.setContentLength(request, contentLength);

final FullHttpResponse response = client.post(remoteAddress.address(), request);
assertThat(response.status(), equalTo(expectedStatus));
if (expectedStatus.equals(HttpResponseStatus.CONTINUE)) {
final FullHttpRequest continuationRequest =
new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", Unpooled.EMPTY_BUFFER);
final FullHttpResponse continuationResponse = client.post(remoteAddress.address(), continuationRequest);

assertThat(continuationResponse.status(), is(HttpResponseStatus.OK));
assertThat(new String(ByteBufUtil.getBytes(continuationResponse.content()), StandardCharsets.UTF_8), is("done"));
try {
assertThat(response.status(), equalTo(expectedStatus));
if (expectedStatus.equals(HttpResponseStatus.CONTINUE)) {
final FullHttpRequest continuationRequest =
new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.POST, "/", Unpooled.EMPTY_BUFFER);
final FullHttpResponse continuationResponse = client.post(remoteAddress.address(), continuationRequest);
try {
assertThat(continuationResponse.status(), is(HttpResponseStatus.OK));
assertThat(
new String(ByteBufUtil.getBytes(continuationResponse.content()), StandardCharsets.UTF_8), is("done")
);
} finally {
continuationResponse.release();
}
}
} finally {
response.release();
}
}
}
Expand Down Expand Up @@ -280,10 +289,14 @@ public void dispatchBadRequest(final RestRequest request,
final FullHttpRequest request = new DefaultFullHttpRequest(HttpVersion.HTTP_1_1, HttpMethod.GET, url);

final FullHttpResponse response = client.post(remoteAddress.address(), request);
assertThat(response.status(), equalTo(HttpResponseStatus.BAD_REQUEST));
assertThat(
try {
assertThat(response.status(), equalTo(HttpResponseStatus.BAD_REQUEST));
assertThat(
new String(response.content().array(), Charset.forName("UTF-8")),
containsString("you sent a bad request and you should feel bad"));
} finally {
response.release();
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
package org.elasticsearch.http.netty4;

import io.netty.handler.codec.http.FullHttpResponse;
import io.netty.util.ReferenceCounted;
import org.elasticsearch.ESNetty4IntegTestCase;
import org.elasticsearch.common.transport.TransportAddress;
import org.elasticsearch.http.HttpServerTransport;
Expand All @@ -45,14 +46,18 @@ public void testThatNettyHttpServerSupportsPipelining() throws Exception {

HttpServerTransport httpServerTransport = internalCluster().getInstance(HttpServerTransport.class);
TransportAddress[] boundAddresses = httpServerTransport.boundAddress().boundAddresses();
TransportAddress transportAddress = (TransportAddress) randomFrom(boundAddresses);
TransportAddress transportAddress = randomFrom(boundAddresses);

try (Netty4HttpClient nettyHttpClient = new Netty4HttpClient()) {
Collection<FullHttpResponse> responses = nettyHttpClient.get(transportAddress.address(), requests);
assertThat(responses, hasSize(5));
try {
assertThat(responses, hasSize(5));

Collection<String> opaqueIds = Netty4HttpClient.returnOpaqueIds(responses);
assertOpaqueIdsInOrder(opaqueIds);
Collection<String> opaqueIds = Netty4HttpClient.returnOpaqueIds(responses);
assertOpaqueIdsInOrder(opaqueIds);
} finally {
responses.forEach(ReferenceCounted::release);
}
}
}

Expand Down
17 changes: 8 additions & 9 deletions plugins/transport-nio/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ dependencies {
compile "org.elasticsearch:elasticsearch-nio:${version}"

// network stack
compile "io.netty:netty-buffer:4.1.16.Final"
compile "io.netty:netty-codec:4.1.16.Final"
compile "io.netty:netty-codec-http:4.1.16.Final"
compile "io.netty:netty-common:4.1.16.Final"
compile "io.netty:netty-handler:4.1.16.Final"
compile "io.netty:netty-resolver:4.1.16.Final"
compile "io.netty:netty-transport:4.1.16.Final"
compile "io.netty:netty-buffer:4.1.28.Final"
compile "io.netty:netty-codec:4.1.28.Final"
compile "io.netty:netty-codec-http:4.1.28.Final"
compile "io.netty:netty-common:4.1.28.Final"
compile "io.netty:netty-handler:4.1.28.Final"
compile "io.netty:netty-resolver:4.1.28.Final"
compile "io.netty:netty-transport:4.1.28.Final"
}

dependencyLicenses {
Expand Down Expand Up @@ -113,7 +113,6 @@ thirdPartyAudit.excludes = [
'net.jpountz.xxhash.StreamingXXHash32',
'net.jpountz.xxhash.XXHashFactory',
'io.netty.internal.tcnative.CertificateRequestedCallback',
'io.netty.internal.tcnative.CertificateRequestedCallback$KeyMaterial',
'io.netty.internal.tcnative.CertificateVerifier',
'io.netty.internal.tcnative.SessionTicketKey',
'io.netty.internal.tcnative.SniHostNameMatcher',
Expand All @@ -140,6 +139,6 @@ thirdPartyAudit.excludes = [

'org.conscrypt.AllocatedBuffer',
'org.conscrypt.BufferAllocator',
'org.conscrypt.Conscrypt$Engines',
'org.conscrypt.Conscrypt',
'org.conscrypt.HandshakeListener'
]

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d6c2d13492778009d33f60e05ed90bcb535d1fd1

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a38361d893900947524f8a9da980555950e73d6a

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
897100c1022c780b0a436b9349e507e8fa9800dc

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
df69ce8bb9b544a71e7bbee290253cf7c93e6bad

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
a035784682da0126bc25f10713dac732b5082a6d

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
f33557dcb31fa20da075ac05e4808115e32ef9b7

This file was deleted.

Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
d2ef28f49d726737f0ffe84bf66529b3bf6e0c0d
Original file line number Diff line number Diff line change
Expand Up @@ -26,4 +26,6 @@ grant codeBase "${codebase.netty-common}" {
// This should only currently be required as we use the netty http client for tests
// netty makes and accepts socket connections
permission java.net.SocketPermission "*", "accept,connect";
// Netty sets custom classloader for some of its internal threads
permission java.lang.RuntimePermission "*", "setContextClassLoader";
};
Loading