From 361c8868b45fa1ff39125a32d516a22f5d3dba54 Mon Sep 17 00:00:00 2001 From: Evan Hunt Date: Mon, 3 Apr 2023 15:48:44 -0700 Subject: [PATCH] use ISC_REFCOUNT_IMPL for external dns_zone references use the ISC_REFCOUNT implementation for dns_zone_attach() and _detach(). (this applies only to external zone references, not to dns_zone_iattach() and dns_zone_idetach().) use dns_zone_ref() where previously a dummy zone object had been used to increment the reference count. --- lib/dns/include/dns/zone.h | 37 ++++++++----------- lib/dns/zone.c | 75 ++++++++++++++++---------------------- lib/dns/zt.c | 7 ++-- 3 files changed, 50 insertions(+), 69 deletions(-) diff --git a/lib/dns/include/dns/zone.h b/lib/dns/include/dns/zone.h index 0394718c23..0c71cfceaa 100644 --- a/lib/dns/include/dns/zone.h +++ b/lib/dns/include/dns/zone.h @@ -37,6 +37,9 @@ #include #include +/* Define to 1 for detailed reference tracing */ +#undef DNS_ZONE_TRACE + typedef enum { dns_zone_none, dns_zone_primary, @@ -449,28 +452,6 @@ dns__zone_loadpending(dns_zone_t *zone); * tests.) */ -void -dns_zone_attach(dns_zone_t *source, dns_zone_t **target); -/*%< - * Attach '*target' to 'source' incrementing its external - * reference count. - * - * Require: - *\li 'zone' to be a valid zone. - *\li 'target' to be non NULL and '*target' to be NULL. - */ - -void -dns_zone_detach(dns_zone_t **zonep); -/*%< - * Detach from a zone decrementing its external reference count. - * If this was the last external reference to the zone it will be - * shut down and eventually freed. - * - * Require: - *\li 'zonep' to point to a valid zone. - */ - void dns_zone_iattach(dns_zone_t *source, dns_zone_t **target); /*%< @@ -2584,3 +2565,15 @@ dns_zone_check_dnskey_nsec3(dns_zone_t *zone, dns_db_t *db, * \li 'true' if the check passes, that is the zone remains consistent, * 'false' if the zone would have NSEC only DNSKEYs and an NSEC3 chain. */ + +#if DNS_ZONE_TRACE +#define dns_zone_ref(ptr) dns_zone__ref(ptr, __func__, __FILE__, __LINE__) +#define dns_zone_unref(ptr) dns_zone__unref(ptr, __func__, __FILE__, __LINE__) +#define dns_zone_attach(ptr, ptrp) \ + dns_zone__attach(ptr, ptrp, __func__, __FILE__, __LINE__) +#define dns_zone_detach(ptrp) \ + dns_zone__detach(ptrp, __func__, __FILE__, __LINE__) +ISC_REFCOUNT_TRACE_DECL(dns_zone); +#else +ISC_REFCOUNT_DECL(dns_zone); +#endif diff --git a/lib/dns/zone.c b/lib/dns/zone.c index acd2044834..2e31a82963 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -252,7 +252,7 @@ struct dns_zone { bool locked; #endif /* ifdef DNS_ZONE_CHECKLOCK */ isc_mem_t *mctx; - isc_refcount_t erefs; + isc_refcount_t references; isc_rwlock_t dblock; dns_db_t *db; /* Locked by dblock */ @@ -1124,7 +1124,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid) { isc_mutex_init(&zone->lock); ZONEDB_INITLOCK(&zone->dblock); - isc_refcount_init(&zone->erefs, 1); + isc_refcount_init(&zone->references, 1); isc_refcount_init(&zone->irefs, 0); dns_name_init(&zone->origin, NULL); isc_sockaddr_any(&zone->notifysrc4); @@ -1154,8 +1154,8 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx, unsigned int tid) { return (ISC_R_SUCCESS); free_refs: - isc_refcount_decrement0(&zone->erefs); - isc_refcount_destroy(&zone->erefs); + isc_refcount_decrement0(&zone->references); + isc_refcount_destroy(&zone->references); isc_refcount_destroy(&zone->irefs); ZONEDB_DESTROYLOCK(&zone->dblock); isc_mutex_destroy(&zone->lock); @@ -1188,7 +1188,7 @@ zone_free(dns_zone_t *zone) { REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); - isc_refcount_destroy(&zone->erefs); + isc_refcount_destroy(&zone->references); isc_refcount_destroy(&zone->irefs); /* @@ -5326,9 +5326,9 @@ exit_check(dns_zone_t *zone) { isc_refcount_current(&zone->irefs) == 0) { /* - * DNS_ZONEFLG_SHUTDOWN can only be set if erefs == 0. + * DNS_ZONEFLG_SHUTDOWN can only be set if references == 0. */ - INSIST(isc_refcount_current(&zone->erefs) == 0); + INSIST(isc_refcount_current(&zone->references) == 0); return (true); } return (false); @@ -5602,40 +5602,17 @@ closeversion: return (answer); } -void -dns_zone_attach(dns_zone_t *source, dns_zone_t **target) { - REQUIRE(DNS_ZONE_VALID(source)); - REQUIRE(target != NULL && *target == NULL); - isc_refcount_increment(&source->erefs); - *target = source; -} +static void +zone_destroy(dns_zone_t *zone) { + isc_refcount_destroy(&zone->references); -void -dns_zone_detach(dns_zone_t **zonep) { - REQUIRE(zonep != NULL && DNS_ZONE_VALID(*zonep)); - - dns_zone_t *zone = *zonep; - *zonep = NULL; - - if (isc_refcount_decrement(&zone->erefs) == 1) { - isc_refcount_destroy(&zone->erefs); - - /* - * Stop things being restarted after we cancel them below. - */ - DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_EXITING); - dns_zone_log(zone, ISC_LOG_DEBUG(1), - "final reference detached"); - - if (zone->loop != NULL) { - /* - * This zone has a loop; it can clean - * itself up asynchronously. - */ - isc_async_run(zone->loop, zone_shutdown, zone); - return; - } + /* + * Stop things being restarted after we cancel them below. + */ + DNS_ZONE_SETFLAG(zone, DNS_ZONEFLG_EXITING); + dns_zone_log(zone, ISC_LOG_DEBUG(1), "final reference detached"); + if (zone->loop == NULL) { /* * This zone is unmanaged; we're probably running in * named-checkzone or a unit test. There's no loop, so we @@ -5650,9 +5627,21 @@ dns_zone_detach(dns_zone_t **zonep) { INSIST(zone->view == NULL); zone_shutdown(zone); + } else { + /* + * This zone has a loop; it can clean + * itself up asynchronously. + */ + isc_async_run(zone->loop, zone_shutdown, zone); } } +#if DNS_ZONE_TRACE +ISC_REFCOUNT_TRACE_IMPL(dns_zone, zone_destroy); +#else +ISC_REFCOUNT_IMPL(dns_zone, zone_destroy); +#endif + void dns_zone_iattach(dns_zone_t *source, dns_zone_t **target) { REQUIRE(DNS_ZONE_VALID(source)); @@ -5668,7 +5657,7 @@ zone_iattach(dns_zone_t *source, dns_zone_t **target) { REQUIRE(LOCKED_ZONE(source)); REQUIRE(target != NULL && *target == NULL); INSIST(isc_refcount_increment0(&source->irefs) + - isc_refcount_current(&source->erefs) > + isc_refcount_current(&source->references) > 0); *target = source; } @@ -5687,7 +5676,7 @@ zone_idetach(dns_zone_t **zonep) { *zonep = NULL; INSIST(isc_refcount_decrement(&zone->irefs) - 1 + - isc_refcount_current(&zone->erefs) > + isc_refcount_current(&zone->references) > 0); } @@ -14486,7 +14475,7 @@ zone_shutdown(void *arg) { dns_view_t *view = NULL, *prev_view = NULL; REQUIRE(DNS_ZONE_VALID(zone)); - INSIST(isc_refcount_current(&zone->erefs) == 0); + INSIST(isc_refcount_current(&zone->references) == 0); zone_debuglog(zone, __func__, 3, "shutting down"); @@ -21888,7 +21877,7 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) { raw->loop = zone->loop; /* dns_zone_attach(raw, &zone->raw); */ - isc_refcount_increment(&raw->erefs); + isc_refcount_increment(&raw->references); zone->raw = raw; /* dns_zone_iattach(zone, &raw->secure); */ diff --git a/lib/dns/zt.c b/lib/dns/zt.c index 48713d207f..9cfe366ba1 100644 --- a/lib/dns/zt.c +++ b/lib/dns/zt.c @@ -118,8 +118,7 @@ cleanup_zt: isc_result_t dns_zt_mount(dns_zt_t *zt, dns_zone_t *zone) { isc_result_t result; - dns_zone_t *dummy = NULL; - dns_name_t *name; + dns_name_t *name = NULL; REQUIRE(VALID_ZT(zt)); @@ -129,7 +128,7 @@ dns_zt_mount(dns_zt_t *zt, dns_zone_t *zone) { result = dns_rbt_addname(zt->table, name, zone); if (result == ISC_R_SUCCESS) { - dns_zone_attach(zone, &dummy); + dns_zone_ref(zone); } RWUNLOCK(&zt->rwlock, isc_rwlocktype_write); @@ -171,7 +170,7 @@ dns_zt_find(dns_zt_t *zt, const dns_name_t *name, unsigned int options, RWLOCK(&zt->rwlock, isc_rwlocktype_read); result = dns_rbt_findname(zt->table, name, rbtoptions, foundname, - (void **)(void *)&dummy); + (void **)&dummy); if (result == ISC_R_SUCCESS || result == DNS_R_PARTIALMATCH) { /* * If DNS_ZTFIND_MIRROR is set and the zone which was