chg: dev: Extract the resigning heap into a separate struct

In the current implementation, the resigning heap is part of the zone
database. This leads to a cycle, as the database has a reference to its
nodes, but each node needs a reference to the database.

This MR splits the resigning heap into its own separate struct, in order
to help breaking the cycle.

Merge branch 'alessio/split-qpzone-heap-from-qpdb' into 'main'

See merge request isc-projects/bind9!10706

lib/dns/qpzone.c

@@ -91,7 +91,9 @@
 #define QPDB_ATTR_LOADED  0x01
 #define QPDB_ATTR_LOADING 0x02
 
+#ifndef DEFAULT_BUCKETS_COUNT
 #define DEFAULT_BUCKETS_COUNT 17 /*%< Should be prime. */
+#endif
 
 #define QPDBITER_NSEC3_ORIGIN_NODE(qpdb, iterator)        \
 	((iterator)->current == &(iterator)->nsec3iter && \
@@ -108,6 +110,15 @@
 typedef struct qpzonedb qpzonedb_t;
 typedef struct qpznode qpznode_t;
 
+typedef struct qpzone_bucket {
+	/* Per-bucket lock. */
+	isc_rwlock_t lock;
+
+	/* Padding to prevent false sharing between locks. */
+	uint8_t __padding[ISC_OS_CACHELINE_SIZE -
+			  (sizeof(isc_rwlock_t)) % ISC_OS_CACHELINE_SIZE];
+} qpzone_bucket_t;
+
 typedef struct qpz_changed {
 	qpznode_t *node;
 	bool dirty;
@@ -148,10 +159,23 @@ struct qpz_version {
 
 typedef ISC_LIST(qpz_version_t) qpz_versionlist_t;
 
+/* Resigning heap indirection to allow ref counting */
+typedef struct qpz_heap {
+	isc_mem_t *mctx;
+	isc_refcount_t references;
+	/* Locks the data in this struct */
+	isc_mutex_t lock;
+	isc_heap_t *heap;
+} qpz_heap_t;
+
+ISC_REFCOUNT_STATIC_DECL(qpz_heap);
+
 struct qpznode {
 	dns_name_t name;
 	isc_mem_t *mctx;
 
+	qpz_heap_t *heap;
+
 	/*
 	 * 'erefs' counts external references held by a caller: for
 	 * example, it could be incremented by dns_db_findnode(),
@@ -185,15 +209,6 @@ struct qpznode {
 	void *data;
 };
 
-typedef struct qpzone_bucket {
-	/* Per-bucket lock. */
-	isc_rwlock_t lock;
-
-	/* Padding to prevent false sharing between locks. */
-	uint8_t __padding[ISC_OS_CACHELINE_SIZE -
-			  (sizeof(isc_rwlock_t)) % ISC_OS_CACHELINE_SIZE];
-} qpzone_bucket_t;
-
 struct qpzonedb {
 	/* Unlocked. */
 	dns_db_t common;
@@ -233,14 +248,13 @@ struct qpzonedb {
 	isc_loop_t *loop;
 	struct rcu_head rcu_head;
 
-	isc_heap_t *heap; /* Resigning heap */
+	qpz_heap_t *heap; /* Resigning heap */
 
 	dns_qpmulti_t *tree;  /* Main QP trie for data storage */
 	dns_qpmulti_t *nsec;  /* NSEC nodes only */
 	dns_qpmulti_t *nsec3; /* NSEC3 nodes only */
 
-	size_t buckets_count;
+	qpzone_bucket_t buckets[DEFAULT_BUCKETS_COUNT];
-	qpzone_bucket_t buckets[]; /* attribute((counted_by(buckets_count))) */
 };
 
 #ifdef DNS_DB_NODETRACE
@@ -404,6 +418,16 @@ static atomic_uint_fast16_t init_count = 0;
  * Failure to follow this hierarchy can result in deadlock.
  */
 
+static isc_rwlock_t *
+qpzone_get_lock(qpzonedb_t *qpdb, qpznode_t *node) {
+	return &qpdb->buckets[node->locknum].lock;
+}
+
+static uint16_t
+qpzone_get_locknum(void) {
+	return isc_random_uniform(DEFAULT_BUCKETS_COUNT);
+}
+
 /*%
  * Return which RRset should be resigned sooner.  If the RRsets have the
  * same signing time, prefer the other RRset over the SOA RRset.
@@ -507,11 +531,12 @@ free_db_rcu(struct rcu_head *rcu_head) {
 	if (dns_name_dynamic(&qpdb->common.origin)) {
 		dns_name_free(&qpdb->common.origin, qpdb->common.mctx);
 	}
-	for (size_t i = 0; i < qpdb->buckets_count; i++) {
+
+	for (size_t i = 0; i < DEFAULT_BUCKETS_COUNT; i++) {
 		NODE_DESTROYLOCK(&qpdb->buckets[i].lock);
 	}
 
-	isc_heap_destroy(&qpdb->heap);
+	qpz_heap_detach(&qpdb->heap);
 
 	if (qpdb->gluecachestats != NULL) {
 		isc_stats_detach(&qpdb->gluecachestats);
@@ -532,9 +557,7 @@ free_db_rcu(struct rcu_head *rcu_head) {
 		INSIST(!cds_lfht_destroy(qpdb->common.update_listeners, NULL));
 	}
 
-	isc_mem_putanddetach(&qpdb->common.mctx, qpdb,
+	isc_mem_putanddetach(&qpdb->common.mctx, qpdb, sizeof(*qpdb));
-			     sizeof(*qpdb) + qpdb->buckets_count *
-						     sizeof(qpdb->buckets[0]));
 }
 
 static void
@@ -589,17 +612,65 @@ qpdb_destroy(dns_db_t *arg) {
 	qpzonedb_detach(&qpdb);
 }
 
+static qpz_heap_t *
+new_qpz_heap(isc_mem_t *mctx) {
+	qpz_heap_t *new_heap = isc_mem_get(mctx, sizeof(*new_heap));
+	*new_heap = (qpz_heap_t){
+		.references = ISC_REFCOUNT_INITIALIZER(1),
+	};
+
+	isc_mutex_init(&new_heap->lock);
+	isc_heap_create(mctx, resign_sooner, set_index, 0, &new_heap->heap);
+	isc_mem_attach(mctx, &new_heap->mctx);
+
+	return new_heap;
+}
+
+/*
+ * This function accesses the heap lock through the header and node rather than
+ * directly through &qpdb->heap->lock to handle a critical race condition.
+ *
+ * Consider this scenario:
+ * 1. A reference is taken to a qpznode
+ * 2. The database containing that node is freed
+ * 3. The qpznode reference is finally released
+ *
+ * When the qpznode reference is released, it needs to unregister all its
+ * slabheaders from the resigning heap. The heap is a separate refcounted
+ * object with references from both the database and every qpznode. This
+ * design ensures that even after the database is destroyed, if nodes are
+ * still alive, the heap remains accessible for safe cleanup.
+ *
+ * Accessing the heap lock through the database (&qpdb->heap->lock) would
+ * cause a segfault in this scenario, even though the heap itself is still
+ * alive. By going through the node's heap reference, we maintain safe access
+ * to the heap lock regardless of the database's lifecycle.
+ */
+static isc_mutex_t *
+get_heap_lock(dns_slabheader_t *header) {
+	return &HEADERNODE(header)->heap->lock;
+}
+
+static void
+qpz_heap_destroy(qpz_heap_t *qpheap) {
+	isc_mutex_destroy(&qpheap->lock);
+	isc_heap_destroy(&qpheap->heap);
+	isc_mem_putanddetach(&qpheap->mctx, qpheap, sizeof(*qpheap));
+}
+
 static qpznode_t *
 new_qpznode(qpzonedb_t *qpdb, const dns_name_t *name) {
 	qpznode_t *newdata = isc_mem_get(qpdb->common.mctx, sizeof(*newdata));
 	*newdata = (qpznode_t){
 		.name = DNS_NAME_INITEMPTY,
+		.heap = qpdb->heap,
 		.references = ISC_REFCOUNT_INITIALIZER(1),
-		.locknum = isc_random_uniform(qpdb->buckets_count),
+		.locknum = qpzone_get_locknum(),
 	};
 
 	isc_mem_attach(qpdb->common.mctx, &newdata->mctx);
 	dns_name_dup(name, qpdb->common.mctx, &newdata->name);
+	qpz_heap_ref(newdata->heap);
 
 #if DNS_DB_NODETRACE
 	fprintf(stderr, "new_qpznode:%s:%s:%d:%p->references = 1\n", __func__,
@@ -636,14 +707,11 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 	isc_result_t result;
 	dns_qp_t *qp = NULL;
 
-	qpdb = isc_mem_get(mctx,
+	qpdb = isc_mem_get(mctx, sizeof(*qpdb));
-			   sizeof(*qpdb) + DEFAULT_BUCKETS_COUNT *
-						   sizeof(qpdb->buckets[0]));
 	*qpdb = (qpzonedb_t){
 		.common.origin = DNS_NAME_INITEMPTY,
 		.common.rdclass = rdclass,
 		.common.references = ISC_REFCOUNT_INITIALIZER(1),
-		.buckets_count = DEFAULT_BUCKETS_COUNT,
 		.current_serial = 1,
 		.least_serial = 1,
 		.next_serial = 2,
@@ -660,9 +728,9 @@ dns__qpzone_create(isc_mem_t *mctx, const dns_name_t *origin, dns_dbtype_t type,
 
 	qpdb->common.update_listeners = cds_lfht_new(16, 16, 0, 0, NULL);
 
-	isc_heap_create(mctx, resign_sooner, set_index, 0, &qpdb->heap);
+	qpdb->heap = new_qpz_heap(mctx);
 
-	for (size_t i = 0; i < qpdb->buckets_count; i++) {
+	for (size_t i = 0; i < DEFAULT_BUCKETS_COUNT; i++) {
 		NODE_INITLOCK(&qpdb->buckets[i].lock);
 	}
 
@@ -944,7 +1012,7 @@ qpznode_release(qpzonedb_t *qpdb, qpznode_t *node, uint32_t least_serial,
 		 * erefs_increment.  If another thread acquires reference it
 		 * will be larger than 0, if it doesn't it is going to be 0.
 		 */
-		isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
+		isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
 		qpznode_erefs_increment(qpdb, node DNS__DB_FLARG_PASS);
 		NODE_FORCEUPGRADE(nlock, nlocktypep);
 		if (!qpznode_erefs_decrement(qpdb, node DNS__DB_FLARG_PASS)) {
@@ -1038,7 +1106,7 @@ setnsec3parameters(dns_db_t *db, qpz_version_t *version) {
 
 	version->havensec3 = false;
 	node = qpdb->origin;
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 	NODE_RDLOCK(nlock, &nlocktype);
 	for (header = node->data; header != NULL; header = header_next) {
 		header_next = header->next;
@@ -1243,15 +1311,13 @@ newversion(dns_db_t *db, dns_dbversion_t **versionp) {
 }
 
 static void
-resigninsert(qpzonedb_t *qpdb, dns_slabheader_t *newheader) {
+resigninsert(dns_slabheader_t *newheader) {
 	REQUIRE(newheader->heap_index == 0);
 	REQUIRE(!ISC_LINK_LINKED(newheader, link));
 
-	RWLOCK(&qpdb->lock, isc_rwlocktype_write);
+	LOCK(get_heap_lock(newheader));
-	isc_heap_insert(qpdb->heap, newheader);
+	isc_heap_insert(HEADERNODE(newheader)->heap->heap, newheader);
-	RWUNLOCK(&qpdb->lock, isc_rwlocktype_write);
+	UNLOCK(get_heap_lock(newheader));
-
-	newheader->heap = qpdb->heap;
 }
 
 static void
@@ -1261,9 +1327,9 @@ resigndelete(qpzonedb_t *qpdb, qpz_version_t *version,
 		return;
 	}
 
-	RWLOCK(&qpdb->lock, isc_rwlocktype_write);
+	LOCK(get_heap_lock(header));
-	isc_heap_delete(qpdb->heap, header->heap_index);
+	isc_heap_delete(HEADERNODE(header)->heap->heap, header->heap_index);
-	RWUNLOCK(&qpdb->lock, isc_rwlocktype_write);
+	UNLOCK(get_heap_lock(header));
 
 	header->heap_index = 0;
 	qpznode_acquire(qpdb, HEADERNODE(header) DNS__DB_FLARG_PASS);
@@ -1498,10 +1564,10 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
 
 		ISC_LIST_UNLINK(resigned_list, header, link);
 
-		nlock = &qpdb->buckets[HEADERNODE(header)->locknum].lock;
+		nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
 		NODE_WRLOCK(nlock, &nlocktype);
 		if (rollback && !IGNORE(header)) {
-			resigninsert(qpdb, header);
+			resigninsert(header);
 		}
 		qpznode_release(qpdb, HEADERNODE(header), least_serial,
 				&nlocktype DNS__DB_FLARG_PASS);
@@ -1518,7 +1584,7 @@ closeversion(dns_db_t *db, dns_dbversion_t **versionp,
 		isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
 
 		node = changed->node;
-		nlock = &qpdb->buckets[node->locknum].lock;
+		nlock = qpzone_get_lock(qpdb, node);
 
 		NODE_WRLOCK(nlock, &nlocktype);
 		if (rollback) {
@@ -1562,7 +1628,7 @@ qpzone_findrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 	}
 	serial = version->serial;
 
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 	NODE_RDLOCK(nlock, &nlocktype);
 
 	matchtype = DNS_TYPEPAIR_VALUE(type, covers);
@@ -1919,7 +1985,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 		if (loading) {
 			newheader->down = NULL;
 			if (RESIGN(newheader)) {
-				resigninsert(qpdb, newheader);
+				resigninsert(newheader);
 				/* resigndelete not needed here */
 			}
 
@@ -1940,7 +2006,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 			dns_slabheader_destroy(&header);
 		} else {
 			if (RESIGN(newheader)) {
-				resigninsert(qpdb, newheader);
+				resigninsert(newheader);
 				resigndelete(qpdb, version,
 					     header DNS__DB_FLARG_PASS);
 			}
@@ -1972,7 +2038,7 @@ add(qpzonedb_t *qpdb, qpznode_t *node, const dns_name_t *nodename,
 		}
 
 		if (RESIGN(newheader)) {
-			resigninsert(qpdb, newheader);
+			resigninsert(newheader);
 			resigndelete(qpdb, version, header DNS__DB_FLARG_PASS);
 		}
 
@@ -2168,7 +2234,7 @@ loading_addrdataset(void *arg, const dns_name_t *name,
 		newheader->resign_lsb = rdataset->resign & 0x1;
 	}
 
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 	NODE_WRLOCK(nlock, &nlocktype);
 	result = add(qpdb, node, name, qpdb->current_version, newheader,
 		     DNS_DBADD_MERGE, true, NULL, 0 DNS__DB_FLARG_PASS);
@@ -2377,7 +2443,7 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
 
 	header = dns_rdataset_getheader(rdataset);
 
-	nlock = &qpdb->buckets[HEADERNODE(header)->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
 	NODE_WRLOCK(nlock, &nlocktype);
 
 	oldheader = *header;
@@ -2394,20 +2460,22 @@ setsigningtime(dns_db_t *db, dns_rdataset_t *rdataset, isc_stdtime_t resign) {
 	}
 	if (header->heap_index != 0) {
 		INSIST(RESIGN(header));
-		RWLOCK(&qpdb->lock, isc_rwlocktype_write);
+		LOCK(get_heap_lock(header));
 		if (resign == 0) {
-			isc_heap_delete(qpdb->heap, header->heap_index);
+			isc_heap_delete(HEADERNODE(header)->heap->heap,
+					header->heap_index);
 			header->heap_index = 0;
-			header->heap = NULL;
 		} else if (resign_sooner(header, &oldheader)) {
-			isc_heap_increased(qpdb->heap, header->heap_index);
+			isc_heap_increased(HEADERNODE(header)->heap->heap,
+					   header->heap_index);
 		} else if (resign_sooner(&oldheader, header)) {
-			isc_heap_decreased(qpdb->heap, header->heap_index);
+			isc_heap_decreased(HEADERNODE(header)->heap->heap,
+					   header->heap_index);
 		}
-		RWUNLOCK(&qpdb->lock, isc_rwlocktype_write);
+		UNLOCK(get_heap_lock(header));
 	} else if (resign != 0) {
 		DNS_SLABHEADER_SETATTR(header, DNS_SLABHEADERATTR_RESIGN);
-		resigninsert(qpdb, header);
+		resigninsert(header);
 	}
 	NODE_UNLOCK(nlock, &nlocktype);
 	return ISC_R_SUCCESS;
@@ -2420,7 +2488,6 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname,
 	dns_slabheader_t *header = NULL;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
 	isc_rwlock_t *nlock = NULL;
-	uint16_t locknum;
 	isc_result_t result = ISC_R_NOTFOUND;
 
 	REQUIRE(VALID_QPZONE(qpdb));
@@ -2428,26 +2495,28 @@ getsigningtime(dns_db_t *db, isc_stdtime_t *resign, dns_name_t *foundname,
 	REQUIRE(foundname != NULL);
 	REQUIRE(typepair != NULL);
 
-	RWLOCK(&qpdb->lock, isc_rwlocktype_read);
+	LOCK(&qpdb->heap->lock);
-	header = isc_heap_element(qpdb->heap, 1);
+	header = isc_heap_element(qpdb->heap->heap, 1);
 	if (header == NULL) {
-		RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
+		UNLOCK(&qpdb->heap->lock);
 		return ISC_R_NOTFOUND;
 	}
-	locknum = HEADERNODE(header)->locknum;
+	nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
-	RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
+	UNLOCK(&qpdb->heap->lock);
 
 again:
-	nlock = &qpdb->buckets[locknum].lock;
-
 	NODE_RDLOCK(nlock, &nlocktype);
 
-	RWLOCK(&qpdb->lock, isc_rwlocktype_read);
+	LOCK(&qpdb->heap->lock);
-	header = isc_heap_element(qpdb->heap, 1);
+	header = isc_heap_element(qpdb->heap->heap, 1);
-	if (header != NULL && HEADERNODE(header)->locknum != locknum) {
+
-		RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
+	if (header != NULL &&
+	    qpzone_get_lock(qpdb, HEADERNODE(header)) != nlock)
+	{
+		UNLOCK(&qpdb->heap->lock);
 		NODE_UNLOCK(nlock, &nlocktype);
-		locknum = HEADERNODE(header)->locknum;
+
+		nlock = qpzone_get_lock(qpdb, HEADERNODE(header));
 		goto again;
 	}
 
@@ -2459,7 +2528,7 @@ again:
 		*typepair = header->type;
 		result = ISC_R_SUCCESS;
 	}
-	RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
+	UNLOCK(&qpdb->heap->lock);
 	NODE_UNLOCK(nlock, &nlocktype);
 
 	return result;
@@ -2632,8 +2701,7 @@ qpzone_setup_delegation(qpz_search_t *search, dns_dbnode_t **nodep,
 	}
 	if (rdataset != NULL) {
 		isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-		isc_rwlock_t *nlock =
+		isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
-			&search->qpdb->buckets[node->locknum].lock;
 		NODE_RDLOCK(nlock, &nlocktype);
 		bindrdataset(search->qpdb, node, search->zonecut_header,
 			     rdataset DNS__DB_FLARG_PASS);
@@ -2673,7 +2741,7 @@ step(qpz_search_t *search, dns_qpiter_t *it, direction_t direction,
 
 	result = dns_qpiter_current(it, nodename, (void **)&node, NULL);
 	while (result == ISC_R_SUCCESS) {
-		isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
+		isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
 		isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
 		dns_slabheader_t *header_next = NULL;
 
@@ -2838,7 +2906,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep,
 
 		dns_qpchain_node(&search->chain, i, NULL, (void **)&node, NULL);
 
-		nlock = &qpdb->buckets[node->locknum].lock;
+		nlock = qpzone_get_lock(qpdb, node);
 		NODE_RDLOCK(nlock, &nlocktype);
 		/*
 		 * First we try to figure out if this node is active in
@@ -2882,7 +2950,7 @@ find_wildcard(qpz_search_t *search, qpznode_t **nodep,
 				 * is active in the search's version, we're
 				 * done.
 				 */
-				nlock = &qpdb->buckets[wnode->locknum].lock;
+				nlock = qpzone_get_lock(qpdb, wnode);
 				NODE_RDLOCK(nlock, &nlocktype);
 				for (header = wnode->data; header != NULL;
 				     header = header->next)
@@ -3065,8 +3133,7 @@ again:
 	do {
 		dns_slabheader_t *found = NULL, *foundsig = NULL;
 		isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-		isc_rwlock_t *nlock =
+		isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
-			&search->qpdb->buckets[node->locknum].lock;
 		NODE_RDLOCK(nlock, &nlocktype);
 		empty_node = true;
 		for (header = node->data; header != NULL; header = header_next)
@@ -3209,7 +3276,7 @@ qpzone_check_zonecut(qpznode_t *node, void *arg DNS__DB_FLARG) {
 	dns_slabheader_t *found = NULL;
 	isc_result_t result = DNS_R_CONTINUE;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-	isc_rwlock_t *nlock = &search->qpdb->buckets[node->locknum].lock;
+	isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node);
 
 	NODE_RDLOCK(nlock, &nlocktype);
 
@@ -3498,7 +3565,7 @@ found:
 	 * have matched a wildcard.
 	 */
 
-	nlock = &search.qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(search.qpdb, node);
 	NODE_RDLOCK(nlock, &nlocktype);
 
 	if (search.zonecut != NULL) {
@@ -3860,7 +3927,7 @@ tree_exit:
 	if (search.need_cleanup) {
 		node = search.zonecut;
 		INSIST(node != NULL);
-		nlock = &search.qpdb->buckets[node->locknum].lock;
+		nlock = qpzone_get_lock(search.qpdb, node);
 
 		NODE_RDLOCK(nlock, &nlocktype);
 		qpznode_release(search.qpdb, node, 0,
@@ -3936,7 +4003,7 @@ qpzone_detachnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) {
 
 	node = (qpznode_t *)(*nodep);
 	*nodep = NULL;
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 
 	/*
 	 * qpzone_destroy() uses call_rcu() API to destroy the node locks, so it
@@ -4020,7 +4087,7 @@ locknode(dns_db_t *db, dns_dbnode_t *dbnode, isc_rwlocktype_t type) {
 	qpzonedb_t *qpdb = (qpzonedb_t *)db;
 	qpznode_t *node = (qpznode_t *)dbnode;
 
-	RWLOCK(&qpdb->buckets[node->locknum].lock, type);
+	RWLOCK(qpzone_get_lock(qpdb, node), type);
 }
 
 static void
@@ -4028,19 +4095,19 @@ unlocknode(dns_db_t *db, dns_dbnode_t *dbnode, isc_rwlocktype_t type) {
 	qpzonedb_t *qpdb = (qpzonedb_t *)db;
 	qpznode_t *node = (qpznode_t *)dbnode;
 
-	RWUNLOCK(&qpdb->buckets[node->locknum].lock, type);
+	RWUNLOCK(qpzone_get_lock(qpdb, node), type);
 }
 
 static void
 deletedata(dns_db_t *db ISC_ATTR_UNUSED, dns_dbnode_t *node ISC_ATTR_UNUSED,
 	   void *data) {
-	qpzonedb_t *qpdb = (qpzonedb_t *)db;
 	dns_slabheader_t *header = data;
 
-	if (header->heap != NULL && header->heap_index != 0) {
+	if (header->heap_index != 0) {
-		RWLOCK(&qpdb->lock, isc_rwlocktype_write);
+		LOCK(get_heap_lock(header));
-		isc_heap_delete(header->heap, header->heap_index);
+		isc_heap_delete(HEADERNODE(header)->heap->heap,
-		RWUNLOCK(&qpdb->lock, isc_rwlocktype_write);
+				header->heap_index);
+		UNLOCK(get_heap_lock(header));
 	}
 	header->heap_index = 0;
 }
@@ -4074,7 +4141,7 @@ rdatasetiter_first(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
 	qpz_version_t *version = (qpz_version_t *)qrditer->common.version;
 	dns_slabheader_t *header = NULL, *top_next = NULL;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-	isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
+	isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
 
 	NODE_RDLOCK(nlock, &nlocktype);
 
@@ -4117,7 +4184,7 @@ rdatasetiter_next(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
 	dns_slabheader_t *header = NULL;
 	dns_slabheader_t *topheader, *topheader_next = NULL;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-	isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
+	isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
 
 	header = qrditer->current;
 	if (header == NULL) {
@@ -4175,7 +4242,7 @@ rdatasetiter_current(dns_rdatasetiter_t *iterator,
 	qpznode_t *node = (qpznode_t *)qrditer->common.node;
 	dns_slabheader_t *header = NULL;
 	isc_rwlocktype_t nlocktype = isc_rwlocktype_none;
-	isc_rwlock_t *nlock = &qpdb->buckets[node->locknum].lock;
+	isc_rwlock_t *nlock = qpzone_get_lock(qpdb, node);
 
 	header = qrditer->current;
 	REQUIRE(header != NULL);
@@ -4214,7 +4281,7 @@ dereference_iter_node(qpdb_dbiterator_t *iter DNS__DB_FLARG) {
 	}
 
 	iter->node = NULL;
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 
 	NODE_RDLOCK(nlock, &nlocktype);
 	qpznode_release(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
@@ -4710,7 +4777,7 @@ qpzone_addrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 	 * (Note: node lock must be acquired after starting
 	 * the QPDB transaction and released before committing.)
 	 */
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 
 	NODE_WRLOCK(nlock, &nlocktype);
 
@@ -4804,7 +4871,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 		newheader->resign_lsb = rdataset->resign & 0x1;
 	}
 
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 	NODE_WRLOCK(nlock, &nlocktype);
 
 	changed = add_changed(newheader, version DNS__DB_FLARG_PASS);
@@ -4853,7 +4920,7 @@ qpzone_subtractrdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 					newheader, DNS_SLABHEADERATTR_RESIGN);
 				newheader->resign = header->resign;
 				newheader->resign_lsb = header->resign_lsb;
-				resigninsert(qpdb, newheader);
+				resigninsert(newheader);
 			}
 			/*
 			 * We have to set the serial since the rdataslab
@@ -4966,7 +5033,7 @@ qpzone_deleterdataset(dns_db_t *db, dns_dbnode_t *dbnode,
 
 	dns_name_copy(&node->name, nodename);
 
-	nlock = &qpdb->buckets[node->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, node);
 	NODE_WRLOCK(nlock, &nlocktype);
 	result = add(qpdb, node, nodename, version, newheader, DNS_DBADD_FORCE,
 		     false, NULL, 0 DNS__DB_FLARG_PASS);
@@ -4985,7 +5052,7 @@ nodefullname(dns_db_t *db, dns_dbnode_t *node, dns_name_t *name) {
 	REQUIRE(node != NULL);
 	REQUIRE(name != NULL);
 
-	nlock = &qpdb->buckets[qpnode->locknum].lock;
+	nlock = qpzone_get_lock(qpdb, qpnode);
 
 	NODE_RDLOCK(nlock, &nlocktype);
 	dns_name_copy(&qpnode->name, name);
@@ -5389,6 +5456,7 @@ destroy_qpznode(qpznode_t *node) {
 		dns_slabheader_destroy(&current);
 	}
 
+	qpz_heap_unref(node->heap);
 	dns_name_free(&node->name, node->mctx);
 	isc_mem_putanddetach(&node->mctx, node, sizeof(qpznode_t));
 }
@@ -5405,6 +5473,8 @@ ISC_REFCOUNT_STATIC_TRACE_IMPL(qpzonedb, qpzone_destroy);
 ISC_REFCOUNT_STATIC_IMPL(qpzonedb, qpzone_destroy);
 #endif
 
+ISC_REFCOUNT_STATIC_IMPL(qpz_heap, qpz_heap_destroy);
+
 static void
 qp_attach(void *uctx ISC_ATTR_UNUSED, void *pval,
 	  uint32_t ival ISC_ATTR_UNUSED) {

tests/dns/qpzone_test.c

@@ -39,6 +39,7 @@
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wshadow"
 #undef CHECK
+#define DEFAULT_BUCKETS_COUNT 1
 #include "qpzone.c"
 #pragma GCC diagnostic pop
 
@@ -108,7 +109,6 @@ ownercase_test_one(const char *str1, const char *str2) {
 	*qpdb = (qpzonedb_t){
 		.common.methods = &qpdb_zonemethods,
 		.common.mctx = mctx,
-		.buckets_count = 1,
 	};
 	qpznode_t node = { .locknum = 0 };
 	dns_slabheader_t header = {
@@ -176,7 +176,6 @@ ISC_RUN_TEST_IMPL(setownercase) {
 	*qpdb = (qpzonedb_t){
 		.common.methods = &qpdb_zonemethods,
 		.common.mctx = mctx,
-		.buckets_count = 1,
 	};
 	qpznode_t node = { .locknum = 0 };
 	dns_slabheader_t header = {