Fix ns_statscounter_recursclients counting bug

The incrementing and decrementing of 'ns_statscounter_recursclients'
were not properly balanced: for example, it would be incremented for
a prefetch query but not decremented if the query failed.

This commit ensures that the recursion quota and the recursive clients
counter are always in sync with each other.

diff --git a/lib/ns/client.c b/lib/ns/client.c
index 47af2c717b9498a1ab58c1f078576ff2063fe9da..2b3da21b212a398b71fb09171531e5a591b222e7 100644 (file)
--- a/lib/ns/client.c
+++ b/lib/ns/client.c
@@ -269,10 +269,8 @@ ns_client_endrequest(ns_client_t *client) {
         */
        if (client->recursionquota != NULL) {
                isc_quota_detach(&client->recursionquota);
-               if (client->query.prefetch == NULL) {
-                       ns_stats_decrement(client->sctx->nsstats,
-                                          ns_statscounter_recursclients);
-               }
+               ns_stats_decrement(client->sctx->nsstats,
+                                  ns_statscounter_recursclients);
        }
 
        /*

diff --git a/lib/ns/query.c b/lib/ns/query.c
index afc2e5ea8121280700f57cbefe31e0b6a0d8dcaf..21ad1a34ce9cde2a3aac154e9cff25fb82302a19 100644 (file)
--- a/lib/ns/query.c
+++ b/lib/ns/query.c
@@ -2543,11 +2543,10 @@ prefetch_done(isc_task_t *task, isc_event_t *event) {
         */
        if (client->recursionquota != NULL) {
                isc_quota_detach(&client->recursionquota);
+               ns_stats_decrement(client->sctx->nsstats,
+                                  ns_statscounter_recursclients);
        }
 
-       ns_stats_decrement(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        free_devent(client, &event, &devent);
        isc_nmhandle_detach(&client->prefetchhandle);
 }
@@ -2575,6 +2574,8 @@ query_prefetch(ns_client_t *client, dns_name_t *qname,
                                          &client->recursionquota);
                switch (result) {
                case ISC_R_SUCCESS:
+                       ns_stats_increment(client->sctx->nsstats,
+                                          ns_statscounter_recursclients);
                        break;
                case ISC_R_SOFTQUOTA:
                        isc_quota_detach(&client->recursionquota);
@@ -2584,9 +2585,6 @@ query_prefetch(ns_client_t *client, dns_name_t *qname,
                }
        }
 
-       ns_stats_increment(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        tmprdataset = ns_client_newrdataset(client);
        if (tmprdataset == NULL) {
                return;
@@ -2793,6 +2791,8 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) {
                                          &client->recursionquota);
                switch (result) {
                case ISC_R_SUCCESS:
+                       ns_stats_increment(client->sctx->nsstats,
+                                          ns_statscounter_recursclients);
                        break;
                case ISC_R_SOFTQUOTA:
                        isc_quota_detach(&client->recursionquota);
@@ -2802,9 +2802,6 @@ query_rpzfetch(ns_client_t *client, dns_name_t *qname, dns_rdatatype_t type) {
                }
        }
 
-       ns_stats_increment(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        tmprdataset = ns_client_newrdataset(client);
        if (tmprdataset == NULL) {
                return;
@@ -6252,11 +6249,10 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
 
        if (client->recursionquota != NULL) {
                isc_quota_detach(&client->recursionquota);
+               ns_stats_decrement(client->sctx->nsstats,
+                                  ns_statscounter_recursclients);
        }
 
-       ns_stats_decrement(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        LOCK(&client->manager->reclock);
        if (ISC_LINK_LINKED(client, rlink)) {
                ISC_LIST_UNLINK(client->manager->recursing, client, rlink);
@@ -6390,6 +6386,11 @@ check_recursionquota(ns_client_t *client) {
        if (client->recursionquota == NULL) {
                result = isc_quota_attach(&client->sctx->recursionquota,
                                          &client->recursionquota);
+               if (result == ISC_R_SUCCESS || result == ISC_R_SOFTQUOTA) {
+                       ns_stats_increment(client->sctx->nsstats,
+                                          ns_statscounter_recursclients);
+               }
+
                if (result == ISC_R_SOFTQUOTA) {
                        isc_stdtime_t now;
                        isc_stdtime_get(&now);
@@ -6472,9 +6473,6 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
                return (result);
        }
 
-       ns_stats_increment(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        /*
         * Invoke the resolver.
         */
@@ -6765,11 +6763,10 @@ query_hookresume(isc_task_t *task, isc_event_t *event) {
 
        if (client->recursionquota != NULL) {
                isc_quota_detach(&client->recursionquota);
+               ns_stats_decrement(client->sctx->nsstats,
+                                  ns_statscounter_recursclients);
        }
 
-       ns_stats_decrement(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        LOCK(&client->manager->reclock);
        if (ISC_LINK_LINKED(client, rlink)) {
                ISC_LIST_UNLINK(client->manager->recursing, client, rlink);
@@ -6905,9 +6902,6 @@ ns_query_hookasync(query_ctx_t *qctx, ns_query_starthookasync_t runasync,
                goto cleanup;
        }
 
-       ns_stats_increment(client->sctx->nsstats,
-                          ns_statscounter_recursclients);
-
        saved_qctx = isc_mem_get(client->mctx, sizeof(*saved_qctx));
        qctx_save(qctx, saved_qctx);
        result = runasync(saved_qctx, client->mctx, arg, client->task,