andreacavalli/netty5
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Fix DnsNameResolver may have LEAK ByteBuf after cancelling the returned future (#10448)

Browse Source 
Motivation:

If we cancel the returned future of resolve query, we may get LEAK. Try to release the ByteBuf if netty can't pass the DnsRawRecord to the caller.

Modification:

Using debug mode I saw there are two places that don't handle trySuccess with release. Try to release there.

Result:

Fixes #10447.
This commit is contained in:
Koji Lin 2020-08-03 14:58:03 +09:00 committed by GitHub
parent dd168f406e
commit c4754cf7b8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 16 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java
View File

@ -920,13 +920,15 @@ public class DnsNameResolver extends InetNameResolver {
} }
} }
static <T> void trySuccess(Promise<T> promise, T result) { static <T> boolean trySuccess(Promise<T> promise, T result) {
if (!promise.trySuccess(result)) { final boolean notifiedRecords = promise.trySuccess(result);
if (!notifiedRecords) {
// There is nothing really wrong with not be able to notify the promise as we may have raced here because // There is nothing really wrong with not be able to notify the promise as we may have raced here because
// of multiple queries that have been executed. Log it with trace level anyway just in case the user // of multiple queries that have been executed. Log it with trace level anyway just in case the user
// wants to better understand what happened. // wants to better understand what happened.
logger.trace("Failed to notify success ({}) to a promise: {}", result, promise); logger.trace("Failed to notify success ({}) to a promise: {}", result, promise);
} }
return notifiedRecords;
} }
private static void tryFailure(Promise<?> promise, Throwable cause) { private static void tryFailure(Promise<?> promise, Throwable cause) {

14
resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java
View File

@ -216,7 +216,12 @@ abstract class DnsResolveContext<T> {
public void operationComplete(Future<List<T>> future) { public void operationComplete(Future<List<T>> future) {
Throwable cause = future.cause(); Throwable cause = future.cause();
if (cause == null) { if (cause == null) {
promise.trySuccess(future.getNow()); final List<T> result = future.getNow();
if (!promise.trySuccess(result)) {
for (T item : result) {
ReferenceCountUtil.safeRelease(item);
}
}
} else { } else {
if (DnsNameResolver.isTransportOrTimeoutError(cause)) { if (DnsNameResolver.isTransportOrTimeoutError(cause)) {
promise.tryFailure(new SearchDomainUnknownHostException(cause, hostname)); promise.tryFailure(new SearchDomainUnknownHostException(cause, hostname));
@ -966,7 +971,12 @@ abstract class DnsResolveContext<T> {
if (finalResult != null) { if (finalResult != null) {
if (!promise.isDone()) { if (!promise.isDone()) {
// Found at least one resolved record. // Found at least one resolved record.
DnsNameResolver.trySuccess(promise, filterResults(finalResult)); final List<T> result = filterResults(finalResult);
if (!DnsNameResolver.trySuccess(promise, result)) {
for (T item : result) {
ReferenceCountUtil.safeRelease(item);
}
}
} }
return; return;
} }