From 1be1523a9e844098479f7edea7ba42383ca23d6f Mon Sep 17 00:00:00 2001 From: Koji Lin Date: Mon, 3 Aug 2020 14:58:03 +0900 Subject: [PATCH] Fix DnsNameResolver may have LEAK ByteBuf after cancelling the returned future (#10448) 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. --- .../io/netty/resolver/dns/DnsNameResolver.java | 6 ++++-- .../io/netty/resolver/dns/DnsResolveContext.java | 14 ++++++++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java index 78aa3fb5dd..62cce5ec63 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolver.java @@ -917,13 +917,15 @@ public class DnsNameResolver extends InetNameResolver { } } - static void trySuccess(Promise promise, T result) { - if (!promise.trySuccess(result)) { + static boolean trySuccess(Promise promise, T 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 // of multiple queries that have been executed. Log it with trace level anyway just in case the user // wants to better understand what happened. logger.trace("Failed to notify success ({}) to a promise: {}", result, promise); } + return notifiedRecords; } private static void tryFailure(Promise promise, Throwable cause) { diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java index 02a87fbb75..31d17e54b1 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DnsResolveContext.java @@ -205,7 +205,12 @@ abstract class DnsResolveContext { public void operationComplete(Future> future) { Throwable cause = future.cause(); if (cause == null) { - promise.trySuccess(future.getNow()); + final List result = future.getNow(); + if (!promise.trySuccess(result)) { + for (T item : result) { + ReferenceCountUtil.safeRelease(item); + } + } } else { if (DnsNameResolver.isTransportOrTimeoutError(cause)) { promise.tryFailure(new SearchDomainUnknownHostException(cause, hostname)); @@ -948,7 +953,12 @@ abstract class DnsResolveContext { if (finalResult != null) { if (!promise.isDone()) { // Found at least one resolved record. - DnsNameResolver.trySuccess(promise, filterResults(finalResult)); + final List result = filterResults(finalResult); + if (!DnsNameResolver.trySuccess(promise, result)) { + for (T item : result) { + ReferenceCountUtil.safeRelease(item); + } + } } return; }