From b83328606fdb7782eae7f26425a189f1a99bfbac Mon Sep 17 00:00:00 2001 From: Norman Maurer Date: Fri, 4 May 2018 14:19:26 +0200 Subject: [PATCH] DefaultDnsCache should store more then one Entry per hostname. (#7906) Motivation: Due a bug we did never store more then one address per hostname in DefaultDnsCache. Modifications: - Correctly store multiple entries per hostname - Add tests Result: DefaultDnsCache correctly stores more then one entry. Also fixes https://github.com/netty/netty/issues/7882 . --- .../netty/resolver/dns/DefaultDnsCache.java | 20 +++++- .../resolver/dns/DefaultDnsCacheTest.java | 66 +++++++++++++++++-- 2 files changed, 78 insertions(+), 8 deletions(-) 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 2bc509fec8..3cbb1a367d 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 @@ -170,9 +170,9 @@ public class DefaultDnsCache implements DnsCache { Entries oldEntries = resolveCache.putIfAbsent(e.hostname(), entries); if (oldEntries != null) { entries = oldEntries; - entries.add(e); } } + entries.add(e); scheduleCacheExpiration(e, ttl, loop); } @@ -288,11 +288,27 @@ public class DefaultDnsCache implements DnsCache { continue; } } + // Create a new List for COW semantics List newEntries = new ArrayList(entries.size() + 1); - newEntries.addAll(entries); + DefaultDnsCacheEntry replacedEntry = null; + for (int i = 0; i < entries.size(); i++) { + DefaultDnsCacheEntry entry = entries.get(i); + // Only add old entry if the address is not the same as the one we try to add as well. + // In this case we will skip it and just add the new entry as this may have + // more up-to-date data and cancel the old after we were able to update the cache. + if (!e.address().equals(entry.address())) { + newEntries.add(entry); + } else { + assert replacedEntry == null; + replacedEntry = entry; + } + } newEntries.add(e); if (compareAndSet(entries, newEntries)) { + if (replacedEntry != null) { + replacedEntry.cancelExpiration(); + } return; } } else if (compareAndSet(entries, Collections.singletonList(e))) { 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 index 6fa8bc7bcb..a73760ff32 100644 --- a/resolver-dns/src/test/java/io/netty/resolver/dns/DefaultDnsCacheTest.java +++ b/resolver-dns/src/test/java/io/netty/resolver/dns/DefaultDnsCacheTest.java @@ -18,31 +18,40 @@ package io.netty.resolver.dns; import io.netty.channel.DefaultEventLoopGroup; import io.netty.channel.EventLoop; import io.netty.channel.EventLoopGroup; + import io.netty.channel.nio.NioEventLoopGroup; import io.netty.util.NetUtil; -import org.junit.Assert; import org.junit.Test; +import java.net.InetAddress; +import java.util.List; import java.util.concurrent.Callable; import java.util.concurrent.TimeUnit; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertNull; + public class DefaultDnsCacheTest { @Test public void testExpire() throws Throwable { + InetAddress addr1 = InetAddress.getByAddress(new byte[] { 10, 0, 0, 1 }); + InetAddress addr2 = InetAddress.getByAddress(new byte[] { 10, 0, 0, 2 }); 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); + cache.cache("netty.io", null, addr1, 1, loop); + cache.cache("netty.io", null, addr2, 10000, loop); Throwable error = loop.schedule(new Callable() { @Override - public Throwable call() throws Exception { + public Throwable call() { try { - Assert.assertNull(cache.get("netty.io", null)); + assertNull(cache.get("netty.io", null)); return null; } catch (Throwable cause) { return cause; @@ -70,9 +79,54 @@ public class DefaultDnsCacheTest { try { EventLoop loop = group.next(); final DefaultDnsCache cache = new DefaultDnsCache(); - Assert.assertNotNull(cache.cache("netty.io", null, NetUtil.LOCALHOST, days, loop)); + assertNotNull(cache.cache("netty.io", null, NetUtil.LOCALHOST, days, loop)); } finally { group.shutdownGracefully(); } } + + @Test + public void testAddMultipleAddressesForSameHostname() throws Exception { + InetAddress addr1 = InetAddress.getByAddress(new byte[] { 10, 0, 0, 1 }); + InetAddress addr2 = InetAddress.getByAddress(new byte[] { 10, 0, 0, 2 }); + EventLoopGroup group = new DefaultEventLoopGroup(1); + + try { + EventLoop loop = group.next(); + final DefaultDnsCache cache = new DefaultDnsCache(); + cache.cache("netty.io", null, addr1, 1, loop); + cache.cache("netty.io", null, addr2, 10000, loop); + + List entries = cache.get("netty.io", null); + assertEquals(2, entries.size()); + assertEntry(entries.get(0), addr1); + assertEntry(entries.get(1), addr2); + } finally { + group.shutdownGracefully(); + } + } + + @Test + public void testAddSameAddressForSameHostname() throws Exception { + InetAddress addr1 = InetAddress.getByAddress(new byte[] { 10, 0, 0, 1 }); + EventLoopGroup group = new DefaultEventLoopGroup(1); + + try { + EventLoop loop = group.next(); + final DefaultDnsCache cache = new DefaultDnsCache(); + cache.cache("netty.io", null, addr1, 1, loop); + cache.cache("netty.io", null, addr1, 10000, loop); + + List entries = cache.get("netty.io", null); + assertEquals(1, entries.size()); + assertEntry(entries.get(0), addr1); + } finally { + group.shutdownGracefully(); + } + } + + private static void assertEntry(DnsCacheEntry entry, InetAddress address) { + assertEquals(address, entry.address()); + assertNull(entry.cause()); + } }