From d13e7472eab2921b61a345320fe4c9a5797d1931 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= <ondrej@isc.org>
Date: Mon, 27 Feb 2023 23:00:23 +0000
Subject: [PATCH] Unlock catzs during dns__catz_update_cb()

Instead of holding the catzs->lock the whole time we process the catz
update, only hold it for hash table lookup and then release it.  This
should unblock any other threads that might be processing updates to
catzs triggered by extra incoming transfer.

(cherry picked from commit b1cd4a066a63f221a56d9565da4907c2aad7e524)
---
 lib/dns/catz.c             | 33 ++++++++++++++++++---------------
 lib/dns/include/dns/catz.h |  7 +++----
 2 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index c85a67682b..51cd251292 100644
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -1751,8 +1751,8 @@ catz_process_value(dns_catz_zone_t *catz, dns_name_t *name,
 }
 
 isc_result_t
-dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *catz,
-			const dns_name_t *src_name, dns_rdataset_t *rdataset) {
+dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
+			dns_rdataset_t *rdataset) {
 	isc_result_t result;
 	int order;
 	unsigned int nlabels;
@@ -1761,7 +1761,6 @@ dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *catz,
 	dns_rdata_soa_t soa;
 	dns_name_t prefix;
 
-	REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
 	REQUIRE(DNS_CATZ_ZONE_VALID(catz));
 	REQUIRE(ISC_MAGIC_VALID(src_name, DNS_NAME_MAGIC));
 
@@ -2040,12 +2039,12 @@ dns__catz_timer_cb(isc_task_t *task, isc_event_t *event) {
 	REQUIRE(isc_nm_tid() >= 0);
 	REQUIRE(DNS_CATZ_ZONE_VALID(catz));
 
-	LOCK(&catz->catzs->lock);
-
-	if (catz->catzs->shuttingdown) {
-		goto unlock;
+	if (atomic_load(&catz->catzs->shuttingdown)) {
+		return;
 	}
 
+	LOCK(&catz->catzs->lock);
+
 	INSIST(DNS_DB_VALID(catz->db));
 	INSIST(catz->dbversion != NULL);
 
@@ -2063,7 +2062,7 @@ dns__catz_timer_cb(isc_task_t *task, isc_event_t *event) {
 
 	result = isc_time_now(&catz->lastupdated);
 	RUNTIME_CHECK(result == ISC_R_SUCCESS);
-unlock:
+
 	UNLOCK(&catz->catzs->lock);
 }
 
@@ -2199,8 +2198,6 @@ dns__catz_update_cb(void *data) {
 	db = catz->db;
 	catzs = catz->catzs;
 
-	LOCK(&catzs->lock);
-
 	if (atomic_load(&catzs->shuttingdown)) {
 		result = ISC_R_SHUTTINGDOWN;
 		goto exit;
@@ -2212,7 +2209,9 @@ dns__catz_update_cb(void *data) {
 	 * Create a new catz in the same context as current catz.
 	 */
 	dns_name_toregion(&db->origin, &r);
+	LOCK(&catzs->lock);
 	result = isc_ht_find(catzs->zones, r.base, r.length, (void **)&oldcatz);
+	UNLOCK(&catzs->lock);
 	if (result != ISC_R_SUCCESS) {
 		/* This can happen if we remove the zone in the meantime. */
 		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@@ -2293,6 +2292,11 @@ dns__catz_update_cb(void *data) {
 	 * Iterate over database to fill the new zone.
 	 */
 	while (result == ISC_R_SUCCESS) {
+		if (atomic_load(&catzs->shuttingdown)) {
+			result = ISC_R_SHUTTINGDOWN;
+			break;
+		}
+
 		result = dns_dbiterator_current(it, &node, name);
 		if (result != ISC_R_SUCCESS) {
 			isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@@ -2338,7 +2342,7 @@ dns__catz_update_cb(void *data) {
 				goto next;
 			}
 
-			result = dns_catz_update_process(catzs, newcatz, name,
+			result = dns_catz_update_process(newcatz, name,
 							 &rdataset);
 			if (result != ISC_R_SUCCESS) {
 				char typebuf[DNS_RDATATYPE_FORMATSIZE];
@@ -2380,7 +2384,8 @@ dns__catz_update_cb(void *data) {
 	dns_db_closeversion(db, &oldcatz->dbversion, false);
 	isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL, DNS_LOGMODULE_MASTER,
 		      ISC_LOG_DEBUG(3),
-		      "catz: update_from_db: iteration finished");
+		      "catz: update_from_db: iteration finished: %s",
+		      isc_result_totext(result));
 
 	/*
 	 * Check catalog zone version compatibilites.
@@ -2451,8 +2456,6 @@ final:
 
 exit:
 	catz->updateresult = result;
-
-	UNLOCK(&catzs->lock);
 }
 
 static void
@@ -2472,7 +2475,7 @@ dns__catz_done_cb(void *data, isc_result_t result) {
 	dns_name_format(&catz->name, dname, DNS_NAME_FORMATSIZE);
 
 	/* If there's no update pending, or if shutting down, finish. */
-	if (!catz->updatepending || catz->catzs->shuttingdown) {
+	if (!catz->updatepending || atomic_load(&catz->catzs->shuttingdown)) {
 		goto done;
 	}
 
diff --git a/lib/dns/include/dns/catz.h b/lib/dns/include/dns/catz.h
index 4179d3f087..dcec6c627b 100644
--- a/lib/dns/include/dns/catz.h
+++ b/lib/dns/include/dns/catz.h
@@ -262,15 +262,14 @@ dns_catz_zones_merge(dns_catz_zone_t *target, dns_catz_zone_t *newzone);
  */
 
 isc_result_t
-dns_catz_update_process(dns_catz_zones_t *catzs, dns_catz_zone_t *zone,
-			const dns_name_t *src_name, dns_rdataset_t *rdataset);
+dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
+			dns_rdataset_t *rdataset);
 /*%<
  * Process a single rdataset from a catalog zone 'zone' update, src_name is the
  * record name.
  *
  * Requires:
- * \li	'catzs' is a valid dns_catz_zones_t.
- * \li	'zone' is a valid dns_catz_zone_t.
+ * \li	'catz' is a valid dns_catz_zone_t.
  * \li	'src_name' is a valid dns_name_t.
  * \li	'rdataset' is valid rdataset.
  */