From 20424b3bfe8d3fae92c11a30e79aeffd26dc2891 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Mon, 14 Nov 2022 12:18:06 +0000 Subject: [PATCH] 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) --- lib/dns/resolver.c | 53 +++++++++++++++++++++++++++++++++++----------- lib/ns/query.c | 4 +++- 2 files changed, 44 insertions(+), 13 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 18585b5..7cbfbb2 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -11254,8 +11254,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; @@ -11267,32 +11268,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: + ISC_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 f66bab4..4f61374 100644 --- a/lib/ns/query.c +++ b/lib/ns/query.c @@ -6021,7 +6021,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; } -- 2.39.1