Merge branch '3252-repair-isc_task_purgeevent-v9_18' into 'v9_18'

[9.18] Keep the list of scheduled events on the timer

See merge request isc-projects/bind9!7396
This commit is contained in:
Ondřej Surý 2023-01-18 23:32:35 +00:00
commit cb4453faef
15 changed files with 169 additions and 199 deletions

View file

@ -66,6 +66,13 @@
6044. [bug] There was an "RSASHA236" typo in a log message.
[GL !7206]
5845. [bug] Refactor the timer to keep track of posted events
as to use isc_task_purgeevent() instead of using
isc_task_purgerange(). The isc_task_purgeevent()
has been refactored to purge a single event instead
of walking through the list of posted events.
[GL #3252]
5830. [func] Implement incremental resizing of isc_ht hash tables to
perform the rehashing gradually. [GL #3212]

View file

@ -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);

View file

@ -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,

View file

@ -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);

View file

@ -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));
}

View file

@ -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);

View file

@ -2414,6 +2414,9 @@ zone_asyncload(isc_task_t *task, isc_event_t *event) {
(asl->loaded)(asl->loaded_arg, zone, task);
}
/* Reduce the quantum */
isc_task_setquantum(zone->loadtask, 1);
isc_mem_put(zone->mctx, asl, sizeof(*asl));
dns_zone_idetach(&zone);
}
@ -15062,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);
}
@ -19120,7 +19123,7 @@ dns_zonemgr_setsize(dns_zonemgr_t *zmgr, int num_zones) {
pool = NULL;
if (zmgr->loadtasks == NULL) {
result = isc_taskpool_create(zmgr->taskmgr, zmgr->mctx, ntasks,
2, true, &pool);
UINT_MAX, true, &pool);
} else {
result = isc_taskpool_expand(&zmgr->loadtasks, ntasks, true,
&pool);

View file

@ -54,18 +54,6 @@
*
* Purging calls isc_event_free() on the matching events.
*
* Unsending returns a list of events that matched the pattern.
* The caller is then responsible for them.
*
* Consumers of events should purge, not unsend.
*
* Producers of events often want to remove events when the caller indicates
* it is no longer interested in the object, e.g. by canceling a timer.
* Sometimes this can be done by purging, but for some event types, the
* calls to isc_event_free() cause deadlock because the event free routine
* wants to acquire a lock the caller is already holding. Unsending instead
* of purging solves this problem. As a general rule, producers should only
* unsend events which they have sent.
*/
/***
@ -337,34 +325,6 @@ isc_task_purgeevent(isc_task_t *task, isc_event_t *event);
* or was marked unpurgeable.
*/
unsigned int
isc_task_unsendrange(isc_task_t *task, void *sender, isc_eventtype_t first,
isc_eventtype_t last, void *tag, isc_eventlist_t *events);
/*%<
* Remove events from a task's event queue.
*
* Requires:
*
*\li 'task' is a valid task.
*
*\li last >= first.
*
*\li *events is a valid list.
*
* Ensures:
*
*\li Events in the event queue of 'task' whose sender is 'sender', whose
* type is >= first and <= last, and whose tag is 'tag' will be dequeued
* and appended to *events.
*
*\li A sender of NULL will match any sender. A NULL tag matches any
* tag.
*
* Returns:
*
*\li The number of events unsent.
*/
unsigned int
isc_task_unsend(isc_task_t *task, void *sender, isc_eventtype_t type, void *tag,
isc_eventlist_t *events);
@ -373,27 +333,27 @@ isc_task_unsend(isc_task_t *task, void *sender, isc_eventtype_t type, void *tag,
*
* Notes:
*
*\li This function is equivalent to
*\li This function is equivalent to
*
*\code
* isc_task_unsendrange(task, sender, type, type, tag, events);
* isc_task_unsendrange(task, sender, type, type, tag, events);
*\endcode
*
* Requires:
*
*\li 'task' is a valid task.
*\li 'task' is a valid task.
*
*\li *events is a valid list.
*\li *events is a valid list.
*
* Ensures:
*
*\li Events in the event queue of 'task' whose sender is 'sender', whose
* type is 'type', and whose tag is 'tag' will be dequeued and appended
* to *events.
*\li Events in the event queue of 'task' whose sender is 'sender', whose
* type is 'type', and whose tag is 'tag' will be dequeued and appended
* to *events.
*
* Returns:
*
*\li The number of events unsent.
*\li The number of events unsent.
*/
isc_result_t
@ -528,6 +488,15 @@ isc_task_gettag(isc_task_t *task);
*\li 'task' is a valid task.
*/
void
isc_task_setquantum(isc_task_t *task, unsigned int quantum);
/*%<
* Set future 'task' quantum to 'quantum'. The current 'task' quantum will be
* kept for the current isc_task_run() loop, and will be changed for the next
* run. Therefore, the function is save to use from the event callback as it
* will not affect the current event loop processing.
*/
isc_result_t
isc_task_beginexclusive(isc_task_t *task);
/*%<

View file

@ -87,10 +87,13 @@ typedef enum {
isc_timertype_inactive = 3 /*%< Inactive */
} isc_timertype_t;
typedef struct isc_timerevent {
typedef struct isc_timerevent isc_timerevent_t;
struct isc_timerevent {
struct isc_event common;
isc_time_t due;
} isc_timerevent_t;
ISC_LINK(isc_timerevent_t) ev_timerlink;
};
#define ISC_TIMEREVENT_FIRSTEVENT (ISC_EVENTCLASS_TIMER + 0)
#define ISC_TIMEREVENT_TICK (ISC_EVENTCLASS_TIMER + 1)
@ -222,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:
*
@ -250,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
*
@ -261,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
*/

View file

@ -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

View file

@ -614,12 +614,10 @@ isc_task_purge(isc_task_t *task, void *sender, isc_eventtype_t type,
bool
isc_task_purgeevent(isc_task_t *task, isc_event_t *event) {
isc_event_t *curr_event, *next_event;
bool found = false;
/*
* Purge 'event' from a task's event queue.
*
* XXXRTH: WARNING: This method may be removed before beta.
*/
REQUIRE(VALID_TASK(task));
@ -635,40 +633,22 @@ isc_task_purgeevent(isc_task_t *task, isc_event_t *event) {
*/
LOCK(&task->lock);
for (curr_event = HEAD(task->events); curr_event != NULL;
curr_event = next_event)
{
next_event = NEXT(curr_event, ev_link);
if (curr_event == event && PURGE_OK(event)) {
DEQUEUE(task->events, curr_event, ev_link);
task->nevents--;
break;
}
if (ISC_LINK_LINKED(event, ev_link)) {
DEQUEUE(task->events, event, ev_link);
task->nevents--;
found = true;
}
UNLOCK(&task->lock);
if (curr_event == NULL) {
if (!found) {
return (false);
}
isc_event_free(&curr_event);
isc_event_free(&event);
return (true);
}
unsigned int
isc_task_unsendrange(isc_task_t *task, void *sender, isc_eventtype_t first,
isc_eventtype_t last, void *tag, isc_eventlist_t *events) {
/*
* Remove events from a task's event queue.
*/
REQUIRE(VALID_TASK(task));
XTRACE("isc_task_unsendrange");
return (dequeue_events(task, sender, first, last, tag, events, false));
}
unsigned int
isc_task_unsend(isc_task_t *task, void *sender, isc_eventtype_t type, void *tag,
isc_eventlist_t *events) {
@ -781,6 +761,16 @@ isc_task_getnetmgr(isc_task_t *task) {
return (task->manager->netmgr);
}
void
isc_task_setquantum(isc_task_t *task, unsigned int quantum) {
REQUIRE(VALID_TASK(task));
LOCK(&task->lock);
task->quantum = (quantum > 0) ? quantum
: task->manager->default_quantum;
UNLOCK(&task->lock);
}
/***
*** Task Manager.
***/
@ -791,11 +781,13 @@ task_run(isc_task_t *task) {
bool finished = false;
isc_event_t *event = NULL;
isc_result_t result = ISC_R_SUCCESS;
uint32_t quantum;
REQUIRE(VALID_TASK(task));
LOCK(&task->lock);
/* FIXME */
quantum = task->quantum;
if (task->state != task_state_ready) {
goto done;
}
@ -869,7 +861,7 @@ task_run(isc_task_t *task) {
task->state = task_state_idle;
}
break;
} else if (dispatch_count >= task->quantum) {
} else if (dispatch_count >= quantum) {
/*
* Our quantum has expired, but there is more work to be
* done. We'll requeue it to the ready queue later.

View file

@ -1 +0,0 @@
../../tests/isc

View file

@ -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;
@ -205,27 +205,36 @@ deschedule(isc_timer_t *timer) {
}
static void
destroy(isc_timer_t *timer) {
isc_timermgr_t *manager = timer->manager;
timerevent_unlink(isc_timer_t *timer, isc_timerevent_t *event) {
REQUIRE(ISC_LINK_LINKED(event, ev_timerlink));
ISC_LIST_UNLINK(timer->active, event, ev_timerlink);
}
/*
* The caller must ensure it is safe to destroy the timer.
*/
static void
timerevent_destroy(isc_event_t *event0) {
isc_timer_t *timer = event0->ev_destroy_arg;
isc_timerevent_t *event = (isc_timerevent_t *)event0;
LOCK(&manager->lock);
LOCK(&timer->lock);
if (ISC_LINK_LINKED(event, ev_timerlink)) {
/* The event was unlinked via timer_purge() */
timerevent_unlink(timer, event);
}
UNLOCK(&timer->lock);
(void)isc_task_purgerange(timer->task, timer, ISC_TIMEREVENT_FIRSTEVENT,
ISC_TIMEREVENT_LASTEVENT, NULL);
deschedule(timer);
isc_mem_put(timer->manager->mctx, event, event0->ev_size);
}
UNLINK(manager->timers, timer, link);
static void
timer_purge(isc_timer_t *timer) {
isc_timerevent_t *event = NULL;
UNLOCK(&manager->lock);
isc_task_detach(&timer->task);
isc_mutex_destroy(&timer->lock);
timer->magic = 0;
isc_mem_put(manager->mctx, timer, sizeof(*timer));
while ((event = ISC_LIST_HEAD(timer->active)) != NULL) {
timerevent_unlink(timer, event);
UNLOCK(&timer->lock);
(void)isc_task_purgeevent(timer->task, (isc_event_t *)event);
LOCK(&timer->lock);
}
}
isc_result_t
@ -277,7 +286,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);
@ -309,6 +317,9 @@ isc_timer_create(isc_timermgr_t *manager, isc_timertype_t type,
timer->index = 0;
isc_mutex_init(&timer->lock);
ISC_LINK_INIT(timer, link);
ISC_LIST_INIT(timer->active);
timer->magic = TIMER_MAGIC;
LOCK(&manager->lock);
@ -388,9 +399,7 @@ isc_timer_reset(isc_timer_t *timer, isc_timertype_t type,
LOCK(&timer->lock);
if (purge) {
(void)isc_task_purgerange(timer->task, timer,
ISC_TIMEREVENT_FIRSTEVENT,
ISC_TIMEREVENT_LASTEVENT, NULL);
timer_purge(timer);
}
timer->type = type;
timer->expires = *expires;
@ -461,37 +470,60 @@ 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));
timer = *timerp;
*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));
}
void
isc_timer_detach(isc_timer_t **timerp) {
isc_timer_t *timer;
static void
timer_post_event(isc_timermgr_t *manager, isc_timer_t *timer,
isc_eventtype_t type) {
isc_timerevent_t *event;
XTRACEID("posting", timer);
/*
* Detach *timerp from its timer.
*/
event = (isc_timerevent_t *)isc_event_allocate(
manager->mctx, timer, type, timer->action, timer->arg,
sizeof(*event));
REQUIRE(timerp != NULL);
timer = *timerp;
REQUIRE(VALID_TIMER(timer));
ISC_LINK_INIT(event, ev_timerlink);
((isc_event_t *)event)->ev_destroy = timerevent_destroy;
((isc_event_t *)event)->ev_destroy_arg = timer;
if (isc_refcount_decrement(&timer->references) == 1) {
destroy(timer);
}
event->due = timer->due;
*timerp = NULL;
LOCK(&timer->lock);
ISC_LIST_APPEND(timer->active, event, ev_timerlink);
UNLOCK(&timer->lock);
isc_task_send(timer->task, ISC_EVENT_PTR(&event));
}
static void
dispatch(isc_timermgr_t *manager, isc_time_t *now) {
bool done = false, post_event, need_schedule;
isc_timerevent_t *event;
isc_eventtype_t type = 0;
isc_timer_t *timer;
isc_result_t result;
@ -553,23 +585,7 @@ dispatch(isc_timermgr_t *manager, isc_time_t *now) {
}
if (post_event) {
XTRACEID("posting", timer);
/*
* XXX We could preallocate this event.
*/
event = (isc_timerevent_t *)isc_event_allocate(
manager->mctx, timer, type,
timer->action, timer->arg,
sizeof(*event));
if (event != NULL) {
event->due = timer->due;
isc_task_send(timer->task,
ISC_EVENT_PTR(&event));
} else {
UNEXPECTED_ERROR("couldn't allocate "
"event");
}
timer_post_event(manager, timer, type);
}
timer->index = 0;

View file

@ -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);
}
/*
@ -1389,19 +1389,6 @@ ISC_RUN_TEST_IMPL(purgeevent) {
try_purgeevent(true);
}
/*
* Purge event not purgeable test:
* When the event is not marked as purgable, a call to
* isc_task_purgeevent(task, event) does not purge the event
* 'event' from the task's queue and returns false.
*/
ISC_RUN_TEST_IMPL(purgeevent_notpurge) {
UNUSED(state);
try_purgeevent(false);
}
ISC_TEST_LIST_START
ISC_TEST_ENTRY_CUSTOM(manytasks, _setup4, _teardown)
@ -1413,7 +1400,6 @@ ISC_TEST_ENTRY_CUSTOM(privilege_drop, _setup, _teardown)
ISC_TEST_ENTRY_CUSTOM(privileged_events, _setup, _teardown)
ISC_TEST_ENTRY_CUSTOM(purge, _setup2, _teardown)
ISC_TEST_ENTRY_CUSTOM(purgeevent, _setup2, _teardown)
ISC_TEST_ENTRY_CUSTOM(purgeevent_notpurge, _setup2, _teardown)
ISC_TEST_ENTRY_CUSTOM(task_shutdown, _setup4, _teardown)
ISC_TEST_ENTRY_CUSTOM(task_exclusive, _setup4, _teardown)

View file

@ -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);
}