DNS Resolver should be more consistent with JDK resolution

Motivation:
If there are multiple DNS servers to query Java's DNS resolver will attempt to resolve A and AAAA records in sequential order and will terminate with a failure once all DNS servers have been exhausted. Netty's DNS server will share the same DnsServerAddressStream for the different record types which may send the A question to the first host and the AAAA question to the second host. Netty's DNS resolution also may not progress to the next DNS server in all situations and doesn't have a means to know when resolution has completed.

Modifications:
- DnsServerAddressStream should support new methods to allow the same stream to be used to issue multiple queries (e.g. A and AAAA) against the same host.
- DnsServerAddressStream should support a method to determine when the stream will start to repeat, and therefore a failure can be returned.
- Introduce SequentialDnsServerAddressStreamProvider for sequential use cases

Result:
Fixes netty#6926.

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

@@ -80,13 +80,17 @@ public void operationComplete(Future<AddressedEnvelope<DnsResponse, InetSocketAd
             DnsNameResolverContext.class,
             "onResponseCNAME(..)");
     private static final RuntimeException NO_MATCHING_RECORD_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
-            new RuntimeException("No matching record type record found"),
+            new RuntimeException("No matching record type found"),
             DnsNameResolverContext.class,
             "onResponseAorAAAA(..)");
     private static final RuntimeException UNRECOGNIZED_TYPE_QUERY_FAILED_EXCEPTION = ThrowableUtil.unknownStackTrace(
             new RuntimeException("Response type was unrecognized"),
             DnsNameResolverContext.class,
             "onResponse(..)");
+    private static final RuntimeException NAME_SERVERS_EXHAUSTED_EXCEPTION = ThrowableUtil.unknownStackTrace(
+            new RuntimeException("No name servers returned an answer"),
+            DnsNameResolverContext.class,
+            "tryToFinishResolve(..)");
 
     private final DnsNameResolver parent;
     private final DnsServerAddressStream nameServerAddrs;
