From e711b0304fda06dca09adf24539a3c47762a9ebf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ond=C5=99ej=20Sur=C3=BD?= Date: Mon, 15 Jul 2019 10:27:11 +0200 Subject: [PATCH] Convert more reference counting to isc_refcount API --- lib/dns/include/dns/peer.h | 5 +- lib/dns/peer.c | 55 +++++++------------ lib/dns/zone.c | 107 ++++++++++++++++--------------------- lib/isc/ratelimiter.c | 50 +++++++---------- 4 files changed, 88 insertions(+), 129 deletions(-) diff --git a/lib/dns/include/dns/peer.h b/lib/dns/include/dns/peer.h index 7a25511226..de648e10e7 100644 --- a/lib/dns/include/dns/peer.h +++ b/lib/dns/include/dns/peer.h @@ -32,6 +32,7 @@ #include #include #include +#include #include @@ -47,7 +48,7 @@ struct dns_peerlist { unsigned int magic; - uint32_t refs; + isc_refcount_t refs; isc_mem_t *mem; @@ -56,7 +57,7 @@ struct dns_peerlist { struct dns_peer { unsigned int magic; - uint32_t refs; + isc_refcount_t refs; isc_mem_t *mem; diff --git a/lib/dns/peer.c b/lib/dns/peer.c index bcd6d71f0e..bf1ef07d99 100644 --- a/lib/dns/peer.c +++ b/lib/dns/peer.c @@ -62,7 +62,7 @@ dns_peerlist_new(isc_mem_t *mem, dns_peerlist_t **list) { ISC_LIST_INIT(l->elements); l->mem = mem; - l->refs = 1; + isc_refcount_init(&l->refs, 1); l->magic = DNS_PEERLIST_MAGIC; *list = l; @@ -76,9 +76,7 @@ dns_peerlist_attach(dns_peerlist_t *source, dns_peerlist_t **target) { REQUIRE(target != NULL); REQUIRE(*target == NULL); - source->refs++; - - ENSURE(source->refs != 0xffffffffU); + isc_refcount_increment(&source->refs); *target = source; } @@ -94,12 +92,9 @@ dns_peerlist_detach(dns_peerlist_t **list) { plist = *list; *list = NULL; - REQUIRE(plist->refs > 0); - - plist->refs--; - - if (plist->refs == 0) + if (isc_refcount_decrement(&plist->refs) == 1) { peerlist_delete(&plist); + } } static void @@ -112,7 +107,7 @@ peerlist_delete(dns_peerlist_t **list) { l = *list; - REQUIRE(l->refs == 0); + isc_refcount_destroy(&l->refs); server = ISC_LIST_HEAD(l->elements); while (server != NULL) { @@ -218,26 +213,19 @@ dns_peer_newprefix(isc_mem_t *mem, const isc_netaddr_t *addr, { dns_peer_t *peer; - REQUIRE(peerptr != NULL); + REQUIRE(peerptr != NULL && *peerptr == NULL); peer = isc_mem_get(mem, sizeof(*peer)); - peer->magic = DNS_PEER_MAGIC; - peer->address = *addr; - peer->prefixlen = prefixlen; - peer->mem = mem; - peer->bogus = false; - peer->transfer_format = dns_one_answer; - peer->transfers = 0; - peer->request_ixfr = false; - peer->provide_ixfr = false; - peer->key = NULL; - peer->refs = 1; - peer->transfer_source = NULL; - peer->notify_source = NULL; - peer->query_source = NULL; - - memset(&peer->bitflags, 0x0, sizeof(peer->bitflags)); + *peer = (dns_peer_t){ + .magic = DNS_PEER_MAGIC, + .address = *addr, + .prefixlen = prefixlen, + .mem = mem, + .transfer_format = dns_one_answer, + }; + + isc_refcount_init(&peer->refs, 1); ISC_LINK_INIT(peer, next); @@ -252,9 +240,7 @@ dns_peer_attach(dns_peer_t *source, dns_peer_t **target) { REQUIRE(target != NULL); REQUIRE(*target == NULL); - source->refs++; - - ENSURE(source->refs != 0xffffffffU); + isc_refcount_increment(&source->refs); *target = source; } @@ -268,14 +254,11 @@ dns_peer_detach(dns_peer_t **peer) { REQUIRE(DNS_PEER_VALID(*peer)); p = *peer; - - REQUIRE(p->refs > 0); - *peer = NULL; - p->refs--; - if (p->refs == 0) + if (isc_refcount_decrement(&p->refs) == 1) { peer_delete(&p); + } } static void @@ -288,7 +271,7 @@ peer_delete(dns_peer_t **peer) { p = *peer; - REQUIRE(p->refs == 0); + isc_refcount_destroy(&p->refs); mem = p->mem; p->mem = NULL; diff --git a/lib/dns/zone.c b/lib/dns/zone.c index aeb58365f6..db2be499db 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -205,7 +205,7 @@ struct dns_zone { dns_zonemgr_t *zmgr; ISC_LINK(dns_zone_t) link; /* Used by zmgr. */ isc_timer_t *timer; - unsigned int irefs; + isc_refcount_t irefs; dns_name_t origin; char *masterfile; ISC_LIST(dns_include_t) includes; /* Include files */ @@ -529,7 +529,7 @@ struct dns_unreachable { struct dns_zonemgr { unsigned int magic; isc_mem_t * mctx; - int refs; /* Locked by rwlock */ + isc_refcount_t refs; isc_taskmgr_t * taskmgr; isc_timermgr_t * timermgr; isc_socketmgr_t * socketmgr; @@ -921,8 +921,8 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { zone->db = NULL; zone->zmgr = NULL; ISC_LINK_INIT(zone, link); - isc_refcount_init(&zone->erefs, 1); /* Implicit attach. */ - zone->irefs = 0; + isc_refcount_init(&zone->erefs, 1); + isc_refcount_init(&zone->irefs, 0); dns_name_init(&zone->origin, NULL); zone->strnamerd = NULL; zone->strname = NULL; @@ -1073,7 +1073,7 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { result = isc_stats_create(mctx, &zone->gluecachestats, dns_gluecachestatscounter_max); if (result != ISC_R_SUCCESS) { - goto free_erefs; + goto free_refs; } /* Must be after magic is set. */ @@ -1085,9 +1085,10 @@ dns_zone_create(dns_zone_t **zonep, isc_mem_t *mctx) { *zonep = zone; return (ISC_R_SUCCESS); - free_erefs: + free_refs: INSIST(isc_refcount_decrement(&zone->erefs) > 0); isc_refcount_destroy(&zone->erefs); + isc_refcount_destroy(&zone->irefs); ZONEDB_DESTROYLOCK(&zone->dblock); @@ -1111,7 +1112,7 @@ zone_free(dns_zone_t *zone) { REQUIRE(DNS_ZONE_VALID(zone)); REQUIRE(isc_refcount_current(&zone->erefs) == 0); - REQUIRE(zone->irefs == 0); + REQUIRE(isc_refcount_current(&zone->irefs) == 0); REQUIRE(!LOCKED_ZONE(zone)); REQUIRE(zone->timer == NULL); REQUIRE(zone->zmgr == NULL); @@ -5110,7 +5111,9 @@ static bool exit_check(dns_zone_t *zone) { REQUIRE(LOCKED_ZONE(zone)); - if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN) && zone->irefs == 0) { + if (DNS_ZONE_FLAG(zone, DNS_ZONEFLG_SHUTDOWN) && + isc_refcount_current(&zone->irefs) == 0) + { /* * DNS_ZONEFLG_SHUTDOWN can only be set if erefs == 0. */ @@ -5483,9 +5486,8 @@ zone_iattach(dns_zone_t *source, dns_zone_t **target) { REQUIRE(DNS_ZONE_VALID(source)); REQUIRE(LOCKED_ZONE(source)); REQUIRE(target != NULL && *target == NULL); - INSIST(source->irefs + isc_refcount_current(&source->erefs) > 0); - source->irefs++; - INSIST(source->irefs != 0); + INSIST(isc_refcount_increment0(&source->irefs) + + isc_refcount_current(&source->erefs) > 0); *target = source; } @@ -5501,9 +5503,8 @@ zone_idetach(dns_zone_t **zonep) { REQUIRE(LOCKED_ZONE(*zonep)); *zonep = NULL; - INSIST(zone->irefs > 0); - zone->irefs--; - INSIST(zone->irefs + isc_refcount_current(&zone->erefs) > 0); + INSIST(isc_refcount_decrement(&zone->irefs) - 1 + + isc_refcount_current(&zone->erefs) > 0); } void @@ -5515,13 +5516,14 @@ dns_zone_idetach(dns_zone_t **zonep) { zone = *zonep; *zonep = NULL; - LOCK_ZONE(zone); - INSIST(zone->irefs > 0); - zone->irefs--; - free_needed = exit_check(zone); - UNLOCK_ZONE(zone); - if (free_needed) - zone_free(zone); + if (isc_refcount_decrement(&zone->irefs) == 1) { + LOCK_ZONE(zone); + free_needed = exit_check(zone); + UNLOCK_ZONE(zone); + if (free_needed) { + zone_free(zone); + } + } } isc_mem_t * @@ -10405,8 +10407,8 @@ keyfetch_done(isc_task_t *task, isc_event_t *event) { cleanup: dns_db_detach(&kfetch->db); - INSIST(zone->irefs > 0); - zone->irefs--; + isc_refcount_decrement(&zone->irefs); + kfetch->zone = NULL; if (dns_rdataset_isassociated(keydataset)) { @@ -10537,8 +10539,7 @@ zone_refreshkeys(dns_zone_t *zone) { zone->refreshkeycount++; kfetch->zone = zone; - zone->irefs++; - INSIST(zone->irefs != 0); + isc_refcount_increment0(&zone->irefs); kname = dns_fixedname_initname(&kfetch->name); dns_name_dup(name, zone->mctx, kname); dns_rdataset_init(&kfetch->dnskeyset); @@ -10592,7 +10593,7 @@ zone_refreshkeys(dns_zone_t *zone) { fetching = true; } else { zone->refreshkeycount--; - zone->irefs--; + isc_refcount_decrement(&zone->irefs); dns_db_detach(&kfetch->db); dns_rdataset_disassociate(&kfetch->keydataset); dns_name_free(kname, zone->mctx); @@ -13583,8 +13584,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) { LOCK_ZONE(zone); INSIST(zone != zone->raw); if (linked) { - INSIST(zone->irefs > 0); - zone->irefs--; + isc_refcount_decrement(&zone->irefs); } if (zone->request != NULL) { dns_request_cancel(zone->request); @@ -13611,8 +13611,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) { if (zone->timer != NULL) { isc_timer_detach(&zone->timer); - INSIST(zone->irefs > 0); - zone->irefs--; + isc_refcount_decrement(&zone->irefs); } /* @@ -15208,8 +15207,7 @@ receive_secure_serial(isc_task_t *task, isc_event_t *event) { if (event != NULL) { LOCK_ZONE(zone); - INSIST(zone->irefs > 1); - zone->irefs--; + isc_refcount_decrement(&zone->irefs); ISC_LIST_UNLINK(zone->rss_events, event, ev_link); goto nextevent; } @@ -16215,8 +16213,7 @@ zone_xfrdone(dns_zone_t *zone, isc_result_t result) { if (again && !DNS_ZONE_FLAG(zone, DNS_ZONEFLG_EXITING)) queue_soa_query(zone); - INSIST(zone->irefs > 0); - zone->irefs--; + isc_refcount_decrement(&zone->irefs); free_needed = exit_check(zone); UNLOCK_ZONE(zone); if (free_needed) @@ -16380,9 +16377,7 @@ queue_xfrin(dns_zone_t *zone) { RWLOCK(&zmgr->rwlock, isc_rwlocktype_write); ISC_LIST_APPEND(zmgr->waiting_for_xfrin, zone, statelink); - LOCK_ZONE(zone); - zone->irefs++; - UNLOCK_ZONE(zone); + isc_refcount_increment0(&zone->irefs); zone->statelist = &zmgr->waiting_for_xfrin; result = zmgr_start_xfrin_ifquota(zmgr, zone); RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write); @@ -16850,7 +16845,7 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_taskmgr_t *taskmgr, zmgr = isc_mem_get(mctx, sizeof(*zmgr)); zmgr->mctx = NULL; - zmgr->refs = 1; + isc_refcount_init(&zmgr->refs, 1); isc_mem_attach(mctx, &zmgr->mctx); zmgr->taskmgr = taskmgr; zmgr->timermgr = timermgr; @@ -17012,12 +17007,11 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) { /* * The timer "holds" a iref. */ - zone->irefs++; - INSIST(zone->irefs != 0); + isc_refcount_increment0(&zone->irefs); ISC_LIST_APPEND(zmgr->zones, zone, link); zone->zmgr = zmgr; - zmgr->refs++; + isc_refcount_increment(&zmgr->refs); goto unlock; @@ -17044,9 +17038,10 @@ dns_zonemgr_releasezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) { ISC_LIST_UNLINK(zmgr->zones, zone, link); zone->zmgr = NULL; - zmgr->refs--; - if (zmgr->refs == 0) + + if (isc_refcount_decrement(&zmgr->refs) == 1) { free_now = true; + } UNLOCK_ZONE(zone); RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write); @@ -17061,31 +17056,23 @@ dns_zonemgr_attach(dns_zonemgr_t *source, dns_zonemgr_t **target) { REQUIRE(DNS_ZONEMGR_VALID(source)); REQUIRE(target != NULL && *target == NULL); - RWLOCK(&source->rwlock, isc_rwlocktype_write); - REQUIRE(source->refs > 0); - source->refs++; - INSIST(source->refs > 0); - RWUNLOCK(&source->rwlock, isc_rwlocktype_write); + isc_refcount_increment(&source->refs); + *target = source; } void dns_zonemgr_detach(dns_zonemgr_t **zmgrp) { dns_zonemgr_t *zmgr; - bool free_now = false; REQUIRE(zmgrp != NULL); zmgr = *zmgrp; REQUIRE(DNS_ZONEMGR_VALID(zmgr)); - RWLOCK(&zmgr->rwlock, isc_rwlocktype_write); - zmgr->refs--; - if (zmgr->refs == 0) - free_now = true; - RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write); - - if (free_now) + if (isc_refcount_decrement(&zmgr->refs) == 1) { zonemgr_free(zmgr); + } + *zmgrp = NULL; } @@ -17256,11 +17243,11 @@ static void zonemgr_free(dns_zonemgr_t *zmgr) { isc_mem_t *mctx; - INSIST(zmgr->refs == 0); INSIST(ISC_LIST_EMPTY(zmgr->zones)); zmgr->magic = 0; + isc_refcount_destroy(&zmgr->refs); isc_mutex_destroy(&zmgr->iolock); isc_ratelimiter_detach(&zmgr->notifyrl); isc_ratelimiter_detach(&zmgr->refreshrl); @@ -19514,9 +19501,7 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) { /* * The timer "holds" a iref. */ - raw->irefs++; - INSIST(raw->irefs != 0); - + isc_refcount_increment0(&raw->irefs); /* dns_zone_attach(raw, &zone->raw); */ isc_refcount_increment(&raw->erefs); @@ -19530,7 +19515,7 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) { ISC_LIST_APPEND(zmgr->zones, raw, link); raw->zmgr = zmgr; - zmgr->refs++; + isc_refcount_increment(&zmgr->refs); unlock: UNLOCK_ZONE(raw); diff --git a/lib/isc/ratelimiter.c b/lib/isc/ratelimiter.c index cf97ad7b39..8bee4e7e0f 100644 --- a/lib/isc/ratelimiter.c +++ b/lib/isc/ratelimiter.c @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -32,7 +33,7 @@ typedef enum { struct isc_ratelimiter { isc_mem_t * mctx; isc_mutex_t lock; - int refs; + isc_refcount_t references; isc_task_t * task; isc_timer_t * timer; isc_interval_t interval; @@ -60,14 +61,15 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr, INSIST(ratelimiterp != NULL && *ratelimiterp == NULL); rl = isc_mem_get(mctx, sizeof(*rl)); - rl->mctx = mctx; - rl->refs = 1; - rl->task = task; + *rl = (isc_ratelimiter_t){ + .mctx = mctx, + .task = task, + .pertic = 1, + .state = isc_ratelimiter_idle, + }; + + isc_refcount_init(&rl->references, 1); isc_interval_set(&rl->interval, 0, 0); - rl->timer = NULL; - rl->pertic = 1; - rl->pushpop = false; - rl->state = isc_ratelimiter_idle; ISC_LIST_INIT(rl->pending); isc_mutex_init(&rl->lock); @@ -82,7 +84,7 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr, * Increment the reference count to indicate that we may * (soon) have events outstanding. */ - rl->refs++; + isc_refcount_increment(&rl->references); ISC_EVENT_INIT(&rl->shutdownevent, sizeof(isc_event_t), @@ -213,14 +215,14 @@ ratelimiter_tick(isc_task_t *task, isc_event_t *event) { */ ISC_LIST_UNLINK(rl->pending, p, ev_ratelink); } else { - isc_result_t result; /* * No work left to do. Stop the timer so that we don't * waste resources by having it fire periodically. */ - result = isc_timer_reset(rl->timer, - isc_timertype_inactive, - NULL, NULL, false); + isc_result_t result = + isc_timer_reset(rl->timer, + isc_timertype_inactive, + NULL, NULL, false); RUNTIME_CHECK(result == ISC_R_SUCCESS); rl->state = isc_ratelimiter_idle; pertic = 0; /* Force the loop to exit. */ @@ -237,7 +239,6 @@ ratelimiter_tick(isc_task_t *task, isc_event_t *event) { void isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { isc_event_t *ev; - isc_task_t *task; REQUIRE(rl != NULL); @@ -246,9 +247,9 @@ isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { (void)isc_timer_reset(rl->timer, isc_timertype_inactive, NULL, NULL, false); while ((ev = ISC_LIST_HEAD(rl->pending)) != NULL) { + isc_task_t *task = ev->ev_sender; ISC_LIST_UNLINK(rl->pending, ev, ev_ratelink); ev->ev_attributes |= ISC_EVENTATTR_CANCELED; - task = ev->ev_sender; isc_task_send(task, &ev); } isc_timer_detach(&rl->timer); @@ -280,36 +281,25 @@ ratelimiter_free(isc_ratelimiter_t *rl) { void isc_ratelimiter_attach(isc_ratelimiter_t *source, isc_ratelimiter_t **target) { - REQUIRE(source != NULL); REQUIRE(target != NULL && *target == NULL); - LOCK(&source->lock); - REQUIRE(source->refs > 0); - source->refs++; - INSIST(source->refs > 0); - UNLOCK(&source->lock); + isc_refcount_increment(&source->references); + *target = source; } void isc_ratelimiter_detach(isc_ratelimiter_t **rlp) { isc_ratelimiter_t *rl; - bool free_now = false; REQUIRE(rlp != NULL && *rlp != NULL); rl = *rlp; - LOCK(&rl->lock); - REQUIRE(rl->refs > 0); - rl->refs--; - if (rl->refs == 0) - free_now = true; - UNLOCK(&rl->lock); - - if (free_now) + if (isc_refcount_decrement(&rl->references) == 1) { ratelimiter_free(rl); + } *rlp = NULL; } -- GitLab