Use allocation free algorithm to detect CNAME cache loops (#10291)

Motivation:

We did use a HashSet to detect CNAME cache loops which needs allocations. We can use an algorithm that doesnt need any allocations

Modifications:

Use algorithm that doesnt need allocations

Result:

Less allocations on the slow path

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

@@ -49,7 +49,6 @@ import java.util.AbstractList;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashMap;
-import java.util.HashSet;
 import java.util.IdentityHashMap;
 import java.util.Iterator;
 import java.util.List;
@@ -278,8 +277,9 @@ abstract class DnsResolveContext<T> {
 
     // Resolve the final name from the CNAME cache until there is nothing to follow anymore. This also
     // guards against loops in the cache but early return once a loop is detected.
-    private String cnameResolveFromCache(String name) {
-        DnsCnameCache cnameCache = cnameCache();
+    //
+    // Visible for testing only
+    static String cnameResolveFromCache(DnsCnameCache cnameCache, String name) {
         String first = cnameCache.get(hostnameWithDot(name));
         if (first == null) {
             // Nothing in the cache at all
@@ -300,28 +300,30 @@ abstract class DnsResolveContext<T> {
         return cnameResolveFromCacheLoop(cnameCache, first, second);
     }
 
-    private String cnameResolveFromCacheLoop(DnsCnameCache cnameCache, String first, String mapping) {
-        // Detect loops using a HashSet. We use this as last resort implementation to reduce allocations in the most
-        // common cases.
-        Set<String> cnames = new HashSet<String>(4);
-        cnames.add(first);
-        cnames.add(mapping);
+    static String cnameResolveFromCacheLoop(DnsCnameCache cnameCache, String first, String mapping) {
+        // Detect loops by advance only every other iteration.
+        // See https://en.wikipedia.org/wiki/Cycle_detection#Floyd's_Tortoise_and_Hare
+        boolean advance = false;
 
         String name = mapping;
         // Resolve from cnameCache() until there is no more cname entry cached.
         while ((mapping = cnameCache.get(hostnameWithDot(name))) != null) {
-            if (!cnames.add(mapping)) {
+            if (first.equals(mapping)) {
                 // Follow CNAME from cache would loop. Lets break here.
                 break;
             }
             name = mapping;
+            if (advance) {
+                first = cnameCache.get(first);
+            }
+            advance = !advance;
         }
         return name;
     }
 
     private void internalResolve(String name, Promise<List<T>> promise) {
         // Resolve from cnameCache() until there is no more cname entry cached.
-        name = cnameResolveFromCache(name);
+        name = cnameResolveFromCache(cnameCache(), name);
 
         try {
             DnsServerAddressStream nameServerAddressStream = getNameServers(name);
@@ -993,7 +995,7 @@ abstract class DnsResolveContext<T> {
 
     private void followCname(DnsQuestion question, String cname, DnsQueryLifecycleObserver queryLifecycleObserver,
                              Promise<List<T>> promise) {
-        cname = cnameResolveFromCache(cname);
+        cname = cnameResolveFromCache(cnameCache(), cname);
         DnsServerAddressStream stream = getNameServers(cname);
 
         final DnsQuestion cnameQuestion;

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

@@ -0,0 +1,48 @@
+/*
+ * Copyright 2020 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.embedded.EmbeddedChannel;
+import org.junit.Test;
+
+public class DnsResolveContextTest {
+
+    private static final String HOSTNAME = "netty.io.";
+
+    @Test
+    public void testCnameLoop() {
+        for (int i = 1; i < 128; i++) {
+            DnsResolveContext.cnameResolveFromCache(buildCache(i), HOSTNAME);
+        }
+    }
+
+    private static DnsCnameCache buildCache(int chainLength) {
+        EmbeddedChannel channel = new EmbeddedChannel();
+        DnsCnameCache cache = new DefaultDnsCnameCache();
+        if (chainLength == 1) {
+            cache.cache(HOSTNAME, HOSTNAME, Long.MAX_VALUE, channel.eventLoop());
+        } else {
+            String lastName = HOSTNAME;
+            for (int i = 1; i < chainLength; i++) {
+                String nextName = i + "." + lastName;
+                cache.cache(lastName, nextName, Long.MAX_VALUE, channel.eventLoop());
+                lastName = nextName;
+            }
+            cache.cache(lastName, HOSTNAME, Long.MAX_VALUE, channel.eventLoop());
+        }
+        return cache;
+    }
+}