diff --git a/resolver-dns/src/main/java/io/netty/resolver/dns/DefaultDnsCache.java b/resolver-dns/src/main/java/io/netty/resolver/dns/DefaultDnsCache.java index 2b3477bd38..4d963dfa87 100644 --- a/resolver-dns/src/main/java/io/netty/resolver/dns/DefaultDnsCache.java +++ b/resolver-dns/src/main/java/io/netty/resolver/dns/DefaultDnsCache.java @@ -104,7 +104,7 @@ public class DefaultDnsCache implements DnsCache { Map.Entry e = i.next(); i.remove(); - e.getValue().cancelExpiration(); + e.getValue().clearAndCancel(); } } } @@ -113,7 +113,7 @@ public class DefaultDnsCache implements DnsCache { public boolean clear(String hostname) { checkNotNull(hostname, "hostname"); Entries entries = resolveCache.remove(hostname); - return entries != null && entries.cancelExpiration(); + return entries != null && entries.clearAndCancel(); } private static boolean emptyAdditionals(DnsRecord[] additionals) { @@ -171,18 +171,27 @@ public class DefaultDnsCache implements DnsCache { } } - scheduleCacheExpiration(entries, e, ttl, loop); + scheduleCacheExpiration(e, ttl, loop); } - private void scheduleCacheExpiration(final Entries entries, - final DefaultDnsCacheEntry e, + private void scheduleCacheExpiration(final DefaultDnsCacheEntry e, int ttl, EventLoop loop) { e.scheduleExpiration(loop, new Runnable() { @Override public void run() { - if (entries.remove(e)) { - resolveCache.remove(e.hostname); + // We always remove all entries for a hostname once one entry expire. This is not the + // most efficient to do but this way we can guarantee that if a DnsResolver + // be configured to prefer one ip family over the other we will not return unexpected + // results to the enduser if one of the A or AAAA records has different TTL settings. + // + // As a TTL is just a hint of the maximum time a cache is allowed to cache stuff it's + // completely fine to remove the entry even if the TTL is not reached yet. + // + // See https://github.com/netty/netty/issues/7329 + Entries entries = resolveCache.remove(e.hostname); + if (entries != null) { + entries.clearAndCancel(); } } }, ttl, TimeUnit.SECONDS); @@ -293,43 +302,7 @@ public class DefaultDnsCache implements DnsCache { } } - boolean remove(DefaultDnsCacheEntry entry) { - for (;;) { - List entries = get(); - int size = entries.size(); - if (size == 0) { - return false; - } else if (size == 1) { - if (entries.get(0).equals(entry)) { - // If the list is empty we just return early and so not allocate a new ArrayList. - if (compareAndSet(entries, Collections.emptyList())) { - return true; - } - } else { - return false; - } - } else { - // Just size the new ArrayList as before as we may not find the entry we are looking for and not - // want to cause an extra allocation / memory copy in this case. - // - // Its very likely we find the entry we are looking for so we directly create a new ArrayList - // and fill it. - List newEntries = new ArrayList(size); - for (int i = 0; i < size; i++) { - DefaultDnsCacheEntry e = entries.get(i); - if (!e.equals(entry)) { - newEntries.add(e); - } - } - if (compareAndSet(entries, Collections.unmodifiableList(newEntries))) { - // This will return true if an entry was removed. - return newEntries.size() != size; - } - } - } - } - - boolean cancelExpiration() { + boolean clearAndCancel() { List entries = getAndSet(Collections.emptyList()); if (entries.isEmpty()) { return false; diff --git a/resolver-dns/src/test/java/io/netty/resolver/dns/DefaultDnsCacheTest.java b/resolver-dns/src/test/java/io/netty/resolver/dns/DefaultDnsCacheTest.java new file mode 100644 index 0000000000..7d519e8793 --- /dev/null +++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DefaultDnsCacheTest.java @@ -0,0 +1,58 @@ +/* + * Copyright 2018 The Netty Project + * + * The Netty Project licenses this file to you under the Apache License, + * version 2.0 (the "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at: + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT + * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the + * License for the specific language governing permissions and limitations + * under the License. + */ +package io.netty.resolver.dns; + +import io.netty.channel.DefaultEventLoopGroup; +import io.netty.channel.EventLoop; +import io.netty.channel.EventLoopGroup; +import io.netty.util.NetUtil; +import org.junit.Assert; +import org.junit.Test; + +import java.util.concurrent.Callable; +import java.util.concurrent.TimeUnit; + +public class DefaultDnsCacheTest { + + @Test + public void testExpire() throws Throwable { + EventLoopGroup group = new DefaultEventLoopGroup(1); + + try { + EventLoop loop = group.next(); + final DefaultDnsCache cache = new DefaultDnsCache(); + cache.cache("netty.io", null, NetUtil.LOCALHOST, 1, loop); + cache.cache("netty.io", null, NetUtil.LOCALHOST6, 10000, loop); + + Throwable error = loop.schedule(new Callable() { + @Override + public Throwable call() throws Exception { + try { + Assert.assertNull(cache.get("netty.io", null)); + return null; + } catch (Throwable cause) { + return cause; + } + } + }, 1, TimeUnit.SECONDS).get(); + if (error != null) { + throw error; + } + } finally { + group.shutdownGracefully(); + } + } +}