diff --git a/bin/named/server.c b/bin/named/server.c index f197135b71..26ad34a1e2 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10008,6 +10008,11 @@ run_server(isc_task_t *task, isc_event_t *event) { isc_event_free(&event); + CHECKFATAL(dns_zonemgr_create(named_g_mctx, named_g_loopmgr, + named_g_taskmgr, named_g_netmgr, + &server->zonemgr), + "dns_zonemgr_create"); + CHECKFATAL(dns_dispatchmgr_create(named_g_mctx, named_g_netmgr, &named_g_dispatchmgr), "creating dispatch manager"); @@ -10322,11 +10327,6 @@ named_server_create(isc_mem_t *mctx, named_server_t **serverp) { server->sighup = isc_signal_new( named_g_loopmgr, named_server_reloadwanted, server, SIGHUP); - CHECKFATAL(dns_zonemgr_create(named_g_mctx, named_g_loopmgr, - named_g_taskmgr, named_g_netmgr, - &server->zonemgr), - "dns_zonemgr_create"); - CHECKFATAL(isc_stats_create(server->mctx, &server->sockstats, isc_sockstatscounter_max), "isc_stats_create"); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index 9a7ab8f89b..08e0000bd8 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -18865,7 +18865,7 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, dns_zonemgr_t **zmgrp) { dns_zonemgr_t *zmgr; isc_result_t result; - isc_loop_t *mainloop = isc_loop_main(loopmgr); + isc_loop_t *loop = isc_loop_current(loopmgr); REQUIRE(mctx != NULL); REQUIRE(loopmgr != NULL); @@ -18899,35 +18899,11 @@ dns_zonemgr_create(isc_mem_t *mctx, isc_loopmgr_t *loopmgr, /* Unreachable lock. */ isc_rwlock_init(&zmgr->urlock, 0, 0); - result = isc_ratelimiter_create(mainloop, &zmgr->checkdsrl); - INSIST(result == ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) { - goto free_urlock; - } - - result = isc_ratelimiter_create(mainloop, &zmgr->notifyrl); - INSIST(result == ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) { - goto free_checkdsrl; - } - - result = isc_ratelimiter_create(mainloop, &zmgr->refreshrl); - INSIST(result == ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) { - goto free_notifyrl; - } - - result = isc_ratelimiter_create(mainloop, &zmgr->startupnotifyrl); - INSIST(result == ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) { - goto free_refreshrl; - } - - result = isc_ratelimiter_create(mainloop, &zmgr->startuprefreshrl); - INSIST(result == ISC_R_SUCCESS); - if (result != ISC_R_SUCCESS) { - goto free_startupnotifyrl; - } + isc_ratelimiter_create(loop, &zmgr->checkdsrl); + isc_ratelimiter_create(loop, &zmgr->notifyrl); + isc_ratelimiter_create(loop, &zmgr->refreshrl); + isc_ratelimiter_create(loop, &zmgr->startupnotifyrl); + isc_ratelimiter_create(loop, &zmgr->startuprefreshrl); zmgr->zonetasks = isc_mem_get( zmgr->mctx, zmgr->workers * sizeof(zmgr->zonetasks[0])); @@ -19012,19 +18988,15 @@ free_zonetasks: isc_ratelimiter_shutdown(zmgr->startuprefreshrl); isc_ratelimiter_detach(&zmgr->startuprefreshrl); -free_startupnotifyrl: isc_ratelimiter_shutdown(zmgr->startupnotifyrl); isc_ratelimiter_detach(&zmgr->startupnotifyrl); -free_refreshrl: isc_ratelimiter_shutdown(zmgr->refreshrl); isc_ratelimiter_detach(&zmgr->refreshrl); -free_notifyrl: isc_ratelimiter_shutdown(zmgr->notifyrl); isc_ratelimiter_detach(&zmgr->notifyrl); -free_checkdsrl: isc_ratelimiter_shutdown(zmgr->checkdsrl); isc_ratelimiter_detach(&zmgr->checkdsrl); -free_urlock: + isc_rwlock_destroy(&zmgr->urlock); isc_rwlock_destroy(&zmgr->rwlock); isc_mem_put(zmgr->mctx, zmgr, sizeof(*zmgr)); diff --git a/lib/isc/include/isc/ratelimiter.h b/lib/isc/include/isc/ratelimiter.h index 5eb54f341f..1f4b8d2d7e 100644 --- a/lib/isc/include/isc/ratelimiter.h +++ b/lib/isc/include/isc/ratelimiter.h @@ -41,8 +41,8 @@ ISC_LANG_BEGINDECLS ***** Functions. *****/ -isc_result_t -isc_ratelimiter_create(isc_loop_t *loop, isc_ratelimiter_t **ratelimiterp); +void +isc_ratelimiter_create(isc_loop_t *loop, isc_ratelimiter_t **rlp); /*%< * Create a rate limiter. The execution interval is initially undefined. */ diff --git a/lib/isc/ratelimiter.c b/lib/isc/ratelimiter.c index f4ebcb22cf..07bc45c236 100644 --- a/lib/isc/ratelimiter.c +++ b/lib/isc/ratelimiter.c @@ -52,15 +52,21 @@ struct isc_ratelimiter { }; static void -ratelimiter_tick(void *arg); +isc__ratelimiter_tick(void *arg); -isc_result_t -isc_ratelimiter_create(isc_loop_t *loop, isc_ratelimiter_t **ratelimiterp) { +static void +isc__ratelimiter_start(void *arg); + +static void +isc__ratelimiter_doshutdown(void *arg); + +void +isc_ratelimiter_create(isc_loop_t *loop, isc_ratelimiter_t **rlp) { isc_ratelimiter_t *rl = NULL; isc_mem_t *mctx; - INSIST(loop != NULL); - INSIST(ratelimiterp != NULL && *ratelimiterp == NULL); + REQUIRE(loop != NULL); + REQUIRE(rlp != NULL && *rlp == NULL); mctx = isc_loop_getmctx(loop); @@ -77,10 +83,11 @@ isc_ratelimiter_create(isc_loop_t *loop, isc_ratelimiter_t **ratelimiterp) { isc_interval_set(&rl->interval, 0, 0); ISC_LIST_INIT(rl->pending); + isc_timer_create(rl->loop, isc__ratelimiter_tick, rl, &rl->timer); + isc_mutex_init(&rl->lock); - *ratelimiterp = rl; - return (ISC_R_SUCCESS); + *rlp = rl; } void @@ -90,10 +97,7 @@ isc_ratelimiter_setinterval(isc_ratelimiter_t *rl, isc_interval_t *interval) { LOCK(&rl->lock); rl->interval = *interval; - /* - * If the timer is currently running, its rate will change during - * the next tick. - */ + /* The interval will be adjusted on the next tick */ UNLOCK(&rl->lock); } @@ -116,6 +120,37 @@ isc_ratelimiter_setpushpop(isc_ratelimiter_t *rl, bool pushpop) { UNLOCK(&rl->lock); } +static void +isc__ratelimiter_start(void *arg) { + isc_ratelimiter_t *rl = arg; + isc_interval_t interval; + + REQUIRE(VALID_RATELIMITER(rl)); + + LOCK(&rl->lock); + switch (rl->state) { + case isc_ratelimiter_ratelimited: + /* The first tick happens immediately */ + isc_interval_set(&interval, 0, 0); + isc_timer_start(rl->timer, isc_timertype_once, &interval); + break; + case isc_ratelimiter_shuttingdown: + /* The ratelimiter is shutting down */ + break; + case isc_ratelimiter_idle: + /* + * This could happen if we are changing the interval on the + * ratelimiter, but all the events were processed and the timer + * was stopped before the new interval could be applied. + */ + break; + default: + UNREACHABLE(); + } + UNLOCK(&rl->lock); + isc_ratelimiter_detach(&rl); +} + isc_result_t isc_ratelimiter_enqueue(isc_ratelimiter_t *rl, isc_task_t *task, isc_event_t **eventp) { @@ -136,9 +171,9 @@ isc_ratelimiter_enqueue(isc_ratelimiter_t *rl, isc_task_t *task, case isc_ratelimiter_idle: /* Start the ratelimiter */ isc_ratelimiter_ref(rl); - isc_async_run(rl->loop, ratelimiter_tick, rl); + isc_async_run(rl->loop, isc__ratelimiter_start, rl); rl->state = isc_ratelimiter_ratelimited; - /* FALLTHROUGH */ + FALLTHROUGH; case isc_ratelimiter_ratelimited: event->ev_sender = task; *eventp = NULL; @@ -175,11 +210,10 @@ isc_ratelimiter_dequeue(isc_ratelimiter_t *rl, isc_event_t *event) { } static void -ratelimiter_tick(void *arg) { +isc__ratelimiter_tick(void *arg) { isc_ratelimiter_t *rl = (isc_ratelimiter_t *)arg; isc_event_t *event; uint32_t pertic; - bool do_destroy = false; ISC_LIST(isc_event_t) pending; REQUIRE(VALID_RATELIMITER(rl)); @@ -187,70 +221,88 @@ ratelimiter_tick(void *arg) { ISC_LIST_INIT(pending); LOCK(&rl->lock); + + REQUIRE(rl->timer != NULL); + if (rl->state == isc_ratelimiter_shuttingdown) { - UNLOCK(&rl->lock); - do_destroy = (rl->timer != NULL); - goto done; + INSIST(EMPTY(rl->pending)); + goto unlock; } - if (rl->timer == NULL) { - isc_timer_create(rl->loop, ratelimiter_tick, rl, &rl->timer); - } - - /* - * If the timer was already running with a different rate, - * this updates it to the correct one. - */ - isc_timer_start(rl->timer, isc_timertype_ticker, &rl->interval); - pertic = rl->pertic; while (pertic != 0) { - pertic--; event = ISC_LIST_HEAD(rl->pending); if (event != NULL) { /* There is work to do. Let's do it after unlocking. */ ISC_LIST_UNLINK(rl->pending, event, ev_ratelink); ISC_LIST_APPEND(pending, event, ev_ratelink); } else { - /* There's no more work to do, destroy the timer */ - do_destroy = true; + /* + * We processed all the scheduled work, but there's a + * room for at least one more event (we haven't consumed + * all of the "pertick"), so we can stop the ratelimiter + * now, and don't worry about isc_ratelimiter_enqueue() + * sending an extra event immediately. + */ rl->state = isc_ratelimiter_idle; break; } + pertic--; } + + if (rl->state != isc_ratelimiter_idle) { + /* Reschedule the timer */ + isc_timer_start(rl->timer, isc_timertype_once, &rl->interval); + } +unlock: UNLOCK(&rl->lock); while ((event = ISC_LIST_HEAD(pending)) != NULL) { ISC_LIST_UNLINK(pending, event, ev_ratelink); isc_task_send(event->ev_sender, &event); } +} -done: - /* No work left to do. Stop and destroy the timer. */ - if (do_destroy) { - isc_timer_destroy(&rl->timer); - isc_ratelimiter_detach(&rl); - } +void +isc__ratelimiter_doshutdown(void *arg) { + isc_ratelimiter_t *rl = arg; + + REQUIRE(VALID_RATELIMITER(rl)); + + LOCK(&rl->lock); + INSIST(rl->state == isc_ratelimiter_shuttingdown); + INSIST(EMPTY(rl->pending)); + + isc_timer_stop(rl->timer); + isc_timer_destroy(&rl->timer); + isc_loop_detach(&rl->loop); + UNLOCK(&rl->lock); + isc_ratelimiter_detach(&rl); } void isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { isc_event_t *event; + ISC_LIST(isc_event_t) pending; REQUIRE(VALID_RATELIMITER(rl)); + ISC_LIST_INIT(pending); + LOCK(&rl->lock); if (rl->state != isc_ratelimiter_shuttingdown) { rl->state = isc_ratelimiter_shuttingdown; - - while ((event = ISC_LIST_HEAD(rl->pending)) != NULL) { - ISC_LIST_UNLINK(rl->pending, event, ev_ratelink); - event->ev_attributes |= ISC_EVENTATTR_CANCELED; - isc_task_send(event->ev_sender, &event); - } - isc_loop_detach(&rl->loop); + ISC_LIST_MOVE(pending, rl->pending); + isc_ratelimiter_ref(rl); + isc_async_run(rl->loop, isc__ratelimiter_doshutdown, rl); } UNLOCK(&rl->lock); + + while ((event = ISC_LIST_HEAD(pending)) != NULL) { + ISC_LIST_UNLINK(pending, event, ev_ratelink); + event->ev_attributes |= ISC_EVENTATTR_CANCELED; + isc_task_send(event->ev_sender, &event); + } } static void