Fix potential DNS cache invalidation across different EventLoops (#14147)

Motivation:

By default, `DnsAddressResolverGroup` sets up a separate `DNSCache`
for each `EventLoop`. This can lead to cache invalidation when
performing DNS resolution across different EventLoops due to the
potential absence of cached DNS records.

Modification:

- initializing the DNSCache in `DnsNameResolverBuilder` during
`DnsAddressResolverGroup` initialization to enable sharing of DNSCache
among EventLoops under the same `DnsAddressResolverGroup`.
- Add unit test: testSharedDNSCacheAcrossEventLoops

Result:

Fixes #14046.

---------

Co-authored-by: Norman Maurer <[email protected]>
Co-authored-by: Bryce Anderson <[email protected]>

Author: 3 people

resolver-dns/src/main/java/io/netty/resolver/dns/DnsAddressResolverGroup.java

@@ -45,23 +45,31 @@ public class DnsAddressResolverGroup extends AddressResolverGroup<InetSocketAddr
     private final ConcurrentMap<String, Promise<List<InetAddress>>> resolveAllsInProgress = newConcurrentHashMap();
 
     public DnsAddressResolverGroup(DnsNameResolverBuilder dnsResolverBuilder) {
-        this.dnsResolverBuilder = dnsResolverBuilder.copy();
+        this.dnsResolverBuilder = withSharedCaches(dnsResolverBuilder.copy());
     }
 
     public DnsAddressResolverGroup(
             Class<? extends DatagramChannel> channelType,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this.dnsResolverBuilder = new DnsNameResolverBuilder();
+        this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
         dnsResolverBuilder.channelType(channelType).nameServerProvider(nameServerProvider);
     }
 
     public DnsAddressResolverGroup(
             ChannelFactory<? extends DatagramChannel> channelFactory,
             DnsServerAddressStreamProvider nameServerProvider) {
-        this.dnsResolverBuilder = new DnsNameResolverBuilder();
+        this.dnsResolverBuilder = withSharedCaches(new DnsNameResolverBuilder());
         dnsResolverBuilder.channelFactory(channelFactory).nameServerProvider(nameServerProvider);
     }
 
+    private static DnsNameResolverBuilder withSharedCaches(DnsNameResolverBuilder dnsResolverBuilder) {
+        /// To avoid each member of the group having its own cache we either use the configured cache
+        // or create a new one to share among the entire group.
+        return dnsResolverBuilder.resolveCache(dnsResolverBuilder.getOrNewCache())
+                .cnameCache(dnsResolverBuilder.getOrNewCnameCache())
+                .authoritativeDnsServerCache(dnsResolverBuilder.getOrNewAuthoritativeDnsServerCache());
+    }
+
     @SuppressWarnings("deprecation")
     @Override
     protected final AddressResolver<InetSocketAddress> newResolver(EventExecutor executor) throws Exception {

resolver-dns/src/main/java/io/netty/resolver/dns/DnsNameResolverBuilder.java

@@ -513,11 +513,17 @@ public DnsNameResolverBuilder ndots(int ndots) {
         return this;
     }
 
-    private DnsCache newCache() {
+   DnsCache getOrNewCache() {
+        if (this.resolveCache != null) {
+            return this.resolveCache;
+        }
         return new DefaultDnsCache(intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE), intValue(negativeTtl, 0));
     }
 
-    private AuthoritativeDnsServerCache newAuthoritativeDnsServerCache() {
+   AuthoritativeDnsServerCache getOrNewAuthoritativeDnsServerCache() {
+        if (this.authoritativeDnsServerCache != null) {
+            return this.authoritativeDnsServerCache;
+        }
         return new DefaultAuthoritativeDnsServerCache(
                 intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE),
                 // Let us use the sane ordering as DnsNameResolver will be used when returning
@@ -530,7 +536,10 @@ private DnsServerAddressStream newQueryServerAddressStream(
         return new ThreadLocalNameServerAddressStream(dnsServerAddressStreamProvider);
     }
 
-    private DnsCnameCache newCnameCache() {
+   DnsCnameCache getOrNewCnameCache() {
+        if (this.cnameCache != null) {
+            return this.cnameCache;
+        }
         return new DefaultDnsCnameCache(
                 intValue(minTtl, 0), intValue(maxTtl, Integer.MAX_VALUE));
     }
@@ -583,10 +592,9 @@ public DnsNameResolver build() {
             logger.debug("authoritativeDnsServerCache and TTLs are mutually exclusive. TTLs are ignored.");
         }
 
-        DnsCache resolveCache = this.resolveCache != null ? this.resolveCache : newCache();
-        DnsCnameCache cnameCache = this.cnameCache != null ? this.cnameCache : newCnameCache();
-        AuthoritativeDnsServerCache authoritativeDnsServerCache = this.authoritativeDnsServerCache != null ?
-                this.authoritativeDnsServerCache : newAuthoritativeDnsServerCache();
+        DnsCache resolveCache = getOrNewCache();
+        DnsCnameCache cnameCache = getOrNewCnameCache();
+        AuthoritativeDnsServerCache authoritativeDnsServerCache = getOrNewAuthoritativeDnsServerCache();
 
         DnsServerAddressStream queryDnsServerAddressStream = this.queryDnsServerAddressStream != null ?
                 this.queryDnsServerAddressStream : newQueryServerAddressStream(dnsServerAddressStreamProvider);

resolver-dns/src/test/java/io/netty/resolver/dns/DnsAddressResolverGroupTest.java

@@ -20,16 +20,21 @@
 import io.netty.channel.nio.NioEventLoopGroup;
 import io.netty.channel.socket.nio.NioDatagramChannel;
 import io.netty.resolver.AddressResolver;
+import io.netty.resolver.InetSocketAddressResolver;
 import io.netty.util.concurrent.Future;
 import io.netty.util.concurrent.FutureListener;
 import io.netty.util.concurrent.Promise;
 import org.junit.jupiter.api.Test;
 
+import java.net.InetAddress;
+import java.net.InetSocketAddress;
 import java.net.SocketAddress;
 import java.nio.channels.UnsupportedAddressTypeException;
+import java.util.concurrent.ExecutionException;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.instanceOf;
+import static org.junit.jupiter.api.Assertions.assertSame;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
 public class DnsAddressResolverGroupTest {
@@ -66,4 +71,50 @@ public void operationComplete(Future<Object> future) {
             defaultEventLoopGroup.shutdownGracefully();
         }
     }
+
+    @Test
+    public void testSharedDNSCacheAcrossEventLoops() throws InterruptedException, ExecutionException {
+        NioEventLoopGroup group = new NioEventLoopGroup(1);
+        final EventLoop loop = group.next();
+        DnsNameResolverBuilder builder = new DnsNameResolverBuilder()
+                .eventLoop(loop).channelType(NioDatagramChannel.class);
+        DnsAddressResolverGroup resolverGroup = new DnsAddressResolverGroup(builder);
+        DefaultEventLoopGroup defaultEventLoopGroup = new DefaultEventLoopGroup(2);
+        EventLoop eventLoop1 = defaultEventLoopGroup.next();
+        EventLoop eventLoop2 = defaultEventLoopGroup.next();
+        try {
+            final Promise<InetSocketAddress> promise1 = loop.newPromise();
+            InetSocketAddressResolver resolver1 = (InetSocketAddressResolver) resolverGroup.getResolver(eventLoop1);
+            InetAddress address1 =
+                    resolve(resolver1, InetSocketAddress.createUnresolved("netty.io", 80), promise1);
+            final Promise<InetSocketAddress> promise2 = loop.newPromise();
+            InetSocketAddressResolver resolver2 = (InetSocketAddressResolver) resolverGroup.getResolver(eventLoop2);
+            InetAddress address2 =
+                    resolve(resolver2, InetSocketAddress.createUnresolved("netty.io", 80), promise2);
+            assertSame(address1, address2);
+        } finally {
+            resolverGroup.close();
+            group.shutdownGracefully();
+            defaultEventLoopGroup.shutdownGracefully();
+        }
+    }
+
+    private InetAddress resolve(InetSocketAddressResolver resolver, SocketAddress socketAddress,
+                                final Promise<InetSocketAddress> promise)
+            throws InterruptedException, ExecutionException {
+        resolver.resolve(socketAddress)
+                .addListener(new FutureListener<InetSocketAddress>() {
+                    @Override
+                    public void operationComplete(Future<InetSocketAddress> future) {
+                        try {
+                            promise.setSuccess(future.get());
+                        } catch (Throwable cause) {
+                            promise.setFailure(cause);
+                        }
+                    }
+                }).await();
+        promise.sync();
+        InetSocketAddress inetSocketAddress = promise.get();
+        return inetSocketAddress.getAddress();
+    }
 }