Check recursions at the end of request processing

ns_client_endrequest() currently contains code that looks for
outstanding quota references and cleans them up if necessary.  This
approach masks programming errors because ns_client_endrequest() is only
called from ns__client_reset_cb(), which in turn is only called when all
references to the client's netmgr handle are released, which in turn
only happens after all recursion completion callbacks are invoked
(because isc_nmhandle_attach() is called before every call to
dns_resolver_createfetch() in lib/ns/query.c and the completion callback
is expected to detach from the handle), which in turn is expected to
happen for all recursions attempts, even those that get canceled.

Furthermore, declaring the prototype of ns_client_endrequest() at the
top of lib/ns/client.c is redundant because the definition of that
function is placed before its first use in that file.  Remove the
redundant function prototype.

Finally, remove INSIST assertions ensuring quota pointers are NULL in
ns__client_reset_cb() because the latter calls ns_client_endrequest() a
few lines earlier.

diff --git a/lib/ns/client.c b/lib/ns/client.c
index ebfcc77ba985e7dddbb72d623a9f8d61f7d9fa04..c6b268b9748d2f4f8712112954d003a0b259cb0c 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -124,8 +124,6 @@ clientmgr_detach(ns_clientmgr_t **mp);
 static void
 clientmgr_destroy(ns_clientmgr_t *manager);
 static void
-ns_client_endrequest(ns_client_t *client);
-static void
 ns_client_dumpmessage(ns_client_t *client, const char *reason);
 static void
 compute_cookie(ns_client_t *client, uint32_t when, uint32_t nonce,
@@ -263,16 +261,10 @@ ns_client_endrequest(ns_client_t *client) {
        dns_message_reset(client->message, DNS_MESSAGE_INTENTPARSE);
 
        /*
-        * Clean up from recursion - normally this would be done in
-        * fetch_callback(), but if we're shutting down and canceling then
-        * it might not have happened.
+        * Ensure there are no recursions that still need to be cleaned up.
         */
        for (int i = 0; i < RECTYPE_COUNT; i++) {
-               if (client->query.recursions[i].quota != NULL) {
-                       isc_quota_detach(&client->query.recursions[i].quota);
-                       ns_stats_decrement(client->manager->sctx->nsstats,
-                                          ns_statscounter_recursclients);
-               }
+               INSIST(client->query.recursions[i].quota == NULL);
        }
 
        /*
@@ -1646,9 +1638,6 @@ ns__client_reset_cb(void *client0) {
        }
 
        client->state = NS_CLIENTSTATE_READY;
-       for (int i = 0; i < RECTYPE_COUNT; i++) {
-               INSIST(client->query.recursions[i].quota == NULL);
-       }
 
 #ifdef WANT_SINGLETRACE
        isc_log_setforcelog(false);