Merge branch 'aram/catz-add-lock-for-dns_catz_zone_t' into 'main'

Add a lock for dns_catz_zone_t

See merge request isc-projects/bind9!7633

lib/dns/catz.c

@@ -101,6 +101,7 @@ struct dns_catz_zone {
 	bool broken;
 
 	isc_refcount_t references;
+	isc_mutex_t lock;
 };
 
 static void
@@ -462,8 +463,17 @@ dns_catz_zone_resetdefoptions(dns_catz_zone_t *catz) {
 	dns_catz_options_init(&catz->defoptions);
 }
 
-isc_result_t
+/*%<
-dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
+ * Merge 'newcatz' into 'catz', calling addzone/delzone/modzone
+ * (from catz->catzs->zmm) for appropriate member zones.
+ *
+ * Requires:
+ * \li	'catz' is a valid dns_catz_zone_t.
+ * \li	'newcatz' is a valid dns_catz_zone_t.
+ *
+ */
+static isc_result_t
+dns__catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
 	isc_result_t result;
 	isc_ht_iter_t *iter1 = NULL, *iter2 = NULL;
 	isc_ht_iter_t *iteradd = NULL, *itermod = NULL;
@@ -476,6 +486,8 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
 	REQUIRE(DNS_CATZ_ZONE_VALID(catz));
 	REQUIRE(DNS_CATZ_ZONE_VALID(newcatz));
 
+	LOCK(&catz->lock);
+
 	/* TODO verify the new zone first! */
 
 	addzone = catz->catzs->zmm->addzone;
@@ -551,12 +563,18 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
 		if (zt_find_result == ISC_R_SUCCESS) {
 			dns_catz_coo_t *coo = NULL;
 			char pczname[DNS_NAME_FORMATSIZE];
+			bool parentcatz_locked = false;
 
 			/*
 			 * Change of ownership (coo) processing, if required
 			 */
 			parentcatz = dns_zone_get_parentcatz(zone);
-			if (parentcatz != NULL && parentcatz != catz &&
+			if (parentcatz != NULL && parentcatz != catz) {
+				UNLOCK(&catz->lock);
+				LOCK(&parentcatz->lock);
+				parentcatz_locked = true;
+			}
+			if (parentcatz_locked &&
 			    isc_ht_find(parentcatz->coos, nentry->name.ndata,
 					nentry->name.length,
 					(void **)&coo) == ISC_R_SUCCESS &&
@@ -582,6 +600,10 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
 					      zname, pczname,
 					      isc_result_totext(result));
 			}
+			if (parentcatz_locked) {
+				UNLOCK(&parentcatz->lock);
+				LOCK(&catz->lock);
+			}
 		}
 		if (zt_find_result == ISC_R_SUCCESS ||
 		    zt_find_result == DNS_R_PARTIALMATCH)
@@ -743,6 +765,8 @@ dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz) {
 	isc_ht_destroy(&toadd);
 	isc_ht_destroy(&tomod);
 
+	UNLOCK(&catz->lock);
+
 	return (result);
 }
 
@@ -794,6 +818,7 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **catzp,
 				   .version = DNS_CATZ_VERSION_UNDEFINED,
 				   .magic = DNS_CATZ_ZONE_MAGIC };
 
+	isc_mutex_init(&catz->lock);
 	isc_refcount_init(&catz->references, 1);
 	isc_ht_init(&catz->entries, catzs->mctx, 4, ISC_HT_CASE_SENSITIVE);
 	isc_ht_init(&catz->coos, catzs->mctx, 4, ISC_HT_CASE_INSENSITIVE);
@@ -978,6 +1003,7 @@ dns__catz_zone_destroy(dns_catz_zone_t *catz) {
 		isc_ht_destroy(&catz->coos);
 	}
 	catz->magic = 0;
+	isc_mutex_destroy(&catz->lock);
 
 	if (catz->updatetimer != NULL) {
 		isc_timer_async_destroy(&catz->updatetimer);
@@ -1772,8 +1798,17 @@ catz_process_value(dns_catz_zone_t *catz, dns_name_t *name,
 	}
 }
 
-isc_result_t
+/*%<
-dns_catz_update_process(dns_catz_zone_t *catz, const dns_name_t *src_name,
+ * Process a single rdataset from a catalog zone 'catz' update, src_name is the
+ * record name.
+ *
+ * Requires:
+ * \li	'catz' is a valid dns_catz_zone_t.
+ * \li	'src_name' is a valid dns_name_t.
+ * \li	'rdataset' is valid rdataset.
+ */
+static isc_result_t
+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;
@@ -2330,7 +2365,15 @@ dns__catz_update_cb(void *data) {
 				goto next;
 			}
 
-			result = dns_catz_update_process(newcatz, name,
+			/*
+			 * Although newcatz->coos is accessed in
+			 * catz_process_coo() in the call-chain below, we don't
+			 * need to hold the newcatz->lock, because the newcatz
+			 * is still local to this thread and function and
+			 * newcatz->coos can't be accessed from the outside
+			 * until dns__catz_zones_merge() has been called.
+			 */
+			result = dns__catz_update_process(newcatz, name,
 							  &rdataset);
 			if (result != ISC_R_SUCCESS) {
 				char typebuf[DNS_RDATATYPE_FORMATSIZE];
@@ -2413,7 +2456,7 @@ final:
 	/*
 	 * Finally merge new zone into old zone.
 	 */
-	result = dns_catz_zones_merge(oldcatz, newcatz);
+	result = dns__catz_zones_merge(oldcatz, newcatz);
 	dns_catz_detach_catz(&newcatz);
 	if (result != ISC_R_SUCCESS) {
 		isc_log_write(dns_lctx, DNS_LOGCATEGORY_GENERAL,
@@ -2520,7 +2563,7 @@ dns_catz_postreconfig(dns_catz_zones_t *catzs) {
 			result = dns_catz_new_zone(catzs, &newcatz,
 						   &catz->name);
 			INSIST(result == ISC_R_SUCCESS);
-			dns_catz_zones_merge(catz, newcatz);
+			dns__catz_zones_merge(catz, newcatz);
 			dns_catz_detach_catz(&newcatz);
 
 			/* Make sure that we have an empty catalog zone. */

lib/dns/include/dns/catz.h

@@ -249,31 +249,6 @@ dns_catz_zone_resetdefoptions(dns_catz_zone_t *catz);
  * \li	'catz' is a valid dns_catz_zone_t.
  */
 
-isc_result_t
-dns_catz_zones_merge(dns_catz_zone_t *catz, dns_catz_zone_t *newcatz);
-/*%<
- * Merge 'newcatz' into 'catz', calling addzone/delzone/modzone
- * (from catz->catzs->zmm) for appropriate member zones.
- *
- * Requires:
- * \li	'catz' is a valid dns_catz_zone_t.
- * \li	'newcatz' is a valid dns_catz_zone_t.
- *
- */
-
-isc_result_t
-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 'catz' update, src_name is the
- * record name.
- *
- * Requires:
- * \li	'catz' is a valid dns_catz_zone_t.
- * \li	'src_name' is a valid dns_name_t.
- * \li	'rdataset' is valid rdataset.
- */
-
 isc_result_t
 dns_catz_generate_masterfilename(dns_catz_zone_t *catz, dns_catz_entry_t *entry,
 				 isc_buffer_t **buffer);