upstream/bind9
1
0
Fork 0
mirror of https://github.com/isc-projects/bind9.git synced 2026-06-11 10:50:00 -04:00
Code Issues Projects Releases Packages Wiki Activity Actions 2

Fix ns_statscounter_recursclients underflow

Browse source 
The basic scenario for the problem was that in the process of
resolving a query, if any rrset was eligible for prefetching, then it
would trigger a call to query_prefetch(), this call would run in
parallel to the normal query processing.

The problem arises due to the fact that both query_prefetch(), and,
in the original thread, a call to ns_query_recurse(), try to attach
to the recursionquota, but recursing client stats counter is only
incremented if ns_query_recurse() attachs to it first.

Conversely, if fetch_callback() is called before prefetch_done(),
it would not only detach from recursionquota, but also decrement
the stats counter, if query_prefetch() attached to te quota first
that would result in a decrement not matched by an increment, as
expected.

To solve this issue an atomic bool was added, it is set once in
ns_query_recurse(), allowing fetch_callback() to check for it
and decrement stats accordingly.

For a more compreensive explanation check the thread comment below:
https://gitlab.isc.org/isc-projects/bind9/-/issues/1719#note_145857
This commit is contained in:
Diego Fronza 2020-07-08 11:42:32 -03:00
parent 600128ac27
commit aab691d512
2 changed files with 24 additions and 1 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

3
lib/ns/include/ns/client.h
View file

@ -263,7 +263,8 @@ struct ns_client {
#define NS_CLIENTATTR_WANTPAD 0x08000 /*%< pad reply */
#define NS_CLIENTATTR_USEKEEPALIVE 0x10000 /*%< use TCP keepalive */
#define NS_CLIENTATTR_NOSETFC 0x20000 /*%< don't set servfail cache */
#define NS_CLIENTATTR_NOSETFC 0x20000 /*%< don't set servfail cache */
#define NS_CLIENTATTR_RECURSING 0x40000 /*%< client is recursing */
/*
* Flag to use with the SERVFAIL cache to indicate

22
lib/ns/query.c
View file

@ -5686,6 +5686,15 @@ fetch_callback(isc_task_t *task, isc_event_t *event) {
isc_quota_detach(&client->recursionquota);
ns_stats_decrement(client->sctx->nsstats,
ns_statscounter_recursclients);
} else if (client->attributes & NS_CLIENTATTR_RECURSING) {
client->attributes &= ~NS_CLIENTATTR_RECURSING;
/*
* Detached from recursionquota via prefetch_done(),
* but need to decrement recursive client stats here anyway,
* since it was incremented in ns_query_recurse().
*/
ns_stats_decrement(client->sctx->nsstats,
ns_statscounter_recursclients);
}
LOCK(&client->manager->reclock);
@ -5834,6 +5843,7 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
if (result == ISC_R_SUCCESS || result == ISC_R_SOFTQUOTA) {
ns_stats_increment(client->sctx->nsstats,
ns_statscounter_recursclients);
client->attributes |= NS_CLIENTATTR_RECURSING;
}
if (result == ISC_R_SOFTQUOTA) {
@ -5887,6 +5897,18 @@ ns_query_recurse(ns_client_t *client, dns_rdatatype_t qtype, dns_name_t *qname,
}
ns_client_recursing(client);
} else if ((client->attributes & NS_CLIENTATTR_RECURSING) == 0) {
client->attributes |= NS_CLIENTATTR_RECURSING;
/*
* query_prefetch() attached first to client->recursionquota,
* but we must check if NS_CLIENTATTR_RECURSING attribute is
* on, if not then turn it on and increment recursing clients
* stats counter only once. The attribute is also checked in
* fetch_callback() to know if a matching decrement to this
* counter should be applied.
*/
ns_stats_increment(client->sctx->nsstats,
ns_statscounter_recursclients);
}
/*