From a56f9d50619038fc3c7a8c20a59d1a50ed47d8ae Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Mon, 12 Sep 2022 14:51:33 +0200
Subject: [PATCH] Create the spillattimer when needed and destroy it early

Instead of creating dns_resolver .spillattimer when the dns_resolver_t
object is created, create it on the current loop as needed and destroy
it as soon as the timer has finished its job.  This avoids the need to
manipulate the timer from a different thread.
---
 lib/dns/resolver.c | 53 +++++++++++++++++++++++++++++-----------------
 1 file changed, 34 insertions(+), 19 deletions(-)

diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c
index a3ddb7043b..da2f2ab904 100644
--- a/lib/dns/resolver.c
+++ b/lib/dns/resolver.c
@@ -17,6 +17,7 @@
 #include <inttypes.h>
 #include <stdbool.h>
 
+#include <isc/async.h>
 #include <isc/atomic.h>
 #include <isc/counter.h>
 #include <isc/hash.h>
@@ -1738,12 +1739,14 @@ fcount_decr(fetchctx_t *fctx) {
 	UNLOCK(&zbucket->lock);
 }
 
+static void
+spillattimer_countdown(void *arg);
+
 static void
 fctx_sendevents(fetchctx_t *fctx, isc_result_t result, int line) {
 	dns_fetchevent_t *event, *next_event;
 	isc_task_t *task;
 	unsigned int count = 0;
-	isc_interval_t i;
 	bool logit = false;
 	isc_time_t now;
 	unsigned int old_spillat;
@@ -1822,9 +1825,20 @@ fctx_sendevents(fetchctx_t *fctx, isc_result_t result, int line) {
 			if (new_spillat != old_spillat) {
 				logit = true;
 			}
-			isc_interval_set(&i, 20 * 60, 0);
-			isc_timer_start(fctx->res->spillattimer,
-					isc_timertype_ticker, &i);
+
+			/* Timer not running */
+			if (fctx->res->spillattimer == NULL) {
+				isc_interval_t i;
+
+				isc_timer_create(
+					isc_loop_current(fctx->res->loopmgr),
+					spillattimer_countdown, fctx->res,
+					&fctx->res->spillattimer);
+
+				isc_interval_set(&i, 20 * 60, 0);
+				isc_timer_start(fctx->res->spillattimer,
+						isc_timertype_ticker, &i);
+			}
 		}
 		UNLOCK(&fctx->res->lock);
 		if (logit) {
@@ -10196,7 +10210,7 @@ destroy(dns_resolver_t *res) {
 		isc_mem_put(res->mctx, a, sizeof(*a));
 	}
 	dns_badcache_destroy(&res->badcache);
-	isc_timer_destroy(&res->spillattimer);
+
 	dns_view_weakdetach(&res->view);
 	isc_mem_putanddetach(&res->mctx, res, sizeof(*res));
 }
@@ -10204,26 +10218,28 @@ destroy(dns_resolver_t *res) {
 static void
 spillattimer_countdown(void *arg) {
 	dns_resolver_t *res = (dns_resolver_t *)arg;
-	unsigned int count;
-	bool logit = false;
+	unsigned int spillat = 0;
 
 	REQUIRE(VALID_RESOLVER(res));
 
+	if (atomic_load(&res->exiting)) {
+		isc_timer_destroy(&res->spillattimer);
+		return;
+	}
+
 	LOCK(&res->lock);
 	INSIST(!atomic_load_acquire(&res->exiting));
 	if (res->spillat > res->spillatmin) {
-		res->spillat--;
-		logit = true;
+		spillat = --res->spillat;
 	}
 	if (res->spillat <= res->spillatmin) {
-		isc_timer_stop(res->spillattimer);
+		isc_timer_destroy(&res->spillattimer);
 	}
-	count = res->spillat;
 	UNLOCK(&res->lock);
-	if (logit) {
+	if (spillat > 0) {
 		isc_log_write(dns_lctx, DNS_LOGCATEGORY_RESOLVER,
 			      DNS_LOGMODULE_RESOLVER, ISC_LOG_NOTICE,
-			      "clients-per-query decreased to %u", count);
+			      "clients-per-query decreased to %u", spillat);
 	}
 }
 
@@ -10236,7 +10252,6 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr,
 	isc_result_t result = ISC_R_SUCCESS;
 	char name[sizeof("res4294967295")];
 	dns_resolver_t *res = NULL;
-	isc_loop_t *loop = NULL;
 
 	/*
 	 * Create a resolver.
@@ -10324,10 +10339,6 @@ dns_resolver_create(dns_view_t *view, isc_loopmgr_t *loopmgr,
 	isc_mutex_init(&res->lock);
 	isc_mutex_init(&res->primelock);
 
-	loop = isc_loop_main(res->loopmgr);
-
-	isc_timer_create(loop, spillattimer_countdown, res, &res->spillattimer);
-
 	res->magic = RES_MAGIC;
 
 	*resp = res;
@@ -10507,7 +10518,11 @@ dns_resolver_shutdown(dns_resolver_t *res) {
 		isc_ht_iter_destroy(&it);
 		RWUNLOCK(&res->hash_lock, isc_rwlocktype_read);
 
-		isc_timer_stop(res->spillattimer);
+		LOCK(&res->lock);
+		if (res->spillattimer != NULL) {
+			isc_timer_async_destroy(&res->spillattimer);
+		}
+		UNLOCK(&res->lock);
 	}
 }