clean up query correctly if already answered by serve-stale

when a serve-stale answer has been sent, the client continues waiting
for a proper answer. if a final completion event for the client does
arrive, it can just be cleaned up without sending a response, similar
to a canceled fetch.

diff --git a/lib/ns/include/ns/query.h b/lib/ns/include/ns/query.h
index 74a95c9b9448ac9a4f4f624ee158b474d243f536..cee2e1796c4a04e7c42d7fddf0a9beaaed35b0f9 100644 (file)
--- a/lib/ns/include/ns/query.h
+++ b/lib/ns/include/ns/query.h
@@ -100,25 +100,26 @@ struct ns_query {
        bool         root_key_sentinel_not_ta;
 };
 
-#define NS_QUERYATTR_RECURSIONOK     0x00001
-#define NS_QUERYATTR_CACHEOK        0x00002
-#define NS_QUERYATTR_PARTIALANSWER   0x00004
-#define NS_QUERYATTR_NAMEBUFUSED     0x00008
-#define NS_QUERYATTR_RECURSING      0x00010
-#define NS_QUERYATTR_QUERYOKVALID    0x00040
-#define NS_QUERYATTR_QUERYOK        0x00080
-#define NS_QUERYATTR_WANTRECURSION   0x00100
-#define NS_QUERYATTR_SECURE         0x00200
-#define NS_QUERYATTR_NOAUTHORITY     0x00400
-#define NS_QUERYATTR_NOADDITIONAL    0x00800
-#define NS_QUERYATTR_CACHEACLOKVALID 0x01000
-#define NS_QUERYATTR_CACHEACLOK             0x02000
-#define NS_QUERYATTR_DNS64          0x04000
-#define NS_QUERYATTR_DNS64EXCLUDE    0x08000
-#define NS_QUERYATTR_RRL_CHECKED     0x10000
-#define NS_QUERYATTR_REDIRECT       0x20000
-#define NS_QUERYATTR_ANSWERED       0x40000
-#define NS_QUERYATTR_STALEOK        0x80000
+#define NS_QUERYATTR_RECURSIONOK     0x000001
+#define NS_QUERYATTR_CACHEOK        0x000002
+#define NS_QUERYATTR_PARTIALANSWER   0x000004
+#define NS_QUERYATTR_NAMEBUFUSED     0x000008
+#define NS_QUERYATTR_RECURSING      0x000010
+#define NS_QUERYATTR_QUERYOKVALID    0x000040
+#define NS_QUERYATTR_QUERYOK        0x000080
+#define NS_QUERYATTR_WANTRECURSION   0x000100
+#define NS_QUERYATTR_SECURE         0x000200
+#define NS_QUERYATTR_NOAUTHORITY     0x000400
+#define NS_QUERYATTR_NOADDITIONAL    0x000800
+#define NS_QUERYATTR_CACHEACLOKVALID 0x001000
+#define NS_QUERYATTR_CACHEACLOK             0x002000
+#define NS_QUERYATTR_DNS64          0x004000
+#define NS_QUERYATTR_DNS64EXCLUDE    0x008000
+#define NS_QUERYATTR_RRL_CHECKED     0x010000
+#define NS_QUERYATTR_REDIRECT       0x020000
+#define NS_QUERYATTR_ANSWERED       0x040000
+#define NS_QUERYATTR_STALEOK        0x080000
+#define NS_QUERYATTR_STALEPENDING    0x100000
 
 typedef struct query_ctx query_ctx_t;
 

diff --git a/lib/ns/query.c b/lib/ns/query.c
index 2ef0d67c91b541e6d28f5898d627998e4bd1b7bf..d3546f9d5cca7b8918fc3fd76e2bb24ed0981df0 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -134,9 +134,13 @@
 
 #define REDIRECT(c) (((c)->query.attributes & NS_QUERYATTR_REDIRECT) != 0)
 
-/*% Was the query already answered due to stale-answer-client-timeout? */
+/*% Was the client already sent a response? */
 #define QUERY_ANSWERED(q) (((q)->attributes & NS_QUERYATTR_ANSWERED) != 0)
 
