From 246b7084d6ecb692643da464e734d719495ae63f Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 21 Feb 2023 21:14:44 +0000 Subject: [PATCH 1/2] Add shutdown signaling for catalog zones This change should make sure that catalog zone update processing doesn't happen when the catalog zone is being shut down. This should help avoid races when offloading the catalog zone updates in the follow-up commit. --- lib/dns/catz.c | 75 +++++++++++++++++++++++++++++++++----- lib/dns/include/dns/catz.h | 10 +++++ lib/dns/view.c | 2 + 3 files changed, 77 insertions(+), 10 deletions(-) diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 39c1b55c80..ef1beb7b39 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -69,6 +69,7 @@ struct dns_catz_entry { */ struct dns_catz_zone { unsigned int magic; + isc_loop_t *loop; dns_name_t name; dns_catz_zones_t *catzs; dns_rdata_t soa; @@ -101,6 +102,8 @@ struct dns_catz_zone { static void dns__catz_timer_start(dns_catz_zone_t *catz); +static void +dns__catz_timer_stop(void *arg); static isc_result_t catz_process_zones_entry(dns_catz_zone_t *catz, dns_rdataset_t *value, @@ -126,6 +129,7 @@ struct dns_catz_zones { dns_catz_zonemodmethods_t *zmm; isc_loopmgr_t *loopmgr; dns_view_t *view; + atomic_bool shuttingdown; }; void @@ -820,11 +824,26 @@ dns__catz_timer_start(dns_catz_zone_t *catz) { isc_interval_set(&interval, 0, 0); } - isc_timer_create(isc_loop_current(catz->catzs->loopmgr), - dns_catz_update_action, catz, &catz->updatetimer); + catz->loop = isc_loop_current(catz->catzs->loopmgr); + + isc_timer_create(catz->loop, dns_catz_update_action, catz, + &catz->updatetimer); isc_timer_start(catz->updatetimer, isc_timertype_once, &interval); } +static void +dns__catz_timer_stop(void *arg) { + dns_catz_zone_t *catz = arg; + REQUIRE(DNS_CATZ_ZONE_VALID(catz)); + + isc_timer_stop(catz->updatetimer); + isc_timer_destroy(&catz->updatetimer); + catz->loop = NULL; + + dns_catz_unref_catzs(catz->catzs); + dns_catz_detach_catz(&catz); +} + isc_result_t dns_catz_add_zone(dns_catz_zones_t *catzs, const dns_name_t *name, dns_catz_zone_t **catzp) { @@ -888,6 +907,20 @@ dns_catz_get_zone(dns_catz_zones_t *catzs, const dns_name_t *name) { return (found); } +static void +dns__catz_shutdown(dns_catz_zone_t *catz) { + /* lock must be locked */ + if (catz->updatetimer != NULL) { + /* Don't wait for timer to trigger for shutdown */ + INSIST(catz->loop != NULL); + + dns_catz_ref_catzs(catz->catzs); + isc_async_run(catz->loop, dns__catz_timer_stop, catz); + } else { + dns_catz_detach_catz(&catz); + } +} + static void dns__catz_zone_destroy(dns_catz_zone_t *catz) { isc_mem_t *mctx = catz->catzs->mctx; @@ -959,27 +992,44 @@ dns__catz_zone_destroy(dns_catz_zone_t *catz) { static void dns__catz_zones_destroy(dns_catz_zones_t *catzs) { + REQUIRE(atomic_load(&catzs->shuttingdown)); + REQUIRE(catzs->zones == NULL); + + catzs->magic = 0; + isc_mutex_destroy(&catzs->lock); + isc_refcount_destroy(&catzs->references); + + isc_mem_putanddetach(&catzs->mctx, catzs, sizeof(*catzs)); +} + +void +dns_catz_shutdown_catzs(dns_catz_zones_t *catzs) { + REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); + + if (!atomic_compare_exchange_strong(&catzs->shuttingdown, + &(bool){ false }, true)) + { + return; + } + + LOCK(&catzs->lock); if (catzs->zones != NULL) { isc_ht_iter_t *iter = NULL; isc_result_t result; isc_ht_iter_create(catzs->zones, &iter); for (result = isc_ht_iter_first(iter); result == ISC_R_SUCCESS;) { - dns_catz_zone_t *zone = NULL; - isc_ht_iter_current(iter, (void **)&zone); + dns_catz_zone_t *catz = NULL; + isc_ht_iter_current(iter, (void **)&catz); result = isc_ht_iter_delcurrent_next(iter); - dns_catz_detach_catz(&zone); + dns__catz_shutdown(catz); } INSIST(result == ISC_R_NOMORE); isc_ht_iter_destroy(&iter); INSIST(isc_ht_count(catzs->zones) == 0); isc_ht_destroy(&catzs->zones); } - catzs->magic = 0; - isc_mutex_destroy(&catzs->lock); - isc_refcount_destroy(&catzs->references); - - isc_mem_putanddetach(&catzs->mctx, catzs, sizeof(*catzs)); + UNLOCK(&catzs->lock); } #ifdef DNS_CATZ_TRACE @@ -1991,6 +2041,7 @@ dns_catz_update_action(void *arg) { dns_catz_update_from_db(catz->db, catz->catzs); isc_timer_stop(catz->updatetimer); isc_timer_destroy(&catz->updatetimer); + catz->loop = NULL; result = isc_time_now(&catz->lastupdated); RUNTIME_CHECK(result == ISC_R_SUCCESS); UNLOCK(&catz->catzs->lock); @@ -2083,6 +2134,10 @@ dns_catz_update_from_db(dns_db_t *db, dns_catz_zones_t *catzs) { REQUIRE(DNS_DB_VALID(db)); REQUIRE(DNS_CATZ_ZONES_VALID(catzs)); + if (atomic_load(&catzs->shuttingdown)) { + return; + } + dns_name_format(&db->origin, bname, DNS_NAME_FORMATSIZE); /* diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h index 97ee94d56f..67f96e2d21 100644 --- a/lib/dns/include/dns/catz.h +++ b/lib/dns/include/dns/catz.h @@ -438,6 +438,16 @@ dns_catz_get_iterator(dns_catz_zone_t *catz, isc_ht_iter_t **itp); * */ +void +dns_catz_shutdown_catzs(dns_catz_zones_t *catzs); +/*%< + * Shut down the catalog zones. + * + * Requires: + * \li 'catzs' is a valid dns_catz_zones_t. + * + */ + #ifdef DNS_CATZ_TRACE /* Compatibility macros */ #define dns_catz_attach_catz(catz, catzp) \ diff --git a/lib/dns/view.c b/lib/dns/view.c index 0d3580adc6..0cf2a4802e 100644 --- a/lib/dns/view.c +++ b/lib/dns/view.c @@ -300,6 +300,7 @@ destroy(dns_view_t *view) { dns_rpz_detach_rpzs(&view->rpzs); } if (view->catzs != NULL) { + dns_catz_shutdown_catzs(view->catzs); dns_catz_detach_catzs(&view->catzs); } for (dlzdb = ISC_LIST_HEAD(view->dlz_searched); dlzdb != NULL; @@ -548,6 +549,7 @@ dns_view_detach(dns_view_t **viewp) { } } if (view->catzs != NULL) { + dns_catz_shutdown_catzs(view->catzs); dns_catz_detach_catzs(&view->catzs); } if (view->ntatable_priv != NULL) { From c76cc5880380e4570c7c535f350b9459d8c50689 Mon Sep 17 00:00:00 2001 From: Aram Sargsyan Date: Tue, 21 Feb 2023 21:20:56 +0000 Subject: [PATCH 2/2] Add a CHANGES note for [GL !7571] --- CHANGES | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES b/CHANGES index 9c0cf50397..54a2784082 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,5 @@ +6113. [func] Add shutdown signaling for catalog zones. [GL !7571] + 6112. [func] Add reference count tracing for dns_catz_zone_t and dns_catz_zones_t. [GL !7570]