Refactor isc_timer_create() to just create timer

The isc_timer_create() function was a bit conflated.  It could have been
used to create a timer and start it at the same time.  As there was a
single place where this was done before (see the previous commit for
nta.c), this was cleaned up and the isc_timer_create() function was
changed to only create new timer.
This commit is contained in:
Ondřej Surý 2022-03-11 12:09:35 +01:00 committed by Evan Hunt
parent 514053f244
commit c259cecc90
12 changed files with 64 additions and 208 deletions

View file

@ -9942,27 +9942,17 @@ run_server(isc_task_t *task, isc_event_t *event) {
true, &server->interfacemgr),
"creating interface manager");
CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
NULL, NULL, server->task,
interface_timer_tick, server,
&server->interface_timer),
"creating interface timer");
isc_timer_create(named_g_timermgr, server->task, interface_timer_tick,
server, &server->interface_timer);
CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
NULL, NULL, server->task,
heartbeat_timer_tick, server,
&server->heartbeat_timer),
"creating heartbeat timer");
isc_timer_create(named_g_timermgr, server->task, heartbeat_timer_tick,
server, &server->heartbeat_timer);
CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
NULL, NULL, server->task, tat_timer_tick,
server, &server->tat_timer),
"creating trust anchor telemetry timer");
isc_timer_create(named_g_timermgr, server->task, tat_timer_tick, server,
&server->tat_timer);
CHECKFATAL(isc_timer_create(named_g_timermgr, isc_timertype_inactive,
NULL, NULL, server->task, pps_timer_tick,
server, &server->pps_timer),
"creating pps timer");
isc_timer_create(named_g_timermgr, server->task, pps_timer_tick, server,
&server->pps_timer);
CHECKFATAL(
cfg_parser_create(named_g_mctx, named_g_lctx, &named_g_parser),

View file

@ -1400,8 +1400,9 @@ be triggered at that time.
* 'arg' as its argument in task 'task'.
*/
isc_timer_t *timer = NULL;
result = isc_timer_create(timermgr, isc_timertype_once, NULL,
interval, task, timeout, arg, &timer);
result = isc_timer_create(timermgr, task, timeout, arg, &timer);
result = isc_timer_reset(timermgr, isc_timertype_once, NULL,
interval, false);
An event can also be explicitly triggered via `isc_task_send()`.

View file

@ -617,7 +617,6 @@ dns_catz_catzs_set_view(dns_catz_zones_t *catzs, dns_view_t *view) {
isc_result_t
dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
const dns_name_t *name) {
isc_result_t result;
dns_catz_zone_t *new_zone;
REQUIRE(DNS_CATZ_ZONES_VALID(catzs));
@ -634,13 +633,9 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
isc_ht_init(&new_zone->entries, catzs->mctx, 4);
new_zone->updatetimer = NULL;
result = isc_timer_create(catzs->timermgr, isc_timertype_inactive, NULL,
NULL, catzs->updater,
dns_catz_update_taskaction, new_zone,
&new_zone->updatetimer);
if (result != ISC_R_SUCCESS) {
goto cleanup_ht;
}
isc_timer_create(catzs->timermgr, catzs->updater,
dns_catz_update_taskaction, new_zone,
&new_zone->updatetimer);
isc_time_settoepoch(&new_zone->lastupdated);
new_zone->updatepending = false;
@ -658,13 +653,6 @@ dns_catz_new_zone(dns_catz_zones_t *catzs, dns_catz_zone_t **zonep,
*zonep = new_zone;
return (ISC_R_SUCCESS);
cleanup_ht:
isc_ht_destroy(&new_zone->entries);
dns_name_free(&new_zone->name, catzs->mctx);
isc_mem_put(catzs->mctx, new_zone, sizeof(*new_zone));
return (result);
}
isc_result_t

View file

@ -289,13 +289,8 @@ settimer(dns_ntatable_t *ntatable, dns_nta_t *nta, uint32_t lifetime) {
return (ISC_R_SUCCESS);
}
result = isc_timer_create(ntatable->timermgr, isc_timertype_inactive,
NULL, NULL, ntatable->task, checkbogus, nta,
&nta->timer);
if (result != ISC_R_SUCCESS) {
return (result);
}
isc_timer_create(ntatable->timermgr, ntatable->task, checkbogus, nta,
&nta->timer);
isc_interval_set(&interval, view->nta_recheck, 0);
result = isc_timer_reset(nta->timer, isc_timertype_ticker, NULL,
&interval, false);

View file

@ -4857,15 +4857,8 @@ fctx_create(dns_resolver_t *res, isc_task_t *task, const dns_name_t *name,
* lifetime. It will be made active when the fetch is
* started.
*/
iresult = isc_timer_create(res->timermgr, isc_timertype_inactive, NULL,
NULL, res->buckets[bucketnum].task,
fctx_expired, fctx, &fctx->timer);
if (iresult != ISC_R_SUCCESS) {
UNEXPECTED_ERROR(__FILE__, __LINE__, "isc_timer_create: %s",
isc_result_totext(iresult));
result = ISC_R_UNEXPECTED;
goto cleanup_qmessage;
}
isc_timer_create(res->timermgr, res->buckets[bucketnum].task,
fctx_expired, fctx, &fctx->timer);
/*
* Default retry interval initialization. We set the interval
@ -10167,13 +10160,9 @@ dns_resolver_create(dns_view_t *view, isc_taskmgr_t *taskmgr,
}
isc_task_setname(task, "resolver_task", NULL);
result = isc_timer_create(timermgr, isc_timertype_inactive, NULL, NULL,
task, spillattimer_countdown, res,
&res->spillattimer);
isc_timer_create(timermgr, task, spillattimer_countdown, res,
&res->spillattimer);
isc_task_detach(&task);
if (result != ISC_R_SUCCESS) {
goto cleanup_primelock;
}
res->magic = RES_MAGIC;

View file

@ -1514,7 +1514,6 @@ cleanup_rbt:
isc_result_t
dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
dns_rpz_zone_t *zone;
isc_result_t result;
REQUIRE(rpzp != NULL && *rpzp == NULL);
REQUIRE(rpzs != NULL);
@ -1527,13 +1526,8 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
memset(zone, 0, sizeof(*zone));
isc_refcount_init(&zone->refs, 1);
result = isc_timer_create(rpzs->timermgr, isc_timertype_inactive, NULL,
NULL, rpzs->updater,
dns_rpz_update_taskaction, zone,
&zone->updatetimer);
if (result != ISC_R_SUCCESS) {
goto cleanup_timer;
}
isc_timer_create(rpzs->timermgr, rpzs->updater,
dns_rpz_update_taskaction, zone, &zone->updatetimer);
/*
* This will never be used, but costs us nothing and
@ -1573,14 +1567,6 @@ dns_rpz_new_zone(dns_rpz_zones_t *rpzs, dns_rpz_zone_t **rpzp) {
*rpzp = zone;
return (ISC_R_SUCCESS);
cleanup_timer:
isc_refcount_decrementz(&zone->refs);
isc_refcount_destroy(&zone->refs);
isc_mem_put(rpzs->mctx, zone, sizeof(*zone));
return (result);
}
isc_result_t

View file

@ -18946,8 +18946,6 @@ dns_zonemgr_createzone(dns_zonemgr_t *zmgr, dns_zone_t **zonep) {
isc_result_t
dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
isc_result_t result;
REQUIRE(DNS_ZONE_VALID(zone));
REQUIRE(DNS_ZONEMGR_VALID(zmgr));
@ -18972,13 +18970,8 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
isc_task_setname(zone->task, "zone", zone);
isc_task_setname(zone->loadtask, "loadzone", zone);
result = isc_timer_create(zmgr->timermgr, isc_timertype_inactive, NULL,
NULL, zone->task, zone_timer, zone,
&zone->timer);
if (result != ISC_R_SUCCESS) {
goto cleanup_tasks;
}
isc_timer_create(zmgr->timermgr, zone->task, zone_timer, zone,
&zone->timer);
/*
* The timer "holds" a iref.
@ -18991,16 +18984,9 @@ dns_zonemgr_managezone(dns_zonemgr_t *zmgr, dns_zone_t *zone) {
zone->zmgr = zmgr;
isc_refcount_increment(&zmgr->refs);
goto unlock;
cleanup_tasks:
isc_task_detach(&zone->loadtask);
isc_task_detach(&zone->task);
unlock:
UNLOCK_ZONE(zone);
RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write);
return (result);
return (ISC_R_SUCCESS);
}
void
@ -22485,7 +22471,6 @@ dns_zone_getserialupdatemethod(dns_zone_t *zone) {
*/
isc_result_t
dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
isc_result_t result;
dns_zonemgr_t *zmgr;
REQUIRE(DNS_ZONE_VALID(zone));
@ -22510,12 +22495,8 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
LOCK_ZONE(zone);
LOCK_ZONE(raw);
result = isc_timer_create(zmgr->timermgr, isc_timertype_inactive, NULL,
NULL, zone->task, zone_timer, raw,
&raw->timer);
if (result != ISC_R_SUCCESS) {
goto unlock;
}
isc_timer_create(zmgr->timermgr, zone->task, zone_timer, raw,
&raw->timer);
/*
* The timer "holds" a iref.
@ -22536,11 +22517,10 @@ dns_zone_link(dns_zone_t *zone, dns_zone_t *raw) {
raw->zmgr = zmgr;
isc_refcount_increment(&zmgr->refs);
unlock:
UNLOCK_ZONE(raw);
UNLOCK_ZONE(zone);
RWUNLOCK(&zmgr->rwlock, isc_rwlocktype_write);
return (result);
return (ISC_R_SUCCESS);
}
void

View file

@ -101,11 +101,9 @@ typedef struct isc_timerevent {
*** those functions which return an isc_result_t.
***/
isc_result_t
isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type,
const isc_time_t *expires, const isc_interval_t *interval,
isc_task_t *task, isc_taskaction_t action, void *arg,
isc_timer_t **timerp);
void
isc_timer_create(isc_timermgr_t *manager, isc_task_t *task,
isc_taskaction_t action, void *arg, isc_timer_t **timerp);
/*%<
* Create a new 'type' timer managed by 'manager'. The timers parameters
* are specified by 'expires' and 'interval'. Events will be posted to

View file

@ -56,7 +56,6 @@ ratelimiter_shutdowncomplete(isc_task_t *task, isc_event_t *event);
isc_result_t
isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
isc_task_t *task, isc_ratelimiter_t **ratelimiterp) {
isc_result_t result;
isc_ratelimiter_t *rl;
INSIST(ratelimiterp != NULL && *ratelimiterp == NULL);
@ -74,11 +73,7 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
isc_mutex_init(&rl->lock);
result = isc_timer_create(timermgr, isc_timertype_inactive, NULL, NULL,
rl->task, ratelimiter_tick, rl, &rl->timer);
if (result != ISC_R_SUCCESS) {
goto free_mutex;
}
isc_timer_create(timermgr, rl->task, ratelimiter_tick, rl, &rl->timer);
/*
* Increment the reference count to indicate that we may
@ -92,13 +87,6 @@ isc_ratelimiter_create(isc_mem_t *mctx, isc_timermgr_t *timermgr,
*ratelimiterp = rl;
return (ISC_R_SUCCESS);
free_mutex:
isc_refcount_decrementz(&rl->references);
isc_refcount_destroy(&rl->references);
isc_mutex_destroy(&rl->lock);
isc_mem_put(mctx, rl, sizeof(*rl));
return (result);
}
isc_result_t

View file

@ -508,15 +508,17 @@ basic(void **state) {
isc_time_settoepoch(&absolute);
isc_interval_set(&interval, 1, 0);
result = isc_timer_create(timermgr, isc_timertype_ticker, &absolute,
&interval, task1, basic_tick, tick, &ti1);
isc_timer_create(timermgr, task1, basic_tick, tick, &ti1);
result = isc_timer_reset(ti1, isc_timertype_ticker, &absolute,
&interval, false);
assert_int_equal(result, ISC_R_SUCCESS);
ti2 = NULL;
isc_time_settoepoch(&absolute);
isc_interval_set(&interval, 1, 0);
result = isc_timer_create(timermgr, isc_timertype_ticker, &absolute,
&interval, task2, basic_tick, tock, &ti2);
isc_timer_create(timermgr, task2, basic_tick, tock, &ti2);
result = isc_timer_reset(ti2, isc_timertype_ticker, &absolute,
&interval, false);
assert_int_equal(result, ISC_R_SUCCESS);
sleep(2);

View file

@ -130,8 +130,8 @@ setup_test(isc_timertype_t timertype, isc_time_t *expires,
isc_mutex_unlock(&lasttime_mx);
assert_int_equal(result, ISC_R_SUCCESS);
result = isc_timer_create(timermgr, timertype, expires, interval, task,
action, (void *)timertype, &timer);
isc_timer_create(timermgr, task, action, (void *)timertype, &timer);
result = isc_timer_reset(timer, timertype, expires, interval, false);
assert_int_equal(result, ISC_R_SUCCESS);
/*
@ -561,9 +561,9 @@ purge(void **state) {
isc_interval_set(&interval, seconds, 0);
tickertimer = NULL;
result = isc_timer_create(timermgr, isc_timertype_ticker, &expires,
&interval, task1, tick_event, NULL,
&tickertimer);
isc_timer_create(timermgr, task1, tick_event, NULL, &tickertimer);
result = isc_timer_reset(tickertimer, isc_timertype_ticker, &expires,
&interval, false);
assert_int_equal(result, ISC_R_SUCCESS);
oncetimer = NULL;
@ -573,9 +573,9 @@ purge(void **state) {
assert_int_equal(result, ISC_R_SUCCESS);
isc_interval_set(&interval, 0, 0);
result = isc_timer_create(timermgr, isc_timertype_once, &expires,
&interval, task2, once_event, NULL,
&oncetimer);
isc_timer_create(timermgr, task2, once_event, NULL, &oncetimer);
result = isc_timer_reset(oncetimer, isc_timertype_once, &expires,
&interval, false);
assert_int_equal(result, ISC_R_SUCCESS);
/*

View file

@ -219,115 +219,54 @@ destroy(isc_timer_t *timer) {
isc_mem_put(manager->mctx, timer, sizeof(*timer));
}
isc_result_t
isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type,
const isc_time_t *expires, const isc_interval_t *interval,
isc_task_t *task, isc_taskaction_t action, void *arg,
isc_timer_t **timerp) {
void
isc_timer_create(isc_timermgr_t *manager, isc_task_t *task,
isc_taskaction_t action, void *arg, isc_timer_t **timerp) {
REQUIRE(VALID_MANAGER(manager));
REQUIRE(task != NULL);
REQUIRE(action != NULL);
isc_timer_t *timer;
isc_result_t result;
isc_time_t now;
/*
* Create a new 'type' timer managed by 'manager'. The timers
* parameters are specified by 'expires' and 'interval'. Events
* will be posted to 'task' and when dispatched 'action' will be
* called with 'arg' as the arg value. The new timer is returned
* in 'timerp'.
*/
if (expires == NULL) {
expires = isc_time_epoch;
}
if (interval == NULL) {
interval = isc_interval_zero;
}
REQUIRE(type == isc_timertype_inactive ||
!(isc_time_isepoch(expires) && isc_interval_iszero(interval)));
REQUIRE(timerp != NULL && *timerp == NULL);
/*
* Get current time.
*/
if (type != isc_timertype_inactive) {
TIME_NOW(&now);
} else {
/*
* We don't have to do this, but it keeps the compiler from
* complaining about "now" possibly being used without being
* set, even though it will never actually happen.
*/
isc_time_settoepoch(&now);
}
TIME_NOW(&now);
timer = isc_mem_get(manager->mctx, sizeof(*timer));
*timer = (isc_timer_t){
.manager = manager,
.type = isc_timertype_inactive,
.expires = *isc_time_epoch,
.interval = *isc_interval_zero,
.action = action,
.arg = arg,
};
timer->manager = manager;
isc_refcount_init(&timer->references, 1);
if (type == isc_timertype_once && !isc_interval_iszero(interval)) {
result = isc_time_add(&now, interval, &timer->idle);
if (result != ISC_R_SUCCESS) {
isc_mem_put(manager->mctx, timer, sizeof(*timer));
return (result);
}
} else {
isc_time_settoepoch(&timer->idle);
}
isc_time_settoepoch(&timer->idle);
timer->type = type;
timer->expires = *expires;
timer->interval = *interval;
timer->task = NULL;
isc_task_attach(task, &timer->task);
timer->action = action;
/*
* Removing the const attribute from "arg" is the best of two
* evils here. If the timer->arg member is made const, then
* it affects a great many recipients of the timer event
* which did not pass in an "arg" that was truly const.
* Changing isc_timer_create() to not have "arg" prototyped as const,
* though, can cause compilers warnings for calls that *do*
* have a truly const arg. The caller will have to carefully
* keep track of whether arg started as a true const.
*/
DE_CONST(arg, timer->arg);
timer->index = 0;
isc_mutex_init(&timer->lock);
ISC_LINK_INIT(timer, link);
timer->magic = TIMER_MAGIC;
LOCK(&manager->lock);
timer->magic = TIMER_MAGIC;
/*
* Note we don't have to lock the timer like we normally would because
* there are no external references to it yet.
*/
if (type != isc_timertype_inactive) {
result = schedule(timer, &now, true);
} else {
result = ISC_R_SUCCESS;
}
if (result == ISC_R_SUCCESS) {
*timerp = timer;
APPEND(manager->timers, timer, link);
}
*timerp = timer;
LOCK(&manager->lock);
APPEND(manager->timers, timer, link);
UNLOCK(&manager->lock);
if (result != ISC_R_SUCCESS) {
timer->magic = 0;
isc_mutex_destroy(&timer->lock);
isc_task_detach(&timer->task);
isc_mem_put(manager->mctx, timer, sizeof(*timer));
return (result);
}
return (ISC_R_SUCCESS);
}
isc_result_t