From: Ondřej Surý <ondrej@isc.org>
Date: Fri, 1 Apr 2022 08:51:41 +0000 (+0200)
Subject: Stop the zone timer before detaching the timer
X-Git-Tag: v9.19.0~28^2~1
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=40971b22e7695966a898002efa0c0c18649568c4;p=thirdparty%2Fbind9.git

Stop the zone timer before detaching the timer

Previously, the zone timer was not stopped before detaching the timer.
This could lead to a data race where the timer post_event() could fire
before the timer was detached, but then the event would be executed
after the zone was already destroyed.

This was not noticed before because the timing or the ordering of the
actions were different, but it was causing assertion failures in the
libns tests now.

Properly stop the zone timer before detaching the timer object from the
dns_zone.
---

diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index ceb915ed5c8..2b3189cc55c 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -849,6 +849,11 @@ unsigned int dns_zone_mkey_month = MONTH;
 
 #define SEND_BUFFER_SIZE 2048
 
+static void
+zone_timer_start(dns_zone_t *zone, isc_time_t *next, isc_time_t *now);
+static void
+zone_timer_stop(dns_zone_t *zone);
+
 static void
 zone_settimer(dns_zone_t *, isc_time_t *);
 static void
@@ -15077,6 +15082,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) {
 	forward_cancel(zone);
 
 	if (zone->timer != NULL) {
+		zone_timer_stop(zone);
 		isc_timer_detach(&zone->timer);
 		isc_refcount_decrement(&zone->irefs);
 	}
@@ -15129,11 +15135,41 @@ zone_timer(isc_task_t *task, isc_event_t *event) {
 	isc_event_free(&event);
 }
 
+static void
+zone_timer_start(dns_zone_t *zone, isc_time_t *next, isc_time_t *now) {
+	isc_interval_t interval;
+	isc_result_t result;
+
+	if (isc_time_compare(next, now) <= 0) {
+		isc_interval_set(&interval, 0, 1);
+	} else {
+		isc_time_subtract(next, now, &interval);
+	}
+
+	result = isc_timer_reset(zone->timer, isc_timertype_once, &interval,
+				 true);
+	if (result != ISC_R_SUCCESS) {
+		dns_zone_log(zone, ISC_LOG_ERROR,
+			     "could not reset zone timer: %s",
+			     isc_result_totext(result));
+	}
+}
+
+static void
+zone_timer_stop(dns_zone_t *zone) {
+	isc_result_t result = isc_timer_reset(
+		zone->timer, isc_timertype_inactive, NULL, true);
+	if (result != ISC_R_SUCCESS) {
+		dns_zone_log(zone, ISC_LOG_ERROR,
+			     "could not deactivate zone timer: %s",
+			     isc_result_totext(result));
+	}
+}
+
 static void
 zone_settimer(dns_zone_t *zone, isc_time_t *now) {
 	const char me[] = "zone_settimer";
 	isc_time_t next;
-	isc_result_t result;
 
 	REQUIRE(DNS_ZONE_VALID(zone));
 	REQUIRE(LOCKED_ZONE(zone));
@@ -15272,28 +15308,9 @@ zone_settimer(dns_zone_t *zone, isc_time_t *now) {
 
 	if (isc_time_isepoch(&next)) {
 		zone_debuglog(zone, me, 10, "settimer inactive");
-		result = isc_timer_reset(zone->timer, isc_timertype_inactive,
-					 NULL, true);
-		if (result != ISC_R_SUCCESS) {
-			dns_zone_log(zone, ISC_LOG_ERROR,
-				     "could not deactivate zone timer: %s",
-				     isc_result_totext(result));
-		}
+		zone_timer_stop(zone);
 	} else {
-		isc_interval_t interval;
-		if (isc_time_compare(&next, now) <= 0) {
-			isc_interval_set(&interval, 0, 1);
-		} else {
-			isc_time_subtract(&next, now, &interval);
-		}
-
-		result = isc_timer_reset(zone->timer, isc_timertype_once,
-					 &interval, true);
-		if (result != ISC_R_SUCCESS) {
-			dns_zone_log(zone, ISC_LOG_ERROR,
-				     "could not reset zone timer: %s",
-				     isc_result_totext(result));
-		}
+		zone_timer_start(zone, &next, now);
 	}
 }