From 0de313fff70003c8cfe422293625d02ec5de5d47 Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Mon, 2 Dec 2019 16:58:57 +1100 Subject: [PATCH 1/2] lock access to fctx->nqueries (cherry picked from commit 5589748ecac0b016c2221ba74249fbfbddf412b6) --- lib/dns/resolver.c | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 7890407f663..6f672a13a85 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -1136,9 +1136,8 @@ resquery_destroy(resquery_t **queryp) { res = fctx->res; bucket = fctx->bucketnum; - fctx->nqueries--; - LOCK(&res->buckets[bucket].lock); + fctx->nqueries--; empty = fctx_decreference(query->fctx); UNLOCK(&res->buckets[bucket].lock); @@ -1937,6 +1936,7 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, bool have_addr = false; unsigned int srtt; isc_dscp_t dscp = -1; + unsigned int bucketnum; FCTXTRACE("query"); @@ -2162,7 +2162,10 @@ fctx_query(fetchctx_t *fctx, dns_adbaddrinfo_t *addrinfo, fctx->querysent++; ISC_LIST_APPEND(fctx->queries, query, link); - query->fctx->nqueries++; + bucketnum = fctx->bucketnum; + LOCK(&res->buckets[bucketnum].lock); + fctx->nqueries++; + UNLOCK(&res->buckets[bucketnum].lock); if (isc_sockaddr_pf(&addrinfo->sockaddr) == PF_INET) inc_stats(res, dns_resstatscounter_queryv4); else -- GitLab From e40c1582d63f07e4f23a3c82627cf1758e0f669e Mon Sep 17 00:00:00 2001 From: Mark Andrews Date: Thu, 5 Dec 2019 15:25:21 +1100 Subject: [PATCH 2/2] Note bucket lock requirements and move REQUIRE inside locked section. (cherry picked from commit 13aaeaa06f79e520a0b18a431dfe449d2edf4476) --- lib/dns/resolver.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 6f672a13a85..0bcfc227eb0 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -319,7 +319,7 @@ struct fetchctx { /*% * The number of events we're waiting for. */ - unsigned int pending; + unsigned int pending; /* Bucket lock. */ /*% * The number of times we've "restarted" the current @@ -348,7 +348,7 @@ struct fetchctx { /*% * Number of queries that reference this context. */ - unsigned int nqueries; + unsigned int nqueries; /* Bucket lock. */ /*% * The reason to print when logging a successful @@ -393,7 +393,7 @@ struct fetchctx { #define FCTX_ATTR_HAVEANSWER 0x0001 #define FCTX_ATTR_GLUING 0x0002 #define FCTX_ATTR_ADDRWAIT 0x0004 -#define FCTX_ATTR_SHUTTINGDOWN 0x0008 +#define FCTX_ATTR_SHUTTINGDOWN 0x0008 /* Bucket lock */ #define FCTX_ATTR_WANTCACHE 0x0010 #define FCTX_ATTR_WANTNCACHE 0x0020 #define FCTX_ATTR_NEEDEDNS0 0x0040 @@ -5353,11 +5353,12 @@ maybe_destroy(fetchctx_t *fctx, bool locked) { dns_validator_t *validator, *next_validator; bool dodestroy = false; - REQUIRE(SHUTTINGDOWN(fctx)); - bucketnum = fctx->bucketnum; if (!locked) LOCK(&res->buckets[bucketnum].lock); + + REQUIRE(SHUTTINGDOWN(fctx)); + if (fctx->pending != 0 || fctx->nqueries != 0) goto unlock; @@ -7064,6 +7065,9 @@ fctx_increference(fetchctx_t *fctx) { UNLOCK(&fctx->res->buckets[fctx->bucketnum].lock); } +/* + * Requires bucket lock to be held. + */ static bool fctx_decreference(fetchctx_t *fctx) { bool bucket_empty = false; -- GitLab