diff --git a/bin/named/server.c b/bin/named/server.c index f720cb0016..fa327e8da0 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -9893,10 +9893,10 @@ shutdown_server(isc_task_t *task, isc_event_t *event) { isc_mem_put(server->mctx, nsc, sizeof(*nsc)); } - isc_timer_detach(&server->interface_timer); - isc_timer_detach(&server->heartbeat_timer); - isc_timer_detach(&server->pps_timer); - isc_timer_detach(&server->tat_timer); + isc_timer_destroy(&server->interface_timer); + isc_timer_destroy(&server->heartbeat_timer); + isc_timer_destroy(&server->pps_timer); + isc_timer_destroy(&server->tat_timer); ns_interfacemgr_detach(&server->interfacemgr); diff --git a/lib/dns/catz.c b/lib/dns/catz.c index 9be7b91ed8..bd1e76b1a5 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -765,7 +765,7 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) { isc_ht_destroy(&zone->entries); } zone->magic = 0; - isc_timer_detach(&zone->updatetimer); + isc_timer_destroy(&zone->updatetimer); if (zone->db_registered) { INSIST(dns_db_updatenotify_unregister( zone->db, dns_catz_dbupdate_callback, diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 49f4ddef47..c5d3d905a4 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -76,7 +76,7 @@ nta_detach(isc_mem_t *mctx, dns_nta_t **ntap) { if (nta->timer != NULL) { (void)isc_timer_reset( nta->timer, isc_timertype_inactive, NULL, true); - isc_timer_detach(&nta->timer); + isc_timer_destroy(&nta->timer); } if (dns_rdataset_isassociated(&nta->rdataset)) { dns_rdataset_disassociate(&nta->rdataset); @@ -294,7 +294,7 @@ settimer(dns_ntatable_t *ntatable, dns_nta_t *nta, uint32_t lifetime) { result = isc_timer_reset(nta->timer, isc_timertype_ticker, &interval, false); if (result != ISC_R_SUCCESS) { - isc_timer_detach(&nta->timer); + isc_timer_destroy(&nta->timer); } return (result); } @@ -481,7 +481,7 @@ again: if (nta->timer != NULL) { (void)isc_timer_reset( nta->timer, isc_timertype_inactive, NULL, true); - isc_timer_detach(&nta->timer); + isc_timer_destroy(&nta->timer); } result = deletenode(ntatable, foundname); diff --git a/lib/dns/resolver.c b/lib/dns/resolver.c index 53f3c2fa92..c8724f68ba 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4379,7 +4379,7 @@ fctx_destroy(fetchctx_t *fctx, bool exiting) { dns_db_detach(&fctx->cache); dns_adb_detach(&fctx->adb); - isc_timer_detach(&fctx->timer); + isc_timer_destroy(&fctx->timer); dns_resolver_detach(&fctx->res); @@ -4927,7 +4927,7 @@ cleanup_mctx: dns_db_detach(&fctx->cache); cleanup_timer: - isc_timer_detach(&fctx->timer); + isc_timer_destroy(&fctx->timer); cleanup_qmessage: dns_message_detach(&fctx->qmessage); @@ -10118,7 +10118,7 @@ destroy(dns_resolver_t *res) { dns_resolver_reset_ds_digests(res); dns_badcache_destroy(&res->badcache); dns_resolver_resetmustbesecure(res); - isc_timer_detach(&res->spillattimer); + isc_timer_destroy(&res->spillattimer); res->magic = 0; isc_mem_putanddetach(&res->mctx, res, sizeof(*res)); } diff --git a/lib/dns/rpz.c b/lib/dns/rpz.c index d0feec756e..b62c20d107 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -2189,7 +2189,7 @@ rpz_detach(dns_rpz_zone_t **rpzp) { isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, true); - isc_timer_detach(&rpz->updatetimer); + isc_timer_destroy(&rpz->updatetimer); isc_ht_destroy(&rpz->nodes); diff --git a/lib/dns/zone.c b/lib/dns/zone.c index ec6c78030f..76c2554b1a 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -15088,8 +15088,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) { forward_cancel(zone); if (zone->timer != NULL) { - zone_timer_stop(zone); - isc_timer_detach(&zone->timer); + isc_timer_destroy(&zone->timer); isc_refcount_decrement(&zone->irefs); } diff --git a/lib/isc/include/isc/timer.h b/lib/isc/include/isc/timer.h index cbe2372878..132cc67bf6 100644 --- a/lib/isc/include/isc/timer.h +++ b/lib/isc/include/isc/timer.h @@ -182,25 +182,9 @@ isc_timer_reset(isc_timer_t *timer, isc_timertype_t type, */ void -isc_timer_attach(isc_timer_t *timer, isc_timer_t **timerp); +isc_timer_destroy(isc_timer_t **timerp); /*%< - * Attach *timerp to timer. - * - * Requires: - * - *\li 'timer' is a valid timer. - * - *\li 'timerp' points to a NULL timer. - * - * Ensures: - * - *\li *timerp is attached to timer. - */ - -void -isc_timer_detach(isc_timer_t **timerp); -/*%< - * Detach *timerp from its timer. + * Destroy *timerp. * * Requires: * @@ -210,9 +194,6 @@ isc_timer_detach(isc_timer_t **timerp); * *\li *timerp is NULL. * - *\li If '*timerp' is the last reference to the timer, - * then: - * *\code * The timer will be shutdown * @@ -221,9 +202,13 @@ isc_timer_detach(isc_timer_t **timerp); * All resources used by the timer have been freed * * Any events already posted by the timer will be purged. - * Therefore, if isc_timer_detach() is called in the context + * Therefore, if isc_timer_destroy() is called in the context * of the timer's task, it is guaranteed that no more * timer event callbacks will run after the call. + * + * If this function is called from the timer event callback + * the event itself must be destroyed before the timer + * itself. *\endcode */ diff --git a/lib/isc/ratelimiter.c b/lib/isc/ratelimiter.c index ceab020503..4a6bc909c7 100644 --- a/lib/isc/ratelimiter.c +++ b/lib/isc/ratelimiter.c @@ -229,6 +229,7 @@ void isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { isc_event_t *ev; isc_task_t *task; + isc_result_t result; REQUIRE(rl != NULL); @@ -243,7 +244,11 @@ isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { } task = NULL; isc_task_attach(rl->task, &task); - isc_timer_detach(&rl->timer); + + result = isc_timer_reset(rl->timer, isc_timertype_inactive, NULL, + false); + RUNTIME_CHECK(result == ISC_R_SUCCESS); + isc_timer_destroy(&rl->timer); /* * Send an event to our task. The delivery of this event diff --git a/lib/isc/tests/task_test.c b/lib/isc/tests/task_test.c index f71a8ee9c1..a7cffbd8bb 100644 --- a/lib/isc/tests/task_test.c +++ b/lib/isc/tests/task_test.c @@ -304,8 +304,8 @@ basic(void **state) { isc_task_detach(&task4); sleep(10); - isc_timer_detach(&ti1); - isc_timer_detach(&ti2); + isc_timer_destroy(&ti1); + isc_timer_destroy(&ti2); } /* diff --git a/lib/isc/tests/timer_test.c b/lib/isc/tests/timer_test.c index a41f7b8352..75c30851fb 100644 --- a/lib/isc/tests/timer_test.c +++ b/lib/isc/tests/timer_test.c @@ -241,14 +241,14 @@ ticktock(isc_task_t *task, isc_event_t *event) { isc_mutex_unlock(&lasttime_mx); subthread_assert_result_equal(result, ISC_R_SUCCESS); + isc_event_free(&event); + if (atomic_load(&eventcnt) == nevents) { result = isc_time_now(&endtime); subthread_assert_result_equal(result, ISC_R_SUCCESS); - isc_timer_detach(&timer); + isc_timer_destroy(&timer); isc_task_shutdown(task); } - - isc_event_free(&event); } /* @@ -312,9 +312,10 @@ test_idle(isc_task_t *task, isc_event_t *event) { subthread_assert_int_equal(event->ev_type, ISC_TIMEREVENT_ONCE); - isc_timer_detach(&timer); - isc_task_shutdown(task); isc_event_free(&event); + + isc_timer_destroy(&timer); + isc_task_shutdown(task); } /* timer type once idles out */ @@ -387,14 +388,15 @@ test_reset(isc_task_t *task, isc_event_t *event) { &interval, false); subthread_assert_result_equal(result, ISC_R_SUCCESS); } + + isc_event_free(&event); } else { subthread_assert_int_equal(event->ev_type, ISC_TIMEREVENT_ONCE); - isc_timer_detach(&timer); + isc_event_free(&event); + isc_timer_destroy(&timer); isc_task_shutdown(task); } - - isc_event_free(&event); } static void @@ -545,8 +547,8 @@ purge(void **state) { assert_int_equal(atomic_load(&eventcnt), 1); - isc_timer_detach(&tickertimer); - isc_timer_detach(&oncetimer); + isc_timer_destroy(&tickertimer); + isc_timer_destroy(&oncetimer); isc_task_destroy(&task1); isc_task_destroy(&task2); } diff --git a/lib/isc/timer.c b/lib/isc/timer.c index 6ecb1b4c1b..b2f6c0b1c1 100644 --- a/lib/isc/timer.c +++ b/lib/isc/timer.c @@ -59,9 +59,9 @@ struct isc_timer { unsigned int magic; isc_timermgr_t *manager; isc_mutex_t lock; - isc_refcount_t references; /*! Locked by timer lock. */ isc_time_t idle; + ISC_LIST(isc_timerevent_t) active; /*! Locked by manager lock. */ isc_timertype_t type; isc_interval_t interval; @@ -70,7 +70,6 @@ struct isc_timer { void *arg; unsigned int index; isc_time_t due; - ISC_LIST(isc_timerevent_t) active; LINK(isc_timer_t) link; }; @@ -201,13 +200,14 @@ timerevent_destroy(isc_event_t *event0) { isc_timer_t *timer = event0->ev_destroy_arg; isc_timerevent_t *event = (isc_timerevent_t *)event0; + LOCK(&timer->lock); if (ISC_LINK_LINKED(event, ev_timerlink)) { /* The event was unlinked via timer_purge() */ timerevent_unlink(timer, event); } + UNLOCK(&timer->lock); isc_mem_put(timer->manager->mctx, event, event0->ev_size); - isc_timer_detach(&timer); } static void @@ -215,8 +215,10 @@ timer_purge(isc_timer_t *timer) { isc_timerevent_t *event = NULL; while ((event = ISC_LIST_HEAD(timer->active)) != NULL) { + UNLOCK(&timer->lock); bool purged = isc_task_purgeevent(timer->task, (isc_event_t *)event); + LOCK(&timer->lock); if (!purged) { /* * The event has already been executed, but not @@ -227,25 +229,6 @@ timer_purge(isc_timer_t *timer) { } } -static void -destroy(isc_timer_t *timer) { - isc_timermgr_t *manager = timer->manager; - - LOCK(&manager->lock); - - timer_purge(timer); - deschedule(timer); - - UNLINK(manager->timers, timer, link); - - UNLOCK(&manager->lock); - - isc_task_detach(&timer->task); - isc_mutex_destroy(&timer->lock); - timer->magic = 0; - isc_mem_put(manager->mctx, timer, sizeof(*timer)); -} - void isc_timer_create(isc_timermgr_t *manager, isc_task_t *task, isc_taskaction_t action, void *arg, isc_timer_t **timerp) { @@ -272,8 +255,6 @@ isc_timer_create(isc_timermgr_t *manager, isc_task_t *task, .arg = arg, }; - isc_refcount_init(&timer->references, 1); - isc_time_settoepoch(&timer->idle); isc_task_attach(task, &timer->task); @@ -378,30 +359,32 @@ isc_timer_gettype(isc_timer_t *timer) { } void -isc_timer_attach(isc_timer_t *timer, isc_timer_t **timerp) { - REQUIRE(VALID_TIMER(timer)); - REQUIRE(timerp != NULL && *timerp == NULL); - isc_refcount_increment(&timer->references); +isc_timer_destroy(isc_timer_t **timerp) { + isc_timer_t *timer = NULL; + isc_timermgr_t *manager = NULL; - *timerp = timer; -} + REQUIRE(timerp != NULL && VALID_TIMER(*timerp)); -void -isc_timer_detach(isc_timer_t **timerp) { - isc_timer_t *timer; - /* - * Detach *timerp from its timer. - */ - - REQUIRE(timerp != NULL); timer = *timerp; - REQUIRE(VALID_TIMER(timer)); - - if (isc_refcount_decrement(&timer->references) == 1) { - destroy(timer); - } - *timerp = NULL; + + manager = timer->manager; + + LOCK(&manager->lock); + + LOCK(&timer->lock); + timer_purge(timer); + deschedule(timer); + UNLOCK(&timer->lock); + + UNLINK(manager->timers, timer, link); + + UNLOCK(&manager->lock); + + isc_task_detach(&timer->task); + isc_mutex_destroy(&timer->lock); + timer->magic = 0; + isc_mem_put(manager->mctx, timer, sizeof(*timer)); } static void @@ -415,13 +398,13 @@ post_event(isc_timermgr_t *manager, isc_timer_t *timer, isc_eventtype_t type) { ISC_LINK_INIT(event, ev_timerlink); ((isc_event_t *)event)->ev_destroy = timerevent_destroy; - - isc_timer_attach(timer, &(isc_timer_t *){ NULL }); ((isc_event_t *)event)->ev_destroy_arg = timer; event->due = timer->due; + LOCK(&timer->lock); ISC_LIST_APPEND(timer->active, event, ev_timerlink); + UNLOCK(&timer->lock); isc_task_send(timer->task, ISC_EVENT_PTR(&event)); }