mir/bind
2
0
Fork 0
mirror of https://gitlab.isc.org/isc-projects/bind9 synced 2025-08-31 14:35:26 +00:00
Code Issues Releases Wiki Activity

improve node reference counting

Browse Source 
QP database node data is not reference counted the same way RBT nodes
were: in the RBT, node->references could be zero if the node was in the
tree but was not in use by any caller, whereas in the QP trie, the
database itself uses reference counting of nodes internally.

this caused some subtle errors. in RBTDB, when the newref() function is
called and the node reference count was zero, the node lock reference
counter would also be incremented. in the QP trie, this can never
happen - because as long as the node is in the database its reference
count cannot be zero - and so the node lock reference counter was never
incremented.

this has been addressed by maintaining a separate "erefs" counter for
external references to the node. this is the same approach used in the
"qpdb-lite" database in commit e91fbd8dea.

while troubleshooting this issue, some compile errors were discovered
when building with DNS_DB_NODETRACE; those have also been fixed.
This commit is contained in:
Evan Hunt
2023-12-02 01:04:28 -08:00
parent 2b4133a32c
commit 55f38e34dc
2 changed files with 77 additions and 104 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
lib/dns/dnsrps.c
View File

@@ -370,7 +370,8 @@ rpsdb_destroy(dns_db_t *db) {
}
static void
rpsdb_attachnode(dns_db_t *db, dns_dbnode_t *source, dns_dbnode_t **targetp) {
rpsdb_attachnode(dns_db_t *db, dns_dbnode_t *source,
dns_dbnode_t **targetp DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
REQUIRE(VALID_RPSDB(rpsdb));
@@ -382,7 +383,7 @@ rpsdb_attachnode(dns_db_t *db, dns_dbnode_t *source, dns_dbnode_t **targetp) {
}
static void
rpsdb_detachnode(dns_db_t *db, dns_dbnode_t **targetp) {
rpsdb_detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
REQUIRE(VALID_RPSDB(rpsdb));
@@ -395,7 +396,7 @@ rpsdb_detachnode(dns_db_t *db, dns_dbnode_t **targetp) {
static isc_result_t
rpsdb_findnode(dns_db_t *db, const dns_name_t *name, bool create,
dns_dbnode_t **nodep) {
dns_dbnode_t **nodep DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
dns_db_t *dbp = NULL;
@@ -465,7 +466,7 @@ static isc_result_t
rpsdb_findrdataset(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
dns_rdatatype_t type, dns_rdatatype_t covers,
isc_stdtime_t now, dns_rdataset_t *rdataset,
dns_rdataset_t *sigrdataset) {
dns_rdataset_t *sigrdataset DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
dns_rdatatype_t foundtype;
dns_rdataclass_t class;
@@ -575,7 +576,8 @@ static isc_result_t
rpsdb_finddb(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
dns_rdatatype_t type, unsigned int options, isc_stdtime_t now,
dns_dbnode_t **nodep, dns_name_t *foundname,
dns_rdataset_t *rdataset, dns_rdataset_t *sigrdataset) {
dns_rdataset_t *rdataset,
dns_rdataset_t *sigrdataset DNS__DB_FLARG) {
dns_dbnode_t *node = NULL;
UNUSED(version);
@@ -587,16 +589,16 @@ rpsdb_finddb(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
node = NULL;
nodep = &node;
}
rpsdb_findnode(db, name, false, nodep);
rpsdb_findnode(db, name, false, nodep DNS__DB_FLARG_PASS);
dns_name_copy(name, foundname);
return (rpsdb_findrdataset(db, *nodep, NULL, type, 0, 0, rdataset,
sigrdataset));
sigrdataset DNS__DB_FLARG_PASS));
}
static isc_result_t
rpsdb_allrdatasets(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
unsigned int options, isc_stdtime_t now,
dns_rdatasetiter_t **iteratorp) {
dns_rdatasetiter_t **iteratorp DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
rpsdb_rdatasetiter_t *rpsdb_iter = NULL;
@@ -616,7 +618,7 @@ rpsdb_allrdatasets(dns_db_t *db, dns_dbnode_t *node, dns_dbversion_t *version,
},
};
rpsdb_attachnode(db, node, &rpsdb_iter->common.node);
rpsdb_attachnode(db, node, &rpsdb_iter->common.node DNS__DB_FLARG_PASS);
*iteratorp = &rpsdb_iter->common;
@@ -631,18 +633,18 @@ rpsdb_issecure(dns_db_t *db) {
}
static isc_result_t
rpsdb_getoriginnode(dns_db_t *db, dns_dbnode_t **nodep) {
rpsdb_getoriginnode(dns_db_t *db, dns_dbnode_t **nodep DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = (dns_rpsdb_t *)db;
REQUIRE(VALID_RPSDB(rpsdb));
REQUIRE(nodep != NULL && *nodep == NULL);
rpsdb_attachnode(db, &rpsdb->origin_node, nodep);
rpsdb_attachnode(db, &rpsdb->origin_node, nodep DNS__DB_FLARG_PASS);
return (ISC_R_SUCCESS);
}
static void
rpsdb_rdataset_disassociate(dns_rdataset_t *rdataset) {
rpsdb_rdataset_disassociate(dns_rdataset_t *rdataset DNS__DB_FLARG) {
dns_db_t *db = NULL;
/*
@@ -759,7 +761,8 @@ rpsdb_rdataset_current(dns_rdataset_t *rdataset, dns_rdata_t *rdata) {
}
static void
rpsdb_rdataset_clone(dns_rdataset_t *source, dns_rdataset_t *target) {
rpsdb_rdataset_clone(dns_rdataset_t *source,
dns_rdataset_t *target DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = NULL;
dns_db_t *dbp = NULL;
@@ -786,7 +789,7 @@ rpsdb_rdataset_count(dns_rdataset_t *rdataset) {
}
static void
rpsdb_rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp) {
rpsdb_rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = NULL;
dns_rdatasetiter_t *iterator = NULL;
isc_mem_t *mctx = NULL;
@@ -802,7 +805,7 @@ rpsdb_rdatasetiter_destroy(dns_rdatasetiter_t **iteratorp) {
}
static isc_result_t
rpsdb_rdatasetiter_next(dns_rdatasetiter_t *iter) {
rpsdb_rdatasetiter_next(dns_rdatasetiter_t *iter DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = NULL;
rpsdb_rdatasetiter_t *rpsdb_iter = NULL;
dns_rdatatype_t next_type, type;
@@ -872,7 +875,7 @@ rpsdb_rdatasetiter_next(dns_rdatasetiter_t *iter) {
}
static isc_result_t
rpsdb_rdatasetiter_first(dns_rdatasetiter_t *iterator) {
rpsdb_rdatasetiter_first(dns_rdatasetiter_t *iterator DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = NULL;
rpsdb_rdatasetiter_t *rpsdb_iter = NULL;
@@ -882,12 +885,12 @@ rpsdb_rdatasetiter_first(dns_rdatasetiter_t *iterator) {
rpsdb_iter->type = dns_rdatatype_none;
rpsdb_iter->class = dns_rdataclass_reserved0;
return (rpsdb_rdatasetiter_next(iterator));
return (rpsdb_rdatasetiter_next(iterator DNS__DB_FLARG_PASS));
}
static void
rpsdb_rdatasetiter_current(dns_rdatasetiter_t *iterator,
dns_rdataset_t *rdataset) {
dns_rdataset_t *rdataset DNS__DB_FLARG) {
dns_rpsdb_t *rpsdb = NULL;
rpsdb_rdatasetiter_t *rpsdb_iter = NULL;

142
lib/dns/qpzone.c
View File

@@ -154,6 +154,7 @@ struct qpdata {
dns_name_t *name;
isc_mem_t *mctx;
isc_refcount_t references;
isc_refcount_t erefs;
uint16_t locknum;
void *data;
unsigned int : 0;
@@ -779,9 +780,10 @@ static void
newref(qpzonedb_t *qpdb, qpdata_t *node DNS__DB_FLARG) {
uint_fast32_t refs;
refs = isc_refcount_increment0(&node->references);
qpdata_ref(node);
refs = isc_refcount_increment0(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "incr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
fprintf(stderr, "incr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs + 1);
#else
UNUSED(refs);
@@ -928,13 +930,14 @@ clean_zone_node(qpdata_t *node, uint32_t least_serial) {
* threads are decreasing the reference to zero simultaneously and at least
* one of them is going to free the node.
*
* This function returns true if and only if the node reference decreases
* to zero.
* This decrements both the internal and external node reference counters.
* If the external reference count drops to zero, then the node lock
* reference count is also decremented.
*
* NOTE: Decrementing the reference count of a node to zero does not mean it
* will be immediately freed.
* (NOTE: Decrementing the reference count of a node to zero does
* not mean it will be immediately freed.)
*/
static bool
static void
decref(qpzonedb_t *qpdb, qpdata_t *node, uint32_t least_serial,
isc_rwlocktype_t *nlocktypep DNS__DB_FLARG) {
db_nodelock_t *nodelock = NULL;
@@ -945,15 +948,14 @@ decref(qpzonedb_t *qpdb, qpdata_t *node, uint32_t least_serial,
nodelock = &qpdb->node_locks[bucket];
#define KEEP_NODE(n, r) \
((n)->data != NULL || (n) == (r)->origin || (n) == (r)->nsec3_origin)
/* Handle easy and typical case first. */
if (!node->dirty && KEEP_NODE(node, qpdb)) {
refs = isc_refcount_decrement(&node->references);
if (!node->dirty && (node->data != NULL || node == qpdb->origin ||
node == qpdb->nsec3_origin))
{
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
"decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#else
UNUSED(refs);
@@ -968,10 +970,8 @@ decref(qpzonedb_t *qpdb, qpdata_t *node, uint32_t least_serial,
#else
UNUSED(refs);
#endif
return (true);
}
return (false);
goto done;
}
/* Upgrade the lock? */
@@ -979,45 +979,38 @@ decref(qpzonedb_t *qpdb, qpdata_t *node, uint32_t least_serial,
NODE_FORCEUPGRADE(&nodelock->lock, nlocktypep);
}
refs = isc_refcount_decrement(&node->references);
refs = isc_refcount_decrement(&node->erefs);
#if DNS_DB_NODETRACE
fprintf(stderr, "decr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
fprintf(stderr, "decr:node:%s:%s:%u:%p->erefs = %" PRIuFAST32 "\n",
func, file, line, node, refs - 1);
#else
UNUSED(refs);
#endif
if (refs > 1) {
return (false);
}
if (node->dirty) {
if (least_serial == 0) {
/*
* Caller doesn't know the least serial.
* Get it.
*/
RWLOCK(&qpdb->lock, isc_rwlocktype_read);
least_serial = qpdb->least_serial;
RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
if (refs == 1) {
if (node->dirty) {
if (least_serial == 0) {
/*
* Caller doesn't know the least serial.
* Get it.
*/
RWLOCK(&qpdb->lock, isc_rwlocktype_read);
least_serial = qpdb->least_serial;
RWUNLOCK(&qpdb->lock, isc_rwlocktype_read);
}
clean_zone_node(node, least_serial);
}
clean_zone_node(node, least_serial);
}
refs = isc_refcount_decrement(&nodelock->references);
refs = isc_refcount_decrement(&nodelock->references);
#if DNS_DB_NODETRACE
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = %" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
fprintf(stderr,
"decr:nodelock:%s:%s:%u:%p:%p->references = "
"%" PRIuFAST32 "\n",
func, file, line, node, nodelock, refs - 1);
#else
UNUSED(refs);
UNUSED(refs);
#endif
if (KEEP_NODE(node, qpdb)) {
return (true);
}
#undef KEEP_NODE
return (true);
done:
qpdata_unref(node);
}
static void
@@ -1827,21 +1820,16 @@ static qpdb_changed_t *
add_changed(dns_slabheader_t *header, qpdb_version_t *version DNS__DB_FLARG) {
qpdb_changed_t *changed = NULL;
qpzonedb_t *qpdb = (qpzonedb_t *)header->db;
qpdata_t *node = (qpdata_t *)header->node;
changed = isc_mem_get(qpdb->common.mctx, sizeof(*changed));
RWLOCK(&qpdb->lock, isc_rwlocktype_write);
REQUIRE(version->writer);
qpdata_t *node = (qpdata_t *)header->node;
uint_fast32_t refs = isc_refcount_increment(&node->references);
#if DNS_DB_NODETRACE
fprintf(stderr, "incr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
func, file, line, node, refs + 1);
#else
UNUSED(refs);
#endif
*changed = (qpdb_changed_t){ .node = node };
ISC_LIST_INITANDAPPEND(version->changed_list, changed, link);
newref(qpdb, node DNS__DB_FLARG_PASS);
RWUNLOCK(&qpdb->lock, isc_rwlocktype_write);
return (changed);
@@ -3458,7 +3446,7 @@ find(dns_db_t *db, const dns_name_t *name, dns_dbversion_t *version,
isc_result_t tresult;
dns_qpchain_node(&search.chain, i, NULL, (void **)&n, NULL);
tresult = check_zonecut(n, &search);
tresult = check_zonecut(n, &search DNS__DB_FLARG_PASS);
if (tresult != DNS_R_CONTINUE) {
result = tresult;
search.chain.len = i - 1;
@@ -3531,6 +3519,9 @@ found:
* have matched a wildcard.
*/
lock = &search.qpdb->node_locks[node->locknum].lock;
NODE_RDLOCK(lock, &nlocktype);
if (search.zonecut != NULL) {
/*
* If we're beneath a zone cut, we don't want to look for
@@ -3571,15 +3562,7 @@ found:
* We now go looking for rdata...
*/
lock = &search.qpdb->node_locks[node->locknum].lock;
NODE_RDLOCK(lock, &nlocktype);
found = NULL;
foundsig = NULL;
sigtype = DNS_SIGTYPE(type);
nsecheader = NULL;
nsecsig = NULL;
cnamesig = NULL;
empty_node = true;
for (header = node->data; header != NULL; header = header_next) {
header_next = header->next;
@@ -3911,7 +3894,6 @@ allrdatasets(dns_db_t *db, dns_dbnode_t *dbnode, dns_dbversion_t *dbversion,
qpdata_t *node = (qpdata_t *)dbnode;
qpdb_version_t *version = dbversion;
qpdb_rdatasetiter_t *iterator = NULL;
uint_fast32_t refs;
REQUIRE(VALID_QPZONE(qpdb));
@@ -3942,13 +3924,7 @@ allrdatasets(dns_db_t *db, dns_dbnode_t *dbnode, dns_dbversion_t *dbversion,
iterator->common.options = options;
iterator->common.now = now;
refs = isc_refcount_increment(&node->references);
#if DNS_DB_NODETRACE
fprintf(stderr, "incr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
func, file, line, node, refs + 1);
#else
UNUSED(refs);
#endif
newref(qpdb, node DNS__DB_FLARG_PASS);
iterator->current = NULL;
@@ -3960,18 +3936,13 @@ allrdatasets(dns_db_t *db, dns_dbnode_t *dbnode, dns_dbversion_t *dbversion,
static void
attachnode(dns_db_t *db, dns_dbnode_t *source,
dns_dbnode_t **targetp DNS__DB_FLARG) {
REQUIRE(VALID_QPZONE((qpzonedb_t *)db));
qpzonedb_t *qpdb = (qpzonedb_t *)db;
qpdata_t *node = (qpdata_t *)source;
REQUIRE(VALID_QPZONE(qpdb));
REQUIRE(targetp != NULL && *targetp == NULL);
qpdata_t *node = (qpdata_t *)source;
uint_fast32_t refs = isc_refcount_increment(&node->references);
#if DNS_DB_NODETRACE
fprintf(stderr, "incr:node:%s:%s:%u:%p->references = %" PRIuFAST32 "\n",
func, file, line, node, refs + 1);
#else
UNUSED(refs);
#endif
newref(qpdb, node DNS__DB_FLARG_PASS);
*targetp = source;
}
@@ -3992,12 +3963,11 @@ detachnode(dns_db_t *db, dns_dbnode_t **targetp DNS__DB_FLARG) {
nodelock = &qpdb->node_locks[node->locknum];
NODE_RDLOCK(&nodelock->lock, &nlocktype);
if (decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS)) {
if (isc_refcount_current(&nodelock->references) == 0 &&
nodelock->exiting)
{
inactive = true;
}
decref(qpdb, node, 0, &nlocktype DNS__DB_FLARG_PASS);
if (isc_refcount_current(&nodelock->references) == 0 &&
nodelock->exiting)
{
inactive = true;
}
NODE_UNLOCK(&nodelock->lock, &nlocktype);