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 ceb915ed5c838a0d50f6dee5c5ac89a9e89bfdf0..2b3189cc55c92c304729a291c51accb5d64ed032 100644 (file)
--- 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);
        }
 }