+/*% Have we already processed an answer via stale-answer-client-timeout? */
+#define QUERY_STALEPENDING(q) \
+       (((q)->attributes & NS_QUERYATTR_STALEPENDING) != 0)
+
 /*% Does the query allow stale data in the response? */
 #define QUERY_STALEOK(q) (((q)->attributes & NS_QUERYATTR_STALEOK) != 0)
 
@@ -5926,7 +5930,7 @@ query_lookup(query_ctx_t *qctx) {
                                        dns_resolver_destroyfetch(
                                                &qctx->client->query.fetch);
                                }
-                               return query_lookup(qctx);
+                               return (query_lookup(qctx));
                        } else {
                                /*
                                 * Immediately return the stale answer, start a
@@ -5955,6 +5959,13 @@ query_lookup(query_ctx_t *qctx) {
                        if (!stale_found) {
                                return (result);
                        }
+
+                       /*
+                        * There still might be real answer later. Mark the
+                        * query so we'll know we can skip answering.
+                        */
+                       qctx->client->query.attributes |=
+                               NS_QUERYATTR_STALEPENDING;
                }
        }
 
@@ -5967,6 +5978,7 @@ query_lookup(query_ctx_t *qctx) {
                qctx->client->query.attributes |= NS_QUERYATTR_STALEOK;
                qctx->rdataset->attributes |= DNS_RDATASETATTR_STALE_ADDED;
        }
+
        result = query_gotanswer(qctx, result);
 
        if (refresh_rrset) {
@@ -6072,10 +6084,12 @@ static void
 fetch_callback(isc_task_t *task, isc_event_t *event) {
        dns_fetchevent_t *devent = (dns_fetchevent_t *)event;
        dns_fetch_t *fetch = NULL;
-       ns_client_t *client;
-       bool fetch_canceled, client_shuttingdown;
-       isc_result_t result;
+       ns_client_t *client = NULL;
+       bool fetch_canceled = false;
+       bool fetch_answered = false;
+       bool client_shuttingdown = false;
        isc_logcategory_t *logcategory = NS_LOGCATEGORY_QUERY_ERRORS;
+       isc_result_t result;
        int errorloglevel;
        query_ctx_t qctx;
 
@@ -6083,7 +6097,9 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
 
        REQUIRE(event->ev_type == DNS_EVENT_FETCHDONE ||
                event->ev_type == DNS_EVENT_TRYSTALE);
+
        client = devent->ev_arg;
+
        REQUIRE(NS_CLIENT_VALID(client));
        REQUIRE(task == client->task);
        REQUIRE(RECURSING(client));
@@ -6095,7 +6111,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
                isc_event_free(ISC_EVENT_PTR(&event));
                return;
        }
-
        /*
         * We are resuming from recursion. Reset any attributes, options
         * that a lookup due to stale-answer-client-timeout may have set.
@@ -6107,13 +6122,23 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
        client->nodetach = false;
 
        LOCK(&client->query.fetchlock);
-       if (client->query.fetch != NULL) {
+       INSIST(client->query.fetch == devent->fetch ||
+              client->query.fetch == NULL);
+       if (QUERY_STALEPENDING(&client->query)) {
+               /*
+                * We've gotten an authoritative answer to a query that
+                * was left pending after a stale timeout. We don't need
+                * to do anything with it; free all the data and go home.
+                */
+               client->query.fetch = NULL;
+               fetch_answered = true;
+       } else if (client->query.fetch != NULL) {
                /*
                 * This is the fetch we've been waiting for.
                 */
                INSIST(devent->fetch == client->query.fetch);
                client->query.fetch = NULL;
-               fetch_canceled = false;
+
                /*
                 * Update client->now.
                 */
@@ -6126,7 +6151,6 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
                fetch_canceled = true;
        }
        UNLOCK(&client->query.fetchlock);
-       INSIST(client->query.fetch == NULL);
 
        SAVE(fetch, devent->fetch);
 
@@ -6169,7 +6193,7 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
        qctx_init(client, &devent, 0, &qctx);
 
        client_shuttingdown = ns_client_shuttingdown(client);
-       if (fetch_canceled || client_shuttingdown) {
+       if (fetch_canceled || fetch_answered || client_shuttingdown) {
                /*
                 * We've timed out or are shutting down. We can now
                 * free the event and other resources held by qctx, but