fixup! Increase also view usage counter during statistics rendering

Other places using dns_zt_apply use RWLOCK as well. Apply locking also from
other calls to apply. It seems it would be either helpful or not
necessary.

bin/named/statschannel.c

@@ -1969,9 +1969,9 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
 	xmlTextWriterPtr writer = NULL;
 	xmlDocPtr doc = NULL;
 	int xmlrc;
-	dns_view_t *view;
+	dns_view_t *view_next = NULL, *view = NULL;
 	stats_dumparg_t dumparg;
-	dns_stats_t *cacherrstats;
+	dns_stats_t *cacherrstats = NULL;
 	uint64_t nsstat_values[ns_statscounter_max];
 	uint64_t resstat_values[dns_resstatscounter_max];
 	uint64_t adbstat_values[dns_adbstats_max];
@@ -2236,13 +2236,14 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
 	 * Render views.  For each view we know of, call its
 	 * rendering function.
 	 */
-	view = ISC_LIST_HEAD(server->viewlist);
+	view_next = ISC_LIST_HEAD(server->viewlist);
 	TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "views"));
-	while (view != NULL &&
+	while (view_next != NULL &&
 	       ((flags & (STATS_XML_SERVER | STATS_XML_ZONES)) != 0)) {
 		isc_stats_t *istats = NULL;
 		dns_stats_t *dstats = NULL;
 
+		dns_view_attach(view_next, &view);
 		TRY0(xmlTextWriterStartElement(writer, ISC_XMLCHAR "view"));
 		TRY0(xmlTextWriterWriteAttribute(writer, ISC_XMLCHAR "name",
 						 ISC_XMLCHAR view->name));
@@ -2323,7 +2324,8 @@ generatexml(named_server_t *server, uint32_t flags, int *buflen,
 
 		TRY0(xmlTextWriterEndElement(writer)); /* view */
 
-		view = ISC_LIST_NEXT(view, link);
+		view_next = ISC_LIST_NEXT(view, link);
+		dns_view_detach(&view);
 	}
 	TRY0(xmlTextWriterEndElement(writer)); /* /views */
 
@@ -2355,6 +2357,8 @@ cleanup:
 	isc_log_write(named_g_lctx, NAMED_LOGCATEGORY_GENERAL,
 		      NAMED_LOGMODULE_SERVER, ISC_LOG_ERROR,
 		      "failed generating XML response");
+	if (view != NULL)
+		dns_view_detach(&view);
 	if (writer != NULL) {
 		xmlFreeTextWriter(writer);
 	}

lib/dns/zt.c

@@ -263,9 +263,7 @@ dns_zt_load(dns_zt_t *zt, bool stop, bool newonly) {
 	struct zt_load_params params;
 	REQUIRE(VALID_ZT(zt));
 	params.newonly = newonly;
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
 	result = dns_zt_apply(zt, stop, NULL, load, &params);
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
 	return (result);
 }
 
@@ -336,9 +334,7 @@ dns_zt_asyncload(dns_zt_t *zt, bool newonly, dns_zt_allloaded_t alldone,
 	zt->loaddone = alldone;
 	zt->loaddone_arg = arg;
 
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
 	result = dns_zt_apply(zt, false, NULL, asyncload, zt);
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
 
 	/*
 	 * Have all the loads completed?
@@ -384,9 +380,7 @@ dns_zt_freezezones(dns_zt_t *zt, dns_view_t *view, bool freeze) {
 
 	REQUIRE(VALID_ZT(zt));
 
-	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
 	result = dns_zt_apply(zt, false, &tresult, freezezones, &params);
-	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
 	if (tresult == ISC_R_NOTFOUND) {
 		tresult = ISC_R_SUCCESS;
 	}
@@ -524,36 +518,44 @@ dns_zt_setviewrevert(dns_zt_t *zt) {
 isc_result_t
 dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub,
 	     isc_result_t (*action)(dns_zone_t *, void *), void *uap) {
-	dns_rbtnode_t *node;
+	dns_rbtnode_t *node = NULL;
 	dns_rbtnodechain_t chain;
-	isc_result_t result, tresult = ISC_R_SUCCESS;
-	dns_zone_t *zone;
+	isc_result_t result, err = ISC_R_SUCCESS;
+	dns_zone_t *zone = NULL;
 
 	REQUIRE(VALID_ZT(zt));
 	REQUIRE(action != NULL);
 
 	dns_rbtnodechain_init(&chain);
+	RWLOCK(&zt->rwlock, isc_rwlocktype_read);
 	result = dns_rbtnodechain_first(&chain, zt->table, NULL, NULL);
+
 	if (result == ISC_R_NOTFOUND) {
 		/*
 		 * The tree is empty.
 		 */
-		tresult = result;
-		result = ISC_R_NOMORE;
+		err = result;
+		result = ISC_R_SUCCESS;
+		goto cleanup;
 	}
+
 	while (result == DNS_R_NEWORIGIN || result == ISC_R_SUCCESS) {
 		result = dns_rbtnodechain_current(&chain, NULL, NULL, &node);
 		if (result == ISC_R_SUCCESS) {
-			zone = node->data;
-			if (zone != NULL) {
+			if (node->data != NULL) {
+				dns_zone_attach(node->data, &zone);
 				result = (action)(zone, uap);
+				dns_zone_detach(&zone);
 			}
-			if (result != ISC_R_SUCCESS && stop) {
-				tresult = result;
-				goto cleanup; /* don't break */
-			} else if (result != ISC_R_SUCCESS &&
-				   tresult == ISC_R_SUCCESS) {
-				tresult = result;
+
+			if (result != ISC_R_SUCCESS) {
+				if (err == ISC_R_SUCCESS) {
+					err = result;
+				}
+
+				if (stop) {
+					goto cleanup; /* don't break */
+				}
 			}
 		}
 		result = dns_rbtnodechain_next(&chain, NULL, NULL);
@@ -563,9 +565,10 @@ dns_zt_apply(dns_zt_t *zt, bool stop, isc_result_t *sub,
 	}
 
 cleanup:
+	RWUNLOCK(&zt->rwlock, isc_rwlocktype_read);
 	dns_rbtnodechain_invalidate(&chain);
 	if (sub != NULL) {
-		*sub = tresult;
+		*sub = err;
 	}
 
 	return (result);