diff --git a/bin/named/server.c b/bin/named/server.c index 1114882ba9..fd6b11e7a4 100644 --- a/bin/named/server.c +++ b/bin/named/server.c @@ -10119,10 +10119,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 ed50dea053..1c0dfc1621 100644 --- a/lib/dns/catz.c +++ b/lib/dns/catz.c @@ -978,7 +978,7 @@ dns_catz_zone_detach(dns_catz_zone_t **zonep) { isc_ht_destroy(&zone->coos); } zone->magic = 0; - isc_timer_detach(&zone->updatetimer); + isc_timer_destroy(&zone->updatetimer); if (zone->db_registered) { dns_db_updatenotify_unregister( zone->db, dns_catz_dbupdate_callback, diff --git a/lib/dns/nta.c b/lib/dns/nta.c index 1c7a68dc54..5614eb8aeb 100644 --- a/lib/dns/nta.c +++ b/lib/dns/nta.c @@ -77,7 +77,7 @@ nta_detach(isc_mem_t *mctx, dns_nta_t **ntap) { (void)isc_timer_reset(nta->timer, isc_timertype_inactive, NULL, NULL, true); - isc_timer_detach(&nta->timer); + isc_timer_destroy(&nta->timer); } if (dns_rdataset_isassociated(&nta->rdataset)) { dns_rdataset_disassociate(&nta->rdataset); @@ -293,6 +293,9 @@ settimer(dns_ntatable_t *ntatable, dns_nta_t *nta, uint32_t lifetime) { result = isc_timer_create(ntatable->timermgr, isc_timertype_ticker, NULL, &interval, ntatable->task, checkbogus, nta, &nta->timer); + if (result != ISC_R_SUCCESS) { + isc_timer_destroy(&nta->timer); + } return (result); } @@ -479,7 +482,7 @@ again: (void)isc_timer_reset(nta->timer, isc_timertype_inactive, NULL, 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 880d33b4d3..2cf8a22210 100644 --- a/lib/dns/resolver.c +++ b/lib/dns/resolver.c @@ -4479,7 +4479,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); @@ -5050,7 +5050,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); @@ -10242,7 +10242,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 71340785d6..4ddc6f00fb 100644 --- a/lib/dns/rpz.c +++ b/lib/dns/rpz.c @@ -2231,7 +2231,7 @@ rpz_detach(dns_rpz_zone_t **rpzp) { isc_timer_reset(rpz->updatetimer, isc_timertype_inactive, NULL, 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 556bd65b20..2ee0249b03 100644 --- a/lib/dns/zone.c +++ b/lib/dns/zone.c @@ -15065,7 +15065,7 @@ zone_shutdown(isc_task_t *task, isc_event_t *event) { forward_cancel(zone); if (zone->timer != NULL) { - 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 1e828d6bc3..815cf42dbe 100644 --- a/lib/isc/include/isc/timer.h +++ b/lib/isc/include/isc/timer.h @@ -225,25 +225,9 @@ isc_timer_touch(isc_timer_t *timer); */ 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: * @@ -253,9 +237,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 * @@ -264,9 +245,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 601c133a9e..84a848a8a3 100644 --- a/lib/isc/ratelimiter.c +++ b/lib/isc/ratelimiter.c @@ -242,6 +242,7 @@ void isc_ratelimiter_shutdown(isc_ratelimiter_t *rl) { isc_event_t *ev; isc_task_t *task; + isc_result_t result; REQUIRE(rl != NULL); @@ -257,7 +258,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, NULL, + true); + 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/timer.c b/lib/isc/timer.c index 33daa554cd..11bf197a29 100644 --- a/lib/isc/timer.c +++ b/lib/isc/timer.c @@ -60,9 +60,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_time_t expires; @@ -72,7 +72,6 @@ struct isc_timer { void *arg; unsigned int index; isc_time_t due; - ISC_LIST(isc_timerevent_t) active; LINK(isc_timer_t) link; }; @@ -216,13 +215,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 @@ -230,8 +230,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 @@ -242,25 +244,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)); -} - isc_result_t isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type, const isc_time_t *expires, const isc_interval_t *interval, @@ -310,7 +293,6 @@ isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type, timer = isc_mem_get(manager->mctx, sizeof(*timer)); 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); @@ -495,30 +477,32 @@ isc_timer_touch(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 @@ -533,13 +517,13 @@ timer_post_event(isc_timermgr_t *manager, isc_timer_t *timer, 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)); } diff --git a/tests/isc/task_test.c b/tests/isc/task_test.c index e31b54c77f..973ecc9a99 100644 --- a/tests/isc/task_test.c +++ b/tests/isc/task_test.c @@ -525,8 +525,8 @@ ISC_RUN_TEST_IMPL(basic) { 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/tests/isc/timer_test.c b/tests/isc/timer_test.c index 099a3523be..644018aacb 100644 --- a/tests/isc/timer_test.c +++ b/tests/isc/timer_test.c @@ -236,14 +236,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); } /* @@ -329,9 +329,10 @@ test_idle(isc_task_t *task, isc_event_t *event) { subthread_assert_int_equal(event->ev_type, ISC_TIMEREVENT_IDLE); - 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 */ @@ -415,14 +416,15 @@ test_reset(isc_task_t *task, isc_event_t *event) { &expires, &interval, false); subthread_assert_result_equal(result, ISC_R_SUCCESS); } + + isc_event_free(&event); } else { subthread_assert_int_equal(event->ev_type, ISC_TIMEREVENT_LIFE); - isc_timer_detach(&timer); + isc_event_free(&event); + isc_timer_destroy(&timer); isc_task_shutdown(task); } - - isc_event_free(&event); } ISC_RUN_TEST_IMPL(reset) { @@ -578,8 +580,8 @@ ISC_RUN_TEST_IMPL(purge) { 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); }