From aa79283a0d97d5d77d3c17bcb1756035eabe647a Mon Sep 17 00:00:00 2001 From: Andreas Gustafsson Date: Thu, 15 Jun 2000 17:40:22 +0000 Subject: [PATCH] 257. [bug] The server detached the last zone manager reference too early, while it could still be in use by queries. This manifested itself as assertion failures during the shutdown process for busy name servers (RT #133). 256. [func] isc_ratelimiter_t now has attach/detach semantics, and isc_ratelimiter_shutdown guarantees that the rate limiter is detached from its task. 255. [func] New function dns_zonemgr_attach(). --- CHANGES | 11 ++++ bin/named/server.c | 3 +- lib/dns/zone.c | 14 +++--- lib/isc/include/isc/ratelimiter.h | 33 ++++++------ lib/isc/ratelimiter.c | 84 +++++++++++++++++++++++-------- 5 files changed, 103 insertions(+), 42 deletions(-) diff --git a/CHANGES b/CHANGES index e71bad209a..ba023e5b6e 100644 --- a/CHANGES +++ b/CHANGES @@ -1,3 +1,14 @@ + 257. [bug] The server detached the last zone manager reference + too early, while it could still be in use by queries. + This manifested itself as assertion failures during the + shutdown process for busy name servers (RT #133). + + 256. [func] isc_ratelimiter_t now has attach/detach semantics, and + isc_ratelimiter_shutdown guarantees that the rate + limiter is detached from its task. + + 255. [func] New function dns_zonemgr_attach(). + 254. [bug] suppress "query denied" messages on additional data lookups. diff --git a/bin/named/server.c b/bin/named/server.c index f7bc329aa8..97b671d9e3 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -1501,7 +1501,6 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { dns_dispatchmgr_destroy(&ns_g_dispatchmgr); dns_zonemgr_shutdown(server->zonemgr); - dns_zonemgr_detach(&server->zonemgr); isc_task_detach(&server->task); @@ -1600,6 +1599,8 @@ ns_server_destroy(ns_server_t **serverp) { ns_server_t *server = *serverp; REQUIRE(NS_SERVER_VALID(server)); + dns_zonemgr_detach(&server->zonemgr); + if (server->tkeyctx != NULL) dns_tkeyctx_destroy(&server->tkeyctx); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index d0ac141277..e2d8f98fa5 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -15,7 +15,7 @@ * SOFTWARE. */ -/* $Id: zone.c,v 1.147 2000/06/15 16:11:49 gson Exp $ */ +/* $Id: zone.c,v 1.148 2000/06/15 17:40:18 gson Exp $ */ #include @@ -4151,6 +4151,11 @@ dns_zonemgr_shutdown(dns_zonemgr_t *zmgr) { REQUIRE(DNS_ZONEMGR_VALID(zmgr)); isc_ratelimiter_shutdown(zmgr->rl); + + if (zmgr->task != NULL) + isc_task_destroy(&zmgr->task); + if (zmgr->zonetasks != NULL) + isc_taskpool_destroy(&zmgr->zonetasks); } static void @@ -4162,12 +4167,7 @@ zonemgr_free(dns_zonemgr_t *zmgr) { zmgr->magic = 0; - if (zmgr->task != NULL) - isc_task_destroy(&zmgr->task); - if (zmgr->zonetasks != NULL) - isc_taskpool_destroy(&zmgr->zonetasks); - - isc_ratelimiter_destroy(&zmgr->rl); + isc_ratelimiter_detach(&zmgr->rl); isc_rwlock_destroy(&zmgr->conflock); isc_rwlock_destroy(&zmgr->rwlock); diff --git a/lib/isc/include/isc/ratelimiter.h b/lib/isc/include/isc/ratelimiter.h index f7883ca25f..04c8a088e3 100644 --- a/lib/isc/include/isc/ratelimiter.h +++ b/lib/isc/include/isc/ratelimiter.h @@ -88,24 +88,29 @@ isc_ratelimiter_enqueue(isc_ratelimiter_t *rl, isc_task_t *task, void isc_ratelimiter_shutdown(isc_ratelimiter_t *ratelimiter); /* - * Shut down a rate limiter. All events that have not yet been - * dispatched to the task are dispatched immediately with - * the ISC_EVENTATTR_CANCELED bit set in ev_attributes. - * Further attempts to enqueue events will fail with - * ISC_R_SHUTTINGDOWN. + * Shut down a rate limiter. + * + * Ensures: + * All events that have not yet been + * dispatched to the task are dispatched immediately with + * the ISC_EVENTATTR_CANCELED bit set in ev_attributes. + * + * Further attempts to enqueue events will fail with + * ISC_R_SHUTTINGDOWN. + * + * The reatelimiter is no longer attached to its task. */ void -isc_ratelimiter_destroy(isc_ratelimiter_t **ratelimiterp); +isc_ratelimiter_attach(isc_ratelimiter_t *source, isc_ratelimiter_t **target); /* - * Destroy a rate limiter. - * Does not destroy the task or events already queued on it. - * - * Requires: - * The rate limiter to have been shut down. - * - * Ensures: - * Resources used by the rate limiter will be freed. + * Attach to a rate limiter. + */ + +void +isc_ratelimiter_detach(isc_ratelimiter_t **ratelimiterp); +/* + * Detach from a rate limiter. */ ISC_LANG_ENDDECLS diff --git a/lib/isc/ratelimiter.c b/lib/isc/ratelimiter.c index b65140c06f..1e2eabf9fb 100644 --- a/lib/isc/ratelimiter.c +++ b/lib/isc/ratelimiter.c @@ -33,6 +33,7 @@ typedef enum { struct isc_ratelimiter { isc_mem_t * mctx; isc_mutex_t lock; + int refs; isc_task_t * task; isc_timer_t * timer; isc_interval_t interval; @@ -48,12 +49,7 @@ static void ratelimiter_tick(isc_task_t *task, isc_event_t *event); static void -ratelimiter_shutdowncomplete(isc_task_t *task, isc_event_t *event) { - isc_ratelimiter_t *rl = (isc_ratelimiter_t *)event->ev_arg; - UNUSED(task); - isc_mutex_destroy(&rl->lock); - isc_mem_put(rl->mctx, rl, sizeof(*rl)); -} +ratelimiter_shutdowncomplete(isc_task_t *task, isc_event_t *event); isc_result_t isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr, @@ -67,6 +63,7 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr, if (rl == NULL) return ISC_R_NOMEMORY; rl->mctx = mctx; + rl->refs = 1; rl->task = task; isc_interval_set(&rl->interval, 0, 0); rl->timer = NULL; @@ -83,6 +80,12 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr, if (result != ISC_R_SUCCESS) goto free_mutex; + /* + * Increment the reference count to indicate that we may + * (soon) have events outstanding. + */ + rl->refs++; + ISC_EVENT_INIT(&rl->shutdownevent, sizeof(isc_event_t), 0, NULL, ISC_RATELIMITEREVENT_SHUTDOWN, @@ -204,27 +207,68 @@ isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { isc_task_t *task; LOCK(&rl->lock); rl->state = isc_ratelimiter_shuttingdown; - (void) isc_timer_reset(rl->timer, isc_timertype_inactive, - NULL, NULL, ISC_FALSE); + (void)isc_timer_reset(rl->timer, isc_timertype_inactive, + NULL, NULL, ISC_FALSE); while ((ev = ISC_LIST_HEAD(rl->pending)) != NULL) { ISC_LIST_UNLINK(rl->pending, ev, ev_link); ev->ev_attributes |= ISC_EVENTATTR_CANCELED; task = ev->ev_sender; isc_task_send(task, &ev); } + isc_timer_detach(&rl->timer); + /* + * Send an event to our task. The delivery of this event + * indicates that no more timer events will be delivered. + */ + ev = &rl->shutdownevent; + isc_task_send(rl->task, &ev); + UNLOCK(&rl->lock); } -void -isc_ratelimiter_destroy(isc_ratelimiter_t **ratelimiterp) { - isc_ratelimiter_t *rl = *ratelimiterp; - isc_event_t *ev = &rl->shutdownevent; - isc_timer_detach(&rl->timer); - /* - * Send an event to our task and wait for it to be delivered - * before freeing memory. This guarantees that any timer - * event still in the task's queue are delivered first. - */ - isc_task_send(rl->task, &ev); - *ratelimiterp = NULL; +static void +ratelimiter_shutdowncomplete(isc_task_t *task, isc_event_t *event) { + isc_ratelimiter_t *rl = (isc_ratelimiter_t *)event->ev_arg; + + UNUSED(task); + + isc_ratelimiter_detach(&rl); } + +static void +ratelimiter_free(isc_ratelimiter_t *rl) { + isc_mutex_destroy(&rl->lock); + isc_mem_put(rl->mctx, rl, sizeof(*rl)); +} + +void +isc_ratelimiter_attach(isc_ratelimiter_t *source, isc_ratelimiter_t **target) { + REQUIE(souce != NULL); + REQUIRE(target != NULL && *target == NULL); + + LOCK(&source->lock); + REQUIRE(source->refs > 0); + source->refs++; + INSIST(source->refs > 0); + UNLOCK(&source->lock); + *target = source; +} + +void +isc_ratelimiter_detach(isc_ratelimiter_t **rlp) { + isc_ratelimiter_t *rl = *rlp; + isc_boolean_t free_now = ISC_FALSE; + + LOCK(&rl->lock); + REQUIRE(rl->refs > 0); + rl->refs--; + if (rl->refs == 0) + free_now = ISC_TRUE; + UNLOCK(&rl->lock); + + if (free_now) + ratelimiter_free(rl); + + *rlp = NULL; +} +