Fix a clients-per-query miscalculation bug

The number of clients per query is calculated using the pending
fetch responses in the list. The dns_resolver_createfetch() function
includes every item in the list when deciding whether the limit is
reached (i.e. fctx->spilled is true). Then, when the limit is reached,
there is another calculation in fctx_sendevents(), when deciding
whether it is needed to increase the limit, but this time the TRYSTALE
responses are not included in the calculation (because of early break
from the loop), and because of that the limit is never increased.

A single client can have more than one associated response/event in the
list (currently max. two), and calculating them as separate "clients"
is unexpected. E.g. if 'stale-answer-enable' is enabled and
'stale-answer-client-timeout' is enabled and is larger than 0, then
each client will have two events, which will effectively halve the
clients-per-query limit.

Fix the dns_resolver_createfetch() function to calculate only the
regular FETCHDONE responses/events.

Change the fctx_sendevents() function to also calculate only FETCHDONE
responses/events. Currently, this second change doesn't have any impact,
because the TRYSTALE events were already skipped, but having the same
condition in both places will help prevent similar bugs in the future
if a new type of response/event is ever added.

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index ce0d7bc389852b8eca1065df0dca02feb95d70d4..e7e3c3914dc8da300acd372a184db6ba1c3b3eba 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -1553,6 +1553,16 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) {
        for (resp = ISC_LIST_HEAD(fctx->resps); resp != NULL; resp = next) {
                next = ISC_LIST_NEXT(resp, link);
                ISC_LIST_UNLINK(fctx->resps, resp, link);
+
+               /*
+                * Only the regular fetch events should be counted for the
+                * clients-per-query limit, in case if there are multiple events
+                * registered for a single client.
+                */
+               if (resp->type == FETCHDONE) {
+                       count++;
+               }
+
                if (resp->type == TRYSTALE) {
                        /*
                         * Not applicable to TRYSTALE resps; this function is
@@ -1586,7 +1596,6 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result) {
 
                FCTXTRACE("post response event");
                isc_async_run(resp->loop, resp->cb, resp);
-               count++;
        }
        UNLOCK(&fctx->lock);
 
@@ -10391,7 +10400,16 @@ dns_resolver_createfetch(dns_resolver_t *res, const dns_name_t *name,
                                        result = DNS_R_DUPLICATE;
                                        goto unlock;
                                }
-                               count++;
+
+                               /*
+                                * Only the regular fetch events should be
+                                * counted for the clients-per-query limit, in
+                                * case if there are multiple events registered
+                                * for a single client.
+                                */
+                               if (resp->type == FETCHDONE) {
+                                       count++;
+                               }
                        }
                }
                if (count >= spillatmin && spillatmin != 0) {