From: Ondřej Surý <ondrej@sury.org>
Date: Tue, 28 Aug 2018 08:18:59 +0000 (+0200)
Subject: Refactor *_destroy and *_detach functions to unified order of actions.
X-Git-Tag: v9.13.3~28^2~2
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0f24c55d38efe99fe72980f6e443ba60c2b4edc6;p=thirdparty%2Fbind9.git

Refactor *_destroy and *_detach functions to unified order of actions.

This properly orders clearing the freed pointer and calling isc_refcount_destroy
as early as possible to have ability to put proper memory barrier when cleaning
up reference counting.
---

diff --git a/bin/tests/system/dyndb/driver/db.c b/bin/tests/system/dyndb/driver/db.c
index 8df5154d650..fd0f998b74a 100644
--- a/bin/tests/system/dyndb/driver/db.c
+++ b/bin/tests/system/dyndb/driver/db.c
@@ -87,14 +87,13 @@ free_sampledb(sampledb_t *sampledb) {
 
 static void
 detach(dns_db_t **dbp) {
+	REQUIRE(dbp != NULL && VALID_SAMPLEDB((sampledb_t *)(*dbp)));
 	sampledb_t *sampledb = (sampledb_t *)(*dbp);
-
-	REQUIRE(VALID_SAMPLEDB(sampledb));
+	*dbp = NULL;
 
 	if (isc_refcount_decrement(&sampledb->refs) == 1) {
 		free_sampledb(sampledb);
 	}
-	*dbp = NULL;
 }
 
 /*
diff --git a/lib/dns/acl.c b/lib/dns/acl.c
index 2502a14f3f0..69d3b524bdc 100644
--- a/lib/dns/acl.c
+++ b/lib/dns/acl.c
@@ -482,15 +482,13 @@ destroy(dns_acl_t *dacl) {
 
 void
 dns_acl_detach(dns_acl_t **aclp) {
+	REQUIRE(aclp != NULL && DNS_ACL_VALID(*aclp));
 	dns_acl_t *acl = *aclp;
-
-	REQUIRE(DNS_ACL_VALID(acl));
+	*aclp = NULL;
 
 	if (isc_refcount_decrement(&acl->refcount) == 1) {
 		destroy(acl);
 	}
-
-	*aclp = NULL;
 }
 
 
diff --git a/lib/dns/catz.c b/lib/dns/catz.c
index 82c6a70646c..cabaec822eb 100644
--- a/lib/dns/catz.c
+++ b/lib/dns/catz.c
@@ -239,24 +239,18 @@ dns_catz_entry_attach(dns_catz_entry_t *entry, dns_catz_entry_t **entryp) {
 
 void
 dns_catz_entry_detach(dns_catz_zone_t *zone, dns_catz_entry_t **entryp) {
-	dns_catz_entry_t *entry;
-	isc_mem_t *mctx;
-
 	REQUIRE(entryp != NULL && *entryp != NULL);
-
-	entry = *entryp;
-
-	mctx = zone->catzs->mctx;
+	dns_catz_entry_t *entry = *entryp;
+	*entryp = NULL;
 
 	if (isc_refcount_decrement(&entry->refs) == 1) {
+		isc_refcount_destroy(&entry->refs);
+		isc_mem_t *mctx = zone->catzs->mctx;
 		dns_catz_options_free(&entry->opts, mctx);
 		if (dns_name_dynamic(&entry->name))
 			dns_name_free(&entry->name, mctx);
-		isc_refcount_destroy(&entry->refs);
 		isc_mem_put(mctx, entry, sizeof(dns_catz_entry_t));
 	}
-
-	*entryp = NULL;
 }
 
 bool
@@ -741,18 +735,15 @@ dns_catz_zone_attach(dns_catz_zone_t *zone, dns_catz_zone_t **zonep) {
 
 void
 dns_catz_zone_detach(dns_catz_zone_t **zonep) {
-	isc_result_t result;
-	dns_catz_zone_t *zone;
-	isc_ht_iter_t *iter = NULL;
-	isc_mem_t *mctx;
-
 	REQUIRE(zonep != NULL && *zonep != NULL);
-
-	zone = *zonep;
+	dns_catz_zone_t *zone = *zonep;
+	*zonep = NULL;
 
 	if (isc_refcount_decrement(&zone->refs) == 1) {
 		isc_refcount_destroy(&zone->refs);
 		if (zone->entries != NULL) {
+			isc_ht_iter_t *iter = NULL;
+			isc_result_t result;
 			result = isc_ht_iter_create(zone->entries, &iter);
 			INSIST(result == ISC_R_SUCCESS);
 			for (result = isc_ht_iter_first(iter);
@@ -771,13 +762,14 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 			INSIST(isc_ht_count(zone->entries) == 0);
 			isc_ht_destroy(&zone->entries);
 		}
-		mctx = zone->catzs->mctx;
+		isc_mem_t *mctx = zone->catzs->mctx;
 		isc_timer_detach(&zone->updatetimer);
 		if (zone->db_registered == true) {
-			result = dns_db_updatenotify_unregister(zone->db,
-						    dns_catz_dbupdate_callback,
-						    zone->catzs);
-			INSIST(result == ISC_R_SUCCESS);
+			INSIST(dns_db_updatenotify_unregister(
+				       zone->db,
+				       dns_catz_dbupdate_callback,
+				       zone->catzs)
+			       == ISC_R_SUCCESS);
 		}
 		if (zone->dbversion)
 			dns_db_closeversion(zone->db, &zone->dbversion,
@@ -792,32 +784,28 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) {
 		zone->catzs = NULL;
 		isc_mem_put(mctx, zone, sizeof(dns_catz_zone_t));
 	}
-
-	*zonep = NULL;
 }
 
 void
-dns_catz_catzs_detach(dns_catz_zones_t ** catzsp) {
-	dns_catz_zones_t *catzs;
-	isc_ht_iter_t *iter = NULL;
-	isc_result_t result;
-	dns_catz_zone_t *zone;
-
+dns_catz_catzs_detach(dns_catz_zones_t **catzsp) {
+	REQUIRE(catzsp != NULL && *catzsp != NULL);
+	dns_catz_zones_t *catzs = *catzsp;
 
-	REQUIRE(catzsp != NULL);
 	catzs = *catzsp;
-	REQUIRE(catzs != NULL);
-
 	*catzsp = NULL;
+
 	if (isc_refcount_decrement(&catzs->refs) == 1) {
 		isc_refcount_destroy(&catzs->refs);
 		DESTROYLOCK(&catzs->lock);
 		if (catzs->zones != NULL) {
+			isc_ht_iter_t *iter = NULL;
+			isc_result_t result;
 			result = isc_ht_iter_create(catzs->zones, &iter);
 			INSIST(result == ISC_R_SUCCESS);
 			for (result = isc_ht_iter_first(iter);
 			     result == ISC_R_SUCCESS;)
 			{
+				dns_catz_zone_t *zone;
 				isc_ht_iter_current(iter, (void **) &zone);
 				result = isc_ht_iter_delcurrent_next(iter);
 				dns_catz_zone_detach(&zone);
diff --git a/lib/dns/dnstap.c b/lib/dns/dnstap.c
index e50ad134354..077445d005f 100644
--- a/lib/dns/dnstap.c
+++ b/lib/dns/dnstap.c
@@ -579,17 +579,14 @@ destroy(dns_dtenv_t *env) {
 
 void
 dns_dt_detach(dns_dtenv_t **envp) {
-	dns_dtenv_t *env;
-
 	REQUIRE(envp != NULL && VALID_DTENV(*envp));
-
-	env = *envp;
+	dns_dtenv_t *env = *envp;
+	*envp = NULL;
 
 	if (isc_refcount_decrement(&env->refcount) == 1) {
+		isc_refcount_destroy(&env->refcount);
 		destroy(env);
 	}
-
-	*envp = NULL;
 }
 
 static isc_result_t
diff --git a/lib/dns/dst_api.c b/lib/dns/dst_api.c
index 2407c49504f..7633f1c54f9 100644
--- a/lib/dns/dst_api.c
+++ b/lib/dns/dst_api.c
@@ -1130,17 +1130,14 @@ dst_key_attach(dst_key_t *source, dst_key_t **target) {
 
 void
 dst_key_free(dst_key_t **keyp) {
-	isc_mem_t *mctx;
-	dst_key_t *key;
-
 	REQUIRE(dst_initialized == true);
 	REQUIRE(keyp != NULL && VALID_KEY(*keyp));
-
-	key = *keyp;
-	mctx = key->mctx;
+	dst_key_t *key = *keyp;
+	*keyp = NULL;
 
 	if (isc_refcount_decrement(&key->refs) == 1) {
 		isc_refcount_destroy(&key->refs);
+		isc_mem_t *mctx = key->mctx;
 		if (key->keydata.generic != NULL) {
 			INSIST(key->func->destroy != NULL);
 			key->func->destroy(key);
@@ -1157,7 +1154,6 @@ dst_key_free(dst_key_t **keyp) {
 		isc_safe_memwipe(key, sizeof(*key));
 		isc_mem_putanddetach(&mctx, key, sizeof(*key));
 	}
-	*keyp = NULL;
 }
 
 bool
diff --git a/lib/dns/iptable.c b/lib/dns/iptable.c
index daaf3f85a35..36bad9bf587 100644
--- a/lib/dns/iptable.c
+++ b/lib/dns/iptable.c
@@ -148,14 +148,14 @@ dns_iptable_attach(dns_iptable_t *source, dns_iptable_t **target) {
 
 void
 dns_iptable_detach(dns_iptable_t **tabp) {
+	REQUIRE(tabp != NULL && DNS_IPTABLE_VALID(*tabp));
 	dns_iptable_t *tab = *tabp;
-	REQUIRE(DNS_IPTABLE_VALID(tab));
+	*tabp = NULL;
 
 	if (isc_refcount_decrement(&tab->refcount) == 1) {
+		isc_refcount_destroy(&tab->refcount);
 		destroy_iptable(tab);
 	}
-
-	*tabp = NULL;
 }
 
 static void
@@ -168,7 +168,6 @@ destroy_iptable(dns_iptable_t *dtab) {
 		dtab->radix = NULL;
 	}
 
-	isc_refcount_destroy(&dtab->refcount);
 	dtab->magic = 0;
 	isc_mem_putanddetach(&dtab->mctx, dtab, sizeof(*dtab));
 }
diff --git a/lib/dns/keytable.c b/lib/dns/keytable.c
index dfe7d35c2b7..9f61f3dc909 100644
--- a/lib/dns/keytable.c
+++ b/lib/dns/keytable.c
@@ -125,27 +125,19 @@ dns_keytable_attach(dns_keytable_t *source, dns_keytable_t **targetp) {
 
 void
 dns_keytable_detach(dns_keytable_t **keytablep) {
-	dns_keytable_t *keytable;
-
-	/*
-	 * Detach *keytablep from its keytable.
-	 */
-
 	REQUIRE(keytablep != NULL && VALID_KEYTABLE(*keytablep));
-
-	keytable = *keytablep;
+	dns_keytable_t *keytable = *keytablep;
+	*keytablep = NULL;
 
 	if (isc_refcount_decrement(&keytable->references) == 1) {
-		INSIST(isc_refcount_current(&keytable->active_nodes) == 0);
-		isc_refcount_destroy(&keytable->active_nodes);
 		isc_refcount_destroy(&keytable->references);
+		isc_refcount_destroy(&keytable->active_nodes);
 		dns_rbt_destroy(&keytable->table);
 		isc_rwlock_destroy(&keytable->rwlock);
 		keytable->magic = 0;
 		isc_mem_putanddetach(&keytable->mctx,
 				     keytable, sizeof(*keytable));
 	}
-	*keytablep = NULL;
 }
 
 /*%
@@ -587,7 +579,7 @@ dns_keytable_detachkeynode(dns_keytable_t *keytable, dns_keynode_t **keynodep)
 	REQUIRE(VALID_KEYTABLE(keytable));
 	REQUIRE(keynodep != NULL && VALID_KEYNODE(*keynodep));
 
-	(void)isc_refcount_decrement(&keytable->active_nodes);
+	INSIST(isc_refcount_decrement(&keytable->active_nodes) > 0);
 	dns_keynode_detach(keytable->mctx, keynodep);
 }
 
@@ -745,7 +737,7 @@ dns_keytable_forall(dns_keytable_t *keytable,
 			break;
 		}
 	}
-	(void)isc_refcount_decrement(&keytable->active_nodes);
+	INSIST(isc_refcount_decrement(&keytable->active_nodes) > 0);
 
    cleanup:
 	dns_rbtnodechain_invalidate(&chain);
@@ -812,15 +804,17 @@ dns_keynode_attach(dns_keynode_t *source, dns_keynode_t **target) {
 
 void
 dns_keynode_detach(isc_mem_t *mctx, dns_keynode_t **keynode) {
+	REQUIRE(keynode != NULL && VALID_KEYNODE(*keynode));
 	dns_keynode_t *node = *keynode;
-	REQUIRE(VALID_KEYNODE(node));
+	*keynode = NULL;
+
 	if (isc_refcount_decrement(&node->refcount) == 1) {
-		if (node->key != NULL)
-			dst_key_free(&node->key);
 		isc_refcount_destroy(&node->refcount);
+		if (node->key != NULL) {
+			dst_key_free(&node->key);
+		}
 		isc_mem_put(mctx, node, sizeof(dns_keynode_t));
 	}
-	*keynode = NULL;
 }
 
 void
diff --git a/lib/dns/nta.c b/lib/dns/nta.c
index 8f32ae62299..194688f8652 100644
--- a/lib/dns/nta.c
+++ b/lib/dns/nta.c
@@ -66,30 +66,31 @@ nta_ref(dns_nta_t *nta) {
 
 static void
 nta_detach(isc_mem_t *mctx, dns_nta_t **ntap) {
+	REQUIRE(ntap != NULL && VALID_NTA(*ntap));
 	dns_nta_t *nta = *ntap;
-
-	REQUIRE(VALID_NTA(nta));
+	*ntap = NULL;
 
 	if (isc_refcount_decrement(&nta->refcount) == 1) {
+		isc_refcount_destroy(&nta->refcount);
 		nta->magic = 0;
 		if (nta->timer != NULL) {
-			(void) isc_timer_reset(nta->timer,
-					       isc_timertype_inactive,
-					       NULL, NULL, true);
+			(void)isc_timer_reset(nta->timer,
+					      isc_timertype_inactive,
+					      NULL, NULL, true);
 			isc_timer_detach(&nta->timer);
 		}
-		isc_refcount_destroy(&nta->refcount);
-		if (dns_rdataset_isassociated(&nta->rdataset))
+		if (dns_rdataset_isassociated(&nta->rdataset)) {
 			dns_rdataset_disassociate(&nta->rdataset);
-		if (dns_rdataset_isassociated(&nta->sigrdataset))
+		}
+		if (dns_rdataset_isassociated(&nta->sigrdataset)) {
 			dns_rdataset_disassociate(&nta->sigrdataset);
+		}
 		if (nta->fetch != NULL) {
 			dns_resolver_cancelfetch(nta->fetch);
 			dns_resolver_destroyfetch(&nta->fetch);
 		}
 		isc_mem_put(mctx, nta, sizeof(dns_nta_t));
 	}
-	*ntap = NULL;
 }
 
 static void
diff --git a/lib/dns/order.c b/lib/dns/order.c
index 26563af8b3d..f8918f9dc26 100644
--- a/lib/dns/order.c
+++ b/lib/dns/order.c
@@ -134,22 +134,19 @@ dns_order_attach(dns_order_t *source, dns_order_t **target) {
 
 void
 dns_order_detach(dns_order_t **orderp) {
+	REQUIRE(orderp != NULL && DNS_ORDER_VALID(*orderp));
 	dns_order_t *order;
-	dns_order_ent_t *ent;
-
-	REQUIRE(orderp != NULL);
 	order = *orderp;
-	REQUIRE(DNS_ORDER_VALID(order));
+	*orderp = NULL;
 
 	if (isc_refcount_decrement(&order->references) == 1) {
+		isc_refcount_destroy(&order->references);
 		order->magic = 0;
+		dns_order_ent_t *ent;
 		while ((ent = ISC_LIST_HEAD(order->ents)) != NULL) {
 			ISC_LIST_UNLINK(order->ents, ent, link);
 			isc_mem_put(order->mctx, ent, sizeof(*ent));
 		}
-		isc_refcount_destroy(&order->references);
 		isc_mem_putanddetach(&order->mctx, order, sizeof(*order));
 	}
-
-	*orderp = NULL;
 }
diff --git a/lib/dns/portlist.c b/lib/dns/portlist.c
index 92315b7a3e9..9fb983f6c5a 100644
--- a/lib/dns/portlist.c
+++ b/lib/dns/portlist.c
@@ -234,11 +234,9 @@ dns_portlist_attach(dns_portlist_t *portlist, dns_portlist_t **portlistp) {
 
 void
 dns_portlist_detach(dns_portlist_t **portlistp) {
-	dns_portlist_t *portlist;
-
-	REQUIRE(portlistp != NULL);
-	portlist = *portlistp;
-	REQUIRE(DNS_VALID_PORTLIST(portlist));
+	REQUIRE(portlistp != NULL && DNS_VALID_PORTLIST(*portlistp));
+	dns_portlist_t *portlist = *portlistp;
+	*portlistp = NULL;
 
 	if (isc_refcount_decrement(&portlist->refcount) == 1) {
 		portlist->magic = 0;
@@ -251,5 +249,4 @@ dns_portlist_detach(dns_portlist_t **portlistp) {
 		isc_mem_putanddetach(&portlist->mctx, portlist,
 				     sizeof(*portlist));
 	}
-	*portlistp = NULL;
 }
diff --git a/lib/dns/rbtdb.c b/lib/dns/rbtdb.c
index e30f1f0ce60..92d8d0ec727 100644
--- a/lib/dns/rbtdb.c
+++ b/lib/dns/rbtdb.c
@@ -968,7 +968,9 @@ free_rbtdb(dns_rbtdb_t *rbtdb, bool log, isc_event_t *event) {
 
 	if (rbtdb->current_version != NULL) {
 
-		INSIST(isc_refcount_decrement(&rbtdb->current_version->references) == 1);
+		INSIST(isc_refcount_decrement(
+			       &rbtdb->current_version->references)
+		       == 1);
 
 		UNLINK(rbtdb->open_versions, rbtdb->current_version, link);
 		isc_rwlock_destroy(&rbtdb->current_version->glue_rwlock);
@@ -1179,15 +1181,14 @@ maybe_free_rbtdb(dns_rbtdb_t *rbtdb) {
 
 static void
 detach(dns_db_t **dbp) {
+	REQUIRE(dbp != NULL && VALID_RBTDB((dns_rbtdb_t *)(*dbp)));
 	dns_rbtdb_t *rbtdb = (dns_rbtdb_t *)(*dbp);
-
-	REQUIRE(VALID_RBTDB(rbtdb));
+	*dbp = NULL;
 
 	if (isc_refcount_decrement(&rbtdb->references) == 1) {
+		(void)isc_refcount_current(&rbtdb->references);
 		maybe_free_rbtdb(rbtdb);
 	}
-
-	*dbp = NULL;
 }
 
 static void
@@ -1991,7 +1992,7 @@ decrement_reference(dns_rbtdb_t *rbtdb, dns_rbtnode_t *node,
 	} else
 		write_locked = true;
 
-	isc_refcount_decrement(&nodelock->references);
+	INSIST(isc_refcount_decrement(&nodelock->references) > 0);
 
 	if (KEEP_NODE(node, rbtdb))
 		goto restore_locks;
@@ -2363,6 +2364,7 @@ cleanup_dead_nodes_callback(isc_task_t *task, isc_event_t *event) {
 	else {
 		isc_event_free(&event);
 		if (isc_refcount_decrement(&rbtdb->references) == 1) {
+			(void)isc_refcount_current(&rbtdb->references);
 			maybe_free_rbtdb(rbtdb);
 		}
 	}
@@ -2422,6 +2424,7 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp, bool commit) {
 			cur_version = rbtdb->current_version;
 			cur_ref = isc_refcount_decrement(&cur_version->references);
 			if (cur_ref == 1) {
+				(void)isc_refcount_current(&cur_version->references);
 				if (cur_version->serial == rbtdb->least_serial)
 					INSIST(EMPTY(cur_version->changed_list));
 				UNLINK(rbtdb->open_versions,
@@ -8318,7 +8321,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 	rbtdb->next_serial = 2;
 	rbtdb->current_version = allocate_version(mctx, 1, 1, false);
 	if (rbtdb->current_version == NULL) {
-		isc_refcount_decrement(&rbtdb->references);
+		INSIST(isc_refcount_decrement(&rbtdb->references) > 0);
 		free_rbtdb(rbtdb, false, NULL);
 		return (ISC_R_NOMEMORY);
 	}
@@ -8339,7 +8342,7 @@ dns_rbtdb_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 		isc_mem_put(mctx, rbtdb->current_version,
 			    sizeof(*rbtdb->current_version));
 		rbtdb->current_version = NULL;
-		isc_refcount_decrement(&rbtdb->references);
+		INSIST(isc_refcount_decrement(&rbtdb->references) > 0);
 		free_rbtdb(rbtdb, false, NULL);
 		return (result);
 	}
diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c
index 030365cd0ed..5ca06807763 100644
--- a/lib/dns/rpz.c
+++ b/lib/dns/rpz.c
@@ -1484,7 +1484,7 @@ cleanup_task:
 	dns_rbt_destroy(&zones->rbt);
 
 cleanup_rbt:
-	(void)isc_refcount_decrement(&zones->refs);
+	INSIST(isc_refcount_decrement(&zones->refs) > 0);
 	isc_refcount_destroy(&zones->refs);
 
 	DESTROYLOCK(&zones->maint_lock);
@@ -1567,7 +1567,7 @@ cleanup_ht:
 	isc_timer_detach(&zone->updatetimer);
 
 cleanup_timer:
-	isc_refcount_decrement(&zone->refs);
+	INSIST(isc_refcount_decrement(&zone->refs) > 0);
 	isc_refcount_destroy(&zone->refs);
 
 	isc_mem_put(zone->rpzs->mctx, zone, sizeof(*zone));
@@ -2048,9 +2048,9 @@ cidr_free(dns_rpz_zones_t *rpzs) {
  */
 static void
 rpz_detach(dns_rpz_zone_t **rpzp, dns_rpz_zones_t *rpzs) {
-	dns_rpz_zone_t *rpz;
-
-	rpz = *rpzp;
+	REQUIRE(rpzp != NULL && *rpzp != NULL);
+	dns_rpz_zone_t *rpz = *rpzp;
+	*rpzp = NULL;
 
 	if (isc_refcount_decrement(&rpz->refs) == 1) {
 		isc_refcount_destroy(&rpz->refs);
@@ -2086,8 +2086,6 @@ rpz_detach(dns_rpz_zone_t **rpzp, dns_rpz_zones_t *rpzs) {
 
 		isc_mem_put(rpzs->mctx, rpz, sizeof(*rpz));
 	}
-
-	*rpzp = NULL;
 }
 
 void
@@ -2102,23 +2100,20 @@ dns_rpz_attach_rpzs(dns_rpz_zones_t *rpzs, dns_rpz_zones_t **rpzsp) {
  */
 void
 dns_rpz_detach_rpzs(dns_rpz_zones_t **rpzsp) {
-	dns_rpz_zones_t *rpzs;
-	dns_rpz_zone_t *rpz;
-	dns_rpz_num_t rpz_num;
-
-	REQUIRE(rpzsp != NULL);
-	rpzs = *rpzsp;
-	REQUIRE(rpzs != NULL);
-
+	REQUIRE(rpzsp != NULL && *rpzsp != NULL);
+	dns_rpz_zones_t *rpzs = *rpzsp;
 	*rpzsp = NULL;
+
 	if (isc_refcount_decrement(&rpzs->refs) == 1) {
 		isc_refcount_destroy(&rpzs->refs);
-
 		/*
 		 * Forget the last of view's rpz machinery after the last reference.
 		 */
-		for (rpz_num = 0; rpz_num < DNS_RPZ_MAX_ZONES; ++rpz_num) {
-			rpz = rpzs->zones[rpz_num];
+		for (dns_rpz_num_t rpz_num = 0;
+		     rpz_num < DNS_RPZ_MAX_ZONES;
+		     ++rpz_num)
+		{
+			dns_rpz_zone_t *rpz = rpzs->zones[rpz_num];
 			rpzs->zones[rpz_num] = NULL;
 			if (rpz != NULL) {
 				rpz_detach(&rpz, rpzs);
diff --git a/lib/dns/tsig.c b/lib/dns/tsig.c
index 529efdfe844..71580418ac4 100644
--- a/lib/dns/tsig.c
+++ b/lib/dns/tsig.c
@@ -370,7 +370,7 @@ dns_tsigkey_createfromkey(const dns_name_t *name, const dns_name_t *algorithm,
  cleanup_refs:
 	tkey->magic = 0;
 	while (refs-- > 0) {
-		(void)isc_refcount_decrement(&tkey->refs);
+		INSIST(isc_refcount_decrement(&tkey->refs) > 0);
 	}
 	isc_refcount_destroy(&tkey->refs);
 
@@ -729,24 +729,19 @@ tsigkey_free(dns_tsigkey_t *key) {
 		dns_name_free(key->creator, key->mctx);
 		isc_mem_put(key->mctx, key->creator, sizeof(dns_name_t));
 	}
-	isc_refcount_destroy(&key->refs);
 	isc_mem_putanddetach(&key->mctx, key, sizeof(dns_tsigkey_t));
 }
 
 void
 dns_tsigkey_detach(dns_tsigkey_t **keyp) {
-	dns_tsigkey_t *key;
-
-	REQUIRE(keyp != NULL);
-	REQUIRE(VALID_TSIG_KEY(*keyp));
-
-	key = *keyp;
+	REQUIRE(keyp != NULL && VALID_TSIG_KEY(*keyp));
+	dns_tsigkey_t *key = *keyp;
+	*keyp = NULL;
 
 	if (isc_refcount_decrement(&key->refs) == 1) {
+		isc_refcount_destroy(&key->refs);
 		tsigkey_free(key);
 	}
-
-	*keyp = NULL;
 }
 
 void
diff --git a/lib/dns/view.c b/lib/dns/view.c
index 751b0d18afb..09630e6a8a2 100644
--- a/lib/dns/view.c
+++ b/lib/dns/view.c
@@ -311,7 +311,7 @@ dns_view_create(isc_mem_t *mctx, dns_rdataclass_t rdclass,
 		dns_tsigkeyring_detach(&view->dynamickeys);
 
  cleanup_references:
-	(void)isc_refcount_decrement(&view->references);
+	INSIST(isc_refcount_decrement(&view->references) > 0);
 	isc_refcount_destroy(&view->references);
 
 	if (view->fwdtable != NULL)
@@ -542,7 +542,6 @@ destroy(dns_view_t *view) {
 		dns_badcache_destroy(&view->failcache);
 	DESTROYLOCK(&view->new_zone_lock);
 	DESTROYLOCK(&view->lock);
-	isc_refcount_destroy(&view->references);
 	isc_mem_free(view->mctx, view->nta_file);
 	isc_mem_free(view->mctx, view->name);
 	isc_mem_putanddetach(&view->mctx, view, sizeof(*view));
@@ -576,36 +575,42 @@ dns_view_attach(dns_view_t *source, dns_view_t **targetp) {
 
 static void
 view_flushanddetach(dns_view_t **viewp, bool flush) {
-	dns_view_t *view;
-	bool done = false;
+	REQUIRE(viewp != NULL && DNS_VIEW_VALID(*viewp));
+	dns_view_t *view = *viewp;
+	*viewp = NULL;
 
-	REQUIRE(viewp != NULL);
-	view = *viewp;
-	REQUIRE(DNS_VIEW_VALID(view));
+	if (flush) {
+		view->flush = flush;
+	}
 
-	if (flush)
-		view->flush = true;
+	bool done = false;
 	if (isc_refcount_decrement(&view->references) == 1) {
 		dns_zone_t *mkzone = NULL, *rdzone = NULL;
 
+		isc_refcount_destroy(&view->references);
 		LOCK(&view->lock);
-		if (!RESSHUTDOWN(view))
+		if (!RESSHUTDOWN(view)) {
 			dns_resolver_shutdown(view->resolver);
-		if (!ADBSHUTDOWN(view))
+		}
+		if (!ADBSHUTDOWN(view)) {
 			dns_adb_shutdown(view->adb);
-		if (!REQSHUTDOWN(view))
+		}
+		if (!REQSHUTDOWN(view)) {
 			dns_requestmgr_shutdown(view->requestmgr);
+		}
 		if (view->zonetable != NULL) {
-			if (view->flush)
+			if (view->flush) {
 				dns_zt_flushanddetach(&view->zonetable);
-			else
+			} else {
 				dns_zt_detach(&view->zonetable);
+			}
 		}
 		if (view->managed_keys != NULL) {
 			mkzone = view->managed_keys;
 			view->managed_keys = NULL;
-			if (view->flush)
+			if (view->flush) {
 				dns_zone_flush(mkzone);
+			}
 		}
 		if (view->redirect != NULL) {
 			rdzone = view->redirect;
@@ -620,17 +625,20 @@ view_flushanddetach(dns_view_t **viewp, bool flush) {
 		UNLOCK(&view->lock);
 
 		/* Need to detach zones outside view lock */
-		if (mkzone != NULL)
+		if (mkzone != NULL) {
 			dns_zone_detach(&mkzone);
+		}
 
-		if (rdzone != NULL)
+		if (rdzone != NULL) {
 			dns_zone_detach(&rdzone);
+		}
 	}
 
 	*viewp = NULL;
 
-	if (done)
+	if (done) {
 		destroy(view);
+	}
 }
 
 void
diff --git a/lib/dns/zone.c b/lib/dns/zone.c
index b9e2b83e3e2..c350c074739 100644
--- a/lib/dns/zone.c
+++ b/lib/dns/zone.c
@@ -1083,7 +1083,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) {
 		isc_stats_detach(&zone->gluecachestats);
 
  free_erefs:
-	(void)isc_refcount_decrement(&zone->erefs);
+	INSIST(isc_refcount_decrement(&zone->erefs) > 0);
 	isc_refcount_destroy(&zone->erefs);
 
 	ZONEDB_DESTROYLOCK(&zone->dblock);
@@ -1256,7 +1256,6 @@ zone_free(dns_zone_t *zone) {
 	/* last stuff */
 	ZONEDB_DESTROYLOCK(&zone->dblock);
 	DESTROYLOCK(&zone->lock);
-	isc_refcount_destroy(&zone->erefs);
 	zone->magic = 0;
 	mctx = zone->mctx;
 	isc_mem_put(mctx, zone, sizeof(*zone));
@@ -5258,16 +5257,16 @@ dns_zone_attach(dns_zone_t *source, dns_zone_t **target) {
 
 void
 dns_zone_detach(dns_zone_t **zonep) {
-	dns_zone_t *zone;
-	dns_zone_t *raw = NULL;
-	dns_zone_t *secure = NULL;
-	bool free_now = false;
-
 	REQUIRE(zonep != NULL && DNS_ZONE_VALID(*zonep));
+	dns_zone_t *zone = *zonep;
+	*zonep = NULL;
 
-	zone = *zonep;
-
+	bool free_now = false;
+	dns_zone_t *raw = NULL;
+	dns_zone_t *secure = NULL;
 	if (isc_refcount_decrement(&zone->erefs) == 1) {
+		isc_refcount_destroy(&zone->erefs);
+
 		LOCK_ZONE(zone);
 		INSIST(zone != zone->raw);
 		/*
@@ -5303,12 +5302,13 @@ dns_zone_detach(dns_zone_t **zonep) {
 		}
 		UNLOCK_ZONE(zone);
 	}
-	*zonep = NULL;
 	if (free_now) {
-		if (raw != NULL)
+		if (raw != NULL) {
 			dns_zone_detach(&raw);
-		if (secure != NULL)
+		}
+		if (secure != NULL) {
 			dns_zone_idetach(&secure);
+		}
 		zone_free(zone);
 	}
 }
diff --git a/lib/isc/mem.c b/lib/isc/mem.c
index 371b2e3f087..16609cb7523 100644
--- a/lib/isc/mem.c
+++ b/lib/isc/mem.c
@@ -1049,7 +1049,6 @@ destroy(isc__mem_t *ctx) {
 		ctx->malloced -= DEBUG_TABLE_COUNT * sizeof(debuglist_t);
 	}
 #endif
-	isc_refcount_destroy(&ctx->references);
 
 	if (ctx->checkfree) {
 		for (i = 0; i <= ctx->max_size; i++) {
@@ -1106,17 +1105,14 @@ isc__mem_attach(isc_mem_t *source0, isc_mem_t **targetp) {
 
 void
 isc__mem_detach(isc_mem_t **ctxp) {
-	isc__mem_t *ctx;
-
-	REQUIRE(ctxp != NULL);
-	ctx = (isc__mem_t *)*ctxp;
-	REQUIRE(VALID_CONTEXT(ctx));
+	REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp));
+	isc__mem_t *ctx = (isc__mem_t *)*ctxp;
+	*ctxp = NULL;
 
 	if (isc_refcount_decrement(&ctx->references) == 1) {
+		isc_refcount_destroy(&ctx->references);
 		destroy(ctx);
 	}
-
-	*ctxp = NULL;
 }
 
 /*
@@ -1131,38 +1127,24 @@ isc__mem_detach(isc_mem_t **ctxp) {
 
 void
 isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) {
-	isc__mem_t *ctx;
-	size_info *si;
-	size_t oldsize;
-
-	REQUIRE(ctxp != NULL);
-	ctx = (isc__mem_t *)*ctxp;
-	REQUIRE(VALID_CONTEXT(ctx));
+	REQUIRE(ctxp != NULL && VALID_CONTEXT(*ctxp));
 	REQUIRE(ptr != NULL);
-
-	/*
-	 * Must be before mem_putunlocked() as ctxp is usually within
-	 * [ptr..ptr+size).
-	 */
+	isc__mem_t *ctx = (isc__mem_t *)*ctxp;
 	*ctxp = NULL;
 
 	if (ISC_UNLIKELY((isc_mem_debugging &
 			  (ISC_MEM_DEBUGSIZE|ISC_MEM_DEBUGCTX)) != 0))
 	{
 		if ((isc_mem_debugging & ISC_MEM_DEBUGSIZE) != 0) {
-			si = &(((size_info *)ptr)[-1]);
-			oldsize = si->u.size - ALIGNMENT_SIZE;
+			size_info *si = &(((size_info *)ptr)[-1]);
+			size_t oldsize = si->u.size - ALIGNMENT_SIZE;
 			if ((isc_mem_debugging & ISC_MEM_DEBUGCTX) != 0)
 				oldsize -= ALIGNMENT_SIZE;
 			INSIST(oldsize == size);
 		}
 		isc__mem_free((isc_mem_t *)ctx, ptr FLARG_PASS);
 
-		if (isc_refcount_decrement(&ctx->references) == 1) {
-			destroy(ctx);
-		}
-
-		return;
+		goto destroy;
 	}
 
 	MCTXLOCK(ctx, &ctx->lock);
@@ -1177,7 +1159,9 @@ isc___mem_putanddetach(isc_mem_t **ctxp, void *ptr, size_t size FLARG) {
 	}
 	MCTXUNLOCK(ctx, &ctx->lock);
 
+destroy:
 	if (isc_refcount_decrement(&ctx->references) == 1) {
+		isc_refcount_destroy(&ctx->references);
 		destroy(ctx);
 	}
 }
@@ -1200,7 +1184,7 @@ isc__mem_destroy(isc_mem_t **ctxp) {
 		print_active(ctx, stderr);
 	}
 #else
-	(void)isc_refcount_decrement(&ctx->references);
+	INSIST(isc_refcount_decrement(&ctx->references) == 1);
 #endif
 	isc_refcount_destroy(&ctx->references);
 	destroy(ctx);
diff --git a/lib/isc/radix.c b/lib/isc/radix.c
index 040a386f696..8f5de8217fa 100644
--- a/lib/isc/radix.c
+++ b/lib/isc/radix.c
@@ -76,13 +76,12 @@ _new_prefix(isc_mem_t *mctx, isc_prefix_t **target, int family, void *dest,
 
 static void
 _deref_prefix(isc_prefix_t *prefix) {
-	if (prefix == NULL)
-		return;
-
-	if (isc_refcount_decrement(&prefix->refcount) == 1) {
-		isc_refcount_destroy(&prefix->refcount);
-		isc_mem_putanddetach(&prefix->mctx, prefix,
-				     sizeof(isc_prefix_t));
+	if (prefix != NULL) {
+		if (isc_refcount_decrement(&prefix->refcount) == 1) {
+			isc_refcount_destroy(&prefix->refcount);
+			isc_mem_putanddetach(&prefix->mctx, prefix,
+					     sizeof(isc_prefix_t));
+		}
 	}
 }
 
@@ -706,10 +705,3 @@ isc_radix_remove(isc_radix_tree_t *radix, isc_radix_node_t *node) {
 		parent->l = child;
 	}
 }
-
-/*
-Local Variables:
-c-basic-offset: 4
-indent-tabs-mode: t
-End:
-*/
diff --git a/lib/isccfg/aclconf.c b/lib/isccfg/aclconf.c
index 4d43fabce17..02c18209849 100644
--- a/lib/isccfg/aclconf.c
+++ b/lib/isccfg/aclconf.c
@@ -70,14 +70,13 @@ cfg_aclconfctx_attach(cfg_aclconfctx_t *src, cfg_aclconfctx_t **dest) {
 
 void
 cfg_aclconfctx_detach(cfg_aclconfctx_t **actxp) {
-	cfg_aclconfctx_t *actx;
-	dns_acl_t *dacl, *next;
-
 	REQUIRE(actxp != NULL && *actxp != NULL);
-
-	actx = *actxp;
+	cfg_aclconfctx_t *actx = *actxp;
+	*actxp = NULL;
 
 	if (isc_refcount_decrement(&actx->references) == 1) {
+		dns_acl_t *dacl, *next;
+		isc_refcount_destroy(&actx->references);
 		for (dacl = ISC_LIST_HEAD(actx->named_acl_cache);
 		     dacl != NULL;
 		     dacl = next)
@@ -89,8 +88,6 @@ cfg_aclconfctx_detach(cfg_aclconfctx_t **actxp) {
 		}
 		isc_mem_putanddetach(&actx->mctx, actx, sizeof(*actx));
 	}
-
-	*actxp = NULL;
 }
 
 /*
diff --git a/lib/isccfg/parser.c b/lib/isccfg/parser.c
index 656a0f683ba..c370b1980ee 100644
--- a/lib/isccfg/parser.c
+++ b/lib/isccfg/parser.c
@@ -675,7 +675,6 @@ cfg_parser_destroy(cfg_parser_t **pctxp) {
 	cfg_parser_t *pctx;
 
 	REQUIRE(pctxp != NULL && *pctxp != NULL);
-
 	pctx = *pctxp;
 	*pctxp = NULL;
 
@@ -3132,19 +3131,17 @@ cfg_obj_istype(const cfg_obj_t *obj, const cfg_type_t *type) {
  */
 void
 cfg_obj_destroy(cfg_parser_t *pctx, cfg_obj_t **objp) {
-	cfg_obj_t *obj;
-
 	REQUIRE(objp != NULL && *objp != NULL);
 	REQUIRE(pctx != NULL);
 
-	obj = *objp;
+	cfg_obj_t *obj = *objp;
+	*objp = NULL;
 
 	if (isc_refcount_decrement(&obj->references) == 1) {
 		obj->type->rep->free(pctx, obj);
 		isc_refcount_destroy(&obj->references);
 		isc_mem_put(pctx->mctx, obj, sizeof(cfg_obj_t));
 	}
-	*objp = NULL;
 }
 
 void
diff --git a/lib/ns/server.c b/lib/ns/server.c
index 4a133f3c51c..91ae32d6923 100644
--- a/lib/ns/server.c
+++ b/lib/ns/server.c
@@ -127,9 +127,9 @@ void
 ns_server_detach(ns_server_t **sctxp) {
 	ns_server_t *sctx;
 
-	REQUIRE(sctxp != NULL);
+	REQUIRE(sctxp != NULL && SCTX_VALID(*sctxp));
 	sctx = *sctxp;
-	REQUIRE(SCTX_VALID(sctx));
+	*sctxp = NULL;
 
 	if (isc_refcount_decrement(&sctx->references) == 1) {
 		ns_altsecret_t *altsecret;
@@ -185,8 +185,6 @@ ns_server_detach(ns_server_t **sctxp) {
 
 		isc_mem_putanddetach(&sctx->mctx, sctx, sizeof(*sctx));
 	}
-
-	*sctxp = NULL;
 }
 
 isc_result_t