@@ -174,11 +178,15 @@ private void doSearchDomainQuery(int count, FutureListener<T> listener) {
     private void internalResolve(Promise<T> promise) {
         DnsServerAddressStream nameServerAddressStream = getNameServers(hostname);
 
-        for (DnsRecordType type: parent.resolveRecordTypes()) {
-            if (!query(hostname, type, nameServerAddressStream, promise)) {
+        DnsRecordType[] recordTypes = parent.resolveRecordTypes();
+        assert recordTypes.length > 0;
+        final int end = recordTypes.length - 1;
+        for (int i = 0; i < end; ++i) {
+            if (!query(hostname, recordTypes[i], nameServerAddressStream.duplicate(), promise)) {
                 return;
             }
         }
+        query(hostname, recordTypes[end], nameServerAddressStream, promise);
     }
 
     /**
@@ -266,23 +274,25 @@ public void remove() {
         }
     }
 
-    private void query(final DnsServerAddressStream nameServerAddrStream, final DnsQuestion question,
+    private void query(final DnsServerAddressStream nameServerAddrStream, final int nameServerAddrStreamIndex,
+                       final DnsQuestion question,
                        final Promise<T> promise) {
-        query(nameServerAddrStream, question, parent.dnsQueryLifecycleObserverFactory()
-                                                    .newDnsQueryLifecycleObserver(question), promise);
+        query(nameServerAddrStream, nameServerAddrStreamIndex, question,
+                parent.dnsQueryLifecycleObserverFactory().newDnsQueryLifecycleObserver(question), promise);
     }
 
-    private void query(final DnsServerAddressStream nameServerAddrStream, final DnsQuestion question,
+    private void query(final DnsServerAddressStream nameServerAddrStream,
+                       final int nameServerAddrStreamIndex,
+                       final DnsQuestion question,
                        final DnsQueryLifecycleObserver queryLifecycleObserver,
                        final Promise<T> promise) {
-        if (allowedQueries == 0 || promise.isCancelled()) {
-            queryLifecycleObserver.queryCancelled(allowedQueries);
-            tryToFinishResolve(promise);
+        if (nameServerAddrStreamIndex >= nameServerAddrStream.size() || allowedQueries == 0 || promise.isCancelled()) {
+            tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question, queryLifecycleObserver,
+                               promise);
             return;
         }
 
-        allowedQueries --;
-
+        --allowedQueries;
         final InetSocketAddress nameServerAddr = nameServerAddrStream.next();
         final ChannelPromise writePromise = parent.ch.newPromise();
         final Future<AddressedEnvelope<DnsResponse, InetSocketAddress>> f = parent.query0(
@@ -304,21 +314,26 @@ public void operationComplete(Future<AddressedEnvelope<DnsResponse, InetSocketAd
 
                 try {
                     if (future.isSuccess()) {
-                        onResponse(nameServerAddrStream, question, future.getNow(), queryLifecycleObserver, promise);
+                        onResponse(nameServerAddrStream, nameServerAddrStreamIndex, question, future.getNow(),
+                                   queryLifecycleObserver, promise);
                     } else {
                         // Server did not respond or I/O error occurred; try again.
                         queryLifecycleObserver.queryFailed(future.cause());
-                        query(nameServerAddrStream, question, promise);
+                        query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise);
                     }
                 } finally {
-                    tryToFinishResolve(promise);
+                    tryToFinishResolve(nameServerAddrStream, nameServerAddrStreamIndex, question,
+                                       // queryLifecycleObserver has already been terminated at this point so we must
+                                       // not allow it to be terminated again by tryToFinishResolve.
+                                       NoopDnsQueryLifecycleObserver.INSTANCE,
+                                       promise);
                 }
             }
         });
     }
 
-    void onResponse(final DnsServerAddressStream nameServerAddrStream, final DnsQuestion question,
-                    AddressedEnvelope<DnsResponse, InetSocketAddress> envelope,
+    void onResponse(final DnsServerAddressStream nameServerAddrStream, final int nameServerAddrStreamIndex,
+                    final DnsQuestion question, AddressedEnvelope<DnsResponse, InetSocketAddress> envelope,
                     final DnsQueryLifecycleObserver queryLifecycleObserver,
                     Promise<T> promise) {
         try {
@@ -343,7 +358,8 @@ void onResponse(final DnsServerAddressStream nameServerAddrStream, final DnsQues
 
             // Retry with the next server if the server did not tell us that the domain does not exist.
             if (code != DnsResponseCode.NXDOMAIN) {
-                query(nameServerAddrStream, question, queryLifecycleObserver.queryNoAnswer(code), promise);
+                query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question,
+                      queryLifecycleObserver.queryNoAnswer(code), promise);
             } else {
                 queryLifecycleObserver.queryFailed(NXDOMAIN_QUERY_FAILED_EXCEPTION);
             }
@@ -396,7 +412,7 @@ private boolean handleRedirect(
                 }
 
                 if (!nameServers.isEmpty()) {
-                    query(parent.uncachedRedirectDnsServerStream(nameServers), question,
+                    query(parent.uncachedRedirectDnsServerStream(nameServers), 0, question,
                           queryLifecycleObserver.queryRedirected(unmodifiableList(nameServers)), promise);
                     return true;
                 }
@@ -539,7 +555,7 @@ private void onResponseCNAME(
         }
 
         if (found) {
-            followCname(response.sender(), name, resolved, queryLifecycleObserver, promise);
+            followCname(resolved, queryLifecycleObserver, promise);
         } else {
             queryLifecycleObserver.queryFailed(CNAME_NOT_FOUND_QUERY_FAILED_EXCEPTION);
         }
@@ -575,8 +591,15 @@ private static Map<String, String> buildAliasMap(DnsResponse response) {
         return cnames != null? cnames : Collections.<String, String>emptyMap();
     }
 
-    void tryToFinishResolve(Promise<T> promise) {
+    void tryToFinishResolve(final DnsServerAddressStream nameServerAddrStream,
+                            final int nameServerAddrStreamIndex,
+                            final DnsQuestion question,
+                            final DnsQueryLifecycleObserver queryLifecycleObserver,
+                            final Promise<T> promise) {
+        // There are no queries left to try.
         if (!queriesInProgress.isEmpty()) {
+            queryLifecycleObserver.queryCancelled(allowedQueries);
+
             // There are still some queries we did not receive responses for.
             if (gotPreferredAddress()) {
                 // But it's OK to finish the resolution process if we got a resolved address of the preferred type.
@@ -589,6 +612,20 @@ void tryToFinishResolve(Promise<T> promise) {
 
         // There are no queries left to try.
         if (resolvedEntries == null) {
+            if (nameServerAddrStreamIndex < nameServerAddrStream.size()) {
+                if (queryLifecycleObserver == NoopDnsQueryLifecycleObserver.INSTANCE) {
+                    // If the queryLifecycleObserver has already been terminated we should create a new one for this
+                    // fresh query.
+                    query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, promise);
+                } else {
+                    query(nameServerAddrStream, nameServerAddrStreamIndex + 1, question, queryLifecycleObserver,
+                          promise);
+                }
+                return;
+            }
+
+            queryLifecycleObserver.queryFailed(NAME_SERVERS_EXHAUSTED_EXCEPTION);
+
             // .. and we could not find any A/AAAA records.
             if (!triedCNAME) {
                 // As the last resort, try to query CNAME, just in case the name server has it.
@@ -597,6 +634,8 @@ void tryToFinishResolve(Promise<T> promise) {
                 query(hostname, DnsRecordType.CNAME, getNameServers(hostname), promise);
                 return;
             }
+        } else {
+            queryLifecycleObserver.queryCancelled(allowedQueries);
         }
 
         // We have at least one resolved address or tried CNAME as the last resort..
@@ -688,9 +727,7 @@ private DnsServerAddressStream getNameServers(String hostname) {
         return stream == null ? nameServerAddrs : stream;
     }
 
-    private void followCname(InetSocketAddress nameServerAddr, String name, String cname,
-                             final DnsQueryLifecycleObserver queryLifecycleObserver,
-                             Promise<T> promise) {
+    private void followCname(String cname, final DnsQueryLifecycleObserver queryLifecycleObserver, Promise<T> promise) {
         // Use the same server for both CNAME queries
         DnsServerAddressStream stream = DnsServerAddresses.singleton(getNameServers(cname).next()).stream();
 
@@ -704,7 +741,7 @@ private void followCname(InetSocketAddress nameServerAddr, String name, String c
                 queryLifecycleObserver.queryFailed(cause);
                 PlatformDependent.throwException(cause);
             }
-            query(stream, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
+            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
         }
         if (parent.supportsAAAARecords()) {
             try {
@@ -715,7 +752,7 @@ private void followCname(InetSocketAddress nameServerAddr, String name, String c
                 queryLifecycleObserver.queryFailed(cause);
                 PlatformDependent.throwException(cause);
             }
-            query(stream, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
+            query(stream, 0, cnameQuestion, queryLifecycleObserver.queryCNAMEd(cnameQuestion), promise);
         }
     }
 
@@ -725,7 +762,7 @@ private boolean query(String hostname, DnsRecordType type, DnsServerAddressStrea
         if (question == null) {
             return false;
         }
-        query(dnsServerAddressStream, question, promise);
+        query(dnsServerAddressStream, 0, question, promise);
         return true;
     }
 

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

@@ -29,4 +29,19 @@ public interface DnsServerAddressStream {
      * Retrieves the next DNS server address from the stream.
      */
     InetSocketAddress next();
+
+    /**
+     * Get the number of times {@link #next()} will return a distinct element before repeating or terminating.
+     * @return the number of times {@link #next()} will return a distinct element before repeating or terminating.
+     */
+    int size();
+
+    /**
+     * Duplicate this object. The result of this should be able to be independently iterated over via {@link #next()}.
+     * <p>
+     * Note that {@link #clone()} isn't used because it may make sense for some implementations to have the following
+     * relationship {@code x.duplicate() == x}.
+     * @return A duplicate of this object.
+     */
+    DnsServerAddressStream duplicate();
 }

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

@@ -40,6 +40,16 @@ public InetSocketAddress next() {
         return next;
     }
 
+    @Override
+    public int size() {
+        return addresses.length;
+    }
+
+    @Override
+    public SequentialDnsServerAddressStream duplicate() {
+        return new SequentialDnsServerAddressStream(addresses, i);
+    }
+
     @Override
     public String toString() {
         return toString("sequential", i, addresses);

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

@@ -0,0 +1,46 @@
+/*
+ * Copyright 2017 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.util.internal.UnstableApi;
+
+import java.net.InetSocketAddress;
+
+import static io.netty.resolver.dns.DnsServerAddresses.sequential;
+
+/**
+ * A {@link DnsServerAddressStreamProvider} which is backed by a sequential list of DNS servers.
+ */
+@UnstableApi
+public final class SequentialDnsServerAddressStreamProvider extends UniSequentialDnsServerAddressStreamProvider {
+    /**
+     * Create a new instance.
+     * @param addresses The addresses which will be be returned in sequential order via
+     * {@link #nameServerAddressStream(String)}
+     */
+    public SequentialDnsServerAddressStreamProvider(InetSocketAddress... addresses) {
+        super(sequential(addresses));
+    }
+
+    /**
+     * Create a new instance.
+     * @param addresses The addresses which will be be returned in sequential order via
+     * {@link #nameServerAddressStream(String)}
+     */
+    public SequentialDnsServerAddressStreamProvider(Iterable<? extends InetSocketAddress> addresses) {
+        super(sequential(addresses));
+    }
+}

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

@@ -26,12 +26,22 @@ final class ShuffledDnsServerAddressStream implements DnsServerAddressStream {
     private final InetSocketAddress[] addresses;
     private int i;
 
+    /**
+     * Create a new instance.
+     * @param addresses The addresses are not cloned. It is assumed the caller has cloned this array or otherwise will
+     *                  not modify the contents.
+     */
     ShuffledDnsServerAddressStream(InetSocketAddress[] addresses) {
-        this.addresses = addresses.clone();
+        this.addresses = addresses;
 
         shuffle();
     }
 
+    private ShuffledDnsServerAddressStream(InetSocketAddress[] addresses, int startIdx) {
+        this.addresses = addresses;
+        i = startIdx;
+    }
+
     private void shuffle() {
         final InetSocketAddress[] addresses = this.addresses;
         final Random r = PlatformDependent.threadLocalRandom();
@@ -57,6 +67,16 @@ public InetSocketAddress next() {
         return next;
     }
 
+    @Override
+    public int size() {
+        return addresses.length;
+    }
+
+    @Override
+    public ShuffledDnsServerAddressStream duplicate() {
+        return new ShuffledDnsServerAddressStream(addresses, i);
+    }
+
     @Override
     public String toString() {
         return SequentialDnsServerAddressStream.toString("shuffled", i, addresses);

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

@@ -23,19 +23,12 @@
  * A {@link DnsServerAddressStreamProvider} which always uses a single DNS server for resolution.
  */
 @UnstableApi
-public final class SingletonDnsServerAddressStreamProvider implements DnsServerAddressStreamProvider {
-    private final DnsServerAddresses addresses;
-
+public final class SingletonDnsServerAddressStreamProvider extends UniSequentialDnsServerAddressStreamProvider {
     /**
      * Create a new instance.
      * @param address The singleton address to use for every DNS resolution.
      */
     public SingletonDnsServerAddressStreamProvider(final InetSocketAddress address) {
-        addresses = DnsServerAddresses.singleton(address);
-    }
-
-    @Override
-    public DnsServerAddressStream nameServerAddressStream(String hostname) {
-        return addresses.stream();
+        super(DnsServerAddresses.singleton(address));
     }
 }