Cancel all fetch events in dns_resolver_cancelfetch()

Although 'dns_fetch_t' fetch can have two associated events, one for
each of 'DNS_EVENT_FETCHDONE' and 'DNS_EVENT_TRYSTALE' types, the
dns_resolver_cancelfetch() function is designed in a way that it
expects only one existing event, which it must cancel, and when it
happens so that 'stale-answer-client-timeout' is enabled and there
are two events, only one of them is canceled, and it results in an
assertion in dns_resolver_destroyfetch(), when it finds a dangling
event.

Change the logic of dns_resolver_cancelfetch() function so that it
cancels both the events (if they exist), and in the right order.

(cherry picked from commit ec2098ca35039e4f81fd0aa7c525eb960b8f47bf)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index 9dd98bac5880c17b83aaf351b87b7d09e224d7d2..7ec68bcc1c9fad0baf81787476f4c4f78be5aeb6 100644 (file)
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -11275,8 +11275,9 @@ void
 dns_resolver_cancelfetch(dns_fetch_t *fetch) {
        fetchctx_t *fctx;
        dns_resolver_t *res;
-       dns_fetchevent_t *event, *next_event;
-       isc_task_t *etask;
+       dns_fetchevent_t *event = NULL;
+       dns_fetchevent_t *event_trystale = NULL;
+       dns_fetchevent_t *event_fetchdone = NULL;
 
        REQUIRE(DNS_FETCH_VALID(fetch));
        fctx = fetch->private;
@@ -11288,33 +11289,60 @@ dns_resolver_cancelfetch(dns_fetch_t *fetch) {
        LOCK(&res->buckets[fctx->bucketnum].lock);
 
        /*
-        * Find the completion event for this fetch (as opposed
+        * Find the events for this fetch (as opposed
         * to those for other fetches that have joined the same
-        * fctx) and send it with result = ISC_R_CANCELED.
+        * fctx) and send them with result = ISC_R_CANCELED.
         */
-       event = NULL;
        if (fctx->state != fetchstate_done) {
+               dns_fetchevent_t *next_event = NULL;
                for (event = ISC_LIST_HEAD(fctx->events); event != NULL;
                     event = next_event)
                {
                        next_event = ISC_LIST_NEXT(event, ev_link);
                        if (event->fetch == fetch) {
                                ISC_LIST_UNLINK(fctx->events, event, ev_link);
-                               break;
+                               switch (event->ev_type) {
+                               case DNS_EVENT_TRYSTALE:
+                                       INSIST(event_trystale == NULL);
+                                       event_trystale = event;
+                                       break;
+                               case DNS_EVENT_FETCHDONE:
+                                       INSIST(event_fetchdone == NULL);
+                                       event_fetchdone = event;
+                                       break;
+                               default:
+                                       UNREACHABLE();
+                               }
+                               if (event_trystale != NULL &&
+                                   event_fetchdone != NULL)
+                               {
+                                       break;
+                               }
                        }
                }
        }
-       if (event != NULL) {
-               etask = event->ev_sender;
-               event->ev_sender = fctx;
-               event->result = ISC_R_CANCELED;
-               isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event));
+       /*
+        * The "trystale" event must be sent before the "fetchdone" event,
+        * because the latter clears the "recursing" query attribute, which is
+        * required by both events (handled by the same callback function).
+        */
+       if (event_trystale != NULL) {
+               isc_task_t *etask = event_trystale->ev_sender;
+               event_trystale->ev_sender = fctx;
+               event_trystale->result = ISC_R_CANCELED;
+               isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_trystale));
+       }
+       if (event_fetchdone != NULL) {
+               isc_task_t *etask = event_fetchdone->ev_sender;
+               event_fetchdone->ev_sender = fctx;
+               event_fetchdone->result = ISC_R_CANCELED;
+               isc_task_sendanddetach(&etask, ISC_EVENT_PTR(&event_fetchdone));
        }
+
        /*
         * The fctx continues running even if no fetches remain;
         * the answer is still cached.
         */
-
        UNLOCK(&res->buckets[fctx->bucketnum].lock);
 }
 

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 348aee8b3f657938d020e1b1e5c5f47b7aa5485c..d8c3a182188e87333ef33d61682858cf5e2414c0 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -6110,7 +6110,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
        CTRACE(ISC_LOG_DEBUG(3), "fetch_callback");
 
        if (event->ev_type == DNS_EVENT_TRYSTALE) {
-               query_lookup_stale(client);
+               if (devent->result != ISC_R_CANCELED) {
+                       query_lookup_stale(client);
+               }
                isc_event_free(ISC_EVENT_PTR(&event));
                return;
        }