diff --git a/lib/dns/qpzone.c b/lib/dns/qpzone.c index 9aed340983..51f30ddd39 100644 --- a/lib/dns/qpzone.c +++ b/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[]; /* attribute((counted_by(buckets_count))) */ + qpzone_bucket_t buckets[DEFAULT_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, - sizeof(*qpdb) + qpdb->buckets_count * - sizeof(qpdb->buckets[0])); + isc_mem_putanddetach(&qpdb->common.mctx, qpdb, sizeof(*qpdb)); } 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, - sizeof(*qpdb) + DEFAULT_BUCKETS_COUNT * - sizeof(qpdb->buckets[0])); + qpdb = isc_mem_get(mctx, sizeof(*qpdb)); *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); - isc_heap_insert(qpdb->heap, newheader); - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); - - newheader->heap = qpdb->heap; + LOCK(get_heap_lock(newheader)); + isc_heap_insert(HEADERNODE(newheader)->heap->heap, newheader); + UNLOCK(get_heap_lock(newheader)); } static void @@ -1261,9 +1327,9 @@ resigndelete(qpzonedb_t *qpdb, qpz_version_t *version, return; } - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - isc_heap_delete(qpdb->heap, header->heap_index); - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); + LOCK(get_heap_lock(header)); + isc_heap_delete(HEADERNODE(header)->heap->heap, header->heap_index); + 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); - header = isc_heap_element(qpdb->heap, 1); + LOCK(&qpdb->heap->lock); + 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; - RWUNLOCK(&qpdb->lock, isc_rwlocktype_read); + nlock = qpzone_get_lock(qpdb, HEADERNODE(header)); + UNLOCK(&qpdb->heap->lock); again: - nlock = &qpdb->buckets[locknum].lock; - NODE_RDLOCK(nlock, &nlocktype); - RWLOCK(&qpdb->lock, isc_rwlocktype_read); - header = isc_heap_element(qpdb->heap, 1); - if (header != NULL && HEADERNODE(header)->locknum != locknum) { - RWUNLOCK(&qpdb->lock, isc_rwlocktype_read); + LOCK(&qpdb->heap->lock); + header = isc_heap_element(qpdb->heap->heap, 1); + + 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 = - &search->qpdb->buckets[node->locknum].lock; + isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node); 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 = - &search->qpdb->buckets[node->locknum].lock; + isc_rwlock_t *nlock = qpzone_get_lock(search->qpdb, node); 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) { - RWLOCK(&qpdb->lock, isc_rwlocktype_write); - isc_heap_delete(header->heap, header->heap_index); - RWUNLOCK(&qpdb->lock, isc_rwlocktype_write); + if (header->heap_index != 0) { + LOCK(get_heap_lock(header)); + isc_heap_delete(HEADERNODE(header)->heap->heap, + 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(¤t); } + 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) { diff --git a/tests/dns/qpzone_test.c b/tests/dns/qpzone_test.c index 7e716bf024..c464a9e2c1 100644 --- a/tests/dns/qpzone_test.c +++ b/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 = {