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; +} +