From d51399d07caa3c7c7207ecba5cce9ffeec825ed4 Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 19:00:34 +0000 Subject: [PATCH 1/7] Fixed timer lifetime after reset and deletion --- event/hevent.h | 24 ++++++++++++------------ event/hloop.c | 9 ++++++++- 2 files changed, 20 insertions(+), 13 deletions(-) diff --git a/event/hevent.h b/event/hevent.h index 92c69070e..d384fba5e 100644 --- a/event/hevent.h +++ b/event/hevent.h @@ -106,11 +106,12 @@ struct hidle_s { struct list_node node; }; -#define HTIMER_FIELDS \ - HEVENT_FIELDS \ - uint32_t repeat; \ - uint64_t next_timeout; \ - struct heap_node node; +#define HTIMER_FIELDS \ + HEVENT_FIELDS \ + uint32_t repeat; \ + uint64_t next_timeout; \ + struct heap_node node; \ + unsigned pending_reset : 1; struct htimer_s { HTIMER_FIELDS @@ -313,13 +314,12 @@ void hio_memmove_readbuf(hio_t* io); }\ } while(0) -#define EVENT_RESET(ev) \ - do {\ - ev->destroy = 0;\ - EVENT_ACTIVE(ev);\ - ev->pending = 0;\ - } while(0) - +#define EVENT_RESET(ev) \ + do { \ + ev->destroy = 0; \ + EVENT_ACTIVE(ev); \ + } while (0) + #define EVENT_UNPENDING(ev) \ do {\ if (ev->pending) {\ diff --git a/event/hloop.c b/event/hloop.c index b12802435..e7156d6e3 100644 --- a/event/hloop.c +++ b/event/hloop.c @@ -118,10 +118,15 @@ static int hloop_process_pendings(hloop_t* loop) { while (cur) { next = cur->pending_next; if (cur->pending && cur->loop == loop) { - if (cur->active && cur->cb) { + // A reset cancels delivery, not the pending allocation reference. + bool reset_timer = (cur->event_type & HEVENT_TYPE_TIMER) && ((htimer_t*)cur)->pending_reset; + if (cur->active && cur->cb && !reset_timer) { cur->cb(cur); ++ncbs; } + if (cur->event_type & HEVENT_TYPE_TIMER) { + ((htimer_t*)cur)->pending_reset = 0; + } cur->pending = 0; // NOTE: Now we can safely delete event marked as destroy. if (cur->destroy) { @@ -756,6 +761,8 @@ void htimer_reset(htimer_t* timer, uint32_t timeout_ms) { timer->next_timeout = timer->next_timeout / 100000 * 100000; } heap_insert(&loop->timers, &timer->node); + // Keep pending ownership until dispatch releases its current/next pointer. + timer->pending_reset = timer->pending; EVENT_RESET(timer); } From e207033ae836c82f979f6cf86b3f14f6bba26450 Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:43:59 +0000 Subject: [PATCH 2/7] Added timer reset lifetime regression tests --- Makefile | 1 + scripts/unittest.sh | 3 + unittest/CMakeLists.txt | 8 +- unittest/timer_reset_test.c | 956 ++++++++++++++++++++++++++++++++++++ 4 files changed, 967 insertions(+), 1 deletion(-) create mode 100644 unittest/timer_reset_test.c diff --git a/Makefile b/Makefile index 34817430b..2b1ecd50c 100644 --- a/Makefile +++ b/Makefile @@ -409,6 +409,7 @@ unittest: prepare libhv $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Iprotocol -o bin/ping unittest/ping_test.c protocol/icmp.c base/hsocket.c base/htime.c -DPRINT_DEBUG $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Iprotocol -o bin/ftp unittest/ftp_test.c protocol/ftp.c base/hsocket.c base/htime.c $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Iprotocol -Iutil -o bin/sendmail unittest/sendmail_test.c protocol/smtp.c base/hsocket.c base/htime.c util/base64.c + $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Issl -Ievent -o bin/timer_reset_test unittest/timer_reset_test.c -Llib -lhv -pthread $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Issl -Ievent -o bin/hdns_test unittest/hdns_test.c -Llib -lhv -pthread $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Issl -Ievent -o bin/hdns_benchmark unittest/hdns_benchmark.c -Llib -lhv -pthread ifeq ($(WITH_EVPP), yes) diff --git a/scripts/unittest.sh b/scripts/unittest.sh index ca1046e8d..6c8c58091 100755 --- a/scripts/unittest.sh +++ b/scripts/unittest.sh @@ -32,6 +32,9 @@ bin/socketpair_test # bin/objectpool_test bin/sizeof_test bin/http_router_test +if [ -x bin/timer_reset_test ]; then + bin/timer_reset_test || exit $? +fi if [ -x bin/websocket_parser_test ]; then bin/websocket_parser_test || exit $? fi diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 6e17408b3..42a829fff 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -168,9 +168,15 @@ if(WIN32 AND WITH_WEPOLL) add_executable(epoll_init_test epoll_init_test.c ../event/wepoll/wepoll.c) target_include_directories(epoll_init_test PRIVATE .. ../base ../ssl ../event) target_link_libraries(epoll_init_test ${HV_LIBRARIES}) -set(EVENT_UNITTEST_TARGETS epoll_init_test) +list(APPEND EVENT_UNITTEST_TARGETS epoll_init_test) endif() +# ------event: timer reset and lifetime------ +add_executable(timer_reset_test timer_reset_test.c) +target_include_directories(timer_reset_test PRIVATE .. ../base ../ssl ../event) +target_link_libraries(timer_reset_test ${HV_LIBRARIES}) +list(APPEND EVENT_UNITTEST_TARGETS timer_reset_test) + # ------event: async dns------ add_executable(hdns_test hdns_test.c) target_include_directories(hdns_test PRIVATE .. ../base ../ssl ../event) diff --git a/unittest/timer_reset_test.c b/unittest/timer_reset_test.c new file mode 100644 index 000000000..5049eaf2e --- /dev/null +++ b/unittest/timer_reset_test.c @@ -0,0 +1,956 @@ +/* + * Unit tests for timer reset and lifetime handling in libhv. + * + * Covers: + * 1. Resetting a timer that is not due. + * 2. Resetting an expiration already waiting for dispatch. + * 3. Rearming from inside the timer's own callback. + * 4. Cancelling a timer after resetting it in its own callback. + * 5. Cancelling another pending timer after resetting it (both different and same priority queue). + * 6. Repeated reset operations before dispatch settles (both keep and cancel). + * 7. Repeat-timer behavior (one-shot, multiple repetitions, and repeating self-rearm). + * 8. Replacement allocation and cleanup during callback. + */ + +#include "hevent.h" +#include +#include +#include +#include +#include + +#define require(expr) \ + do { \ + if (!(expr)) { \ + fprintf(stderr, "REQUIRE FAILED at %s:%d: %s\n", __FILE__, __LINE__, #expr); \ + exit(EXIT_FAILURE); \ + } \ + } while (0) + +#define require_msg(expr, msg) \ + do { \ + if (!(expr)) { \ + fprintf(stderr, "REQUIRE FAILED at %s:%d: %s (%s)\n", __FILE__, __LINE__, #expr, msg); \ + exit(EXIT_FAILURE); \ + } \ + } while (0) + +static void make_timer_due(hloop_t* loop, htimer_t* timer, uint64_t due_time) { + heap_remove(&loop->timers, &timer->node); + timer->next_timeout = due_time; + heap_insert(&loop->timers, &timer->node); +} + +/* Set deadline shortly before current loop time so the timer is due without + * triggering repeated catch-up iterations on repeating timers. */ +static void make_timer_due_now(hloop_t* loop, htimer_t* timer) { + hloop_update_time(loop); + uint64_t now = loop->cur_hrtime; + uint64_t due = now > 1000 ? now - 1000 : 0; + make_timer_due(loop, timer, due); +} + +/* Set deadline shortly before current loop time with an explicit microsecond + * offset to enforce deterministic minheap ordering. */ +static void make_timer_due_ordered(hloop_t* loop, htimer_t* timer, uint64_t offset_us) { + hloop_update_time(loop); + uint64_t now = loop->cur_hrtime; + uint64_t base = now > 10000 ? now - 10000 : 0; + make_timer_due(loop, timer, base + offset_us); +} + +/* ========================================================================= */ +/* Case 1: Reset a timer that is not due */ +/* ========================================================================= */ + +static int s_case1_target_calls = 0; +static int s_case1_unrelated_calls = 0; + +static void case1_target_cb(htimer_t* timer) { + (void)timer; + s_case1_target_calls++; +} + +static void case1_unrelated_cb(htimer_t* timer) { + (void)timer; + s_case1_unrelated_calls++; +} + +static void test_reset_timer_not_due(void) { + s_case1_target_calls = 0; + s_case1_unrelated_calls = 0; + + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + htimer_t* unrelated = htimer_add(loop, case1_unrelated_cb, 50000, 1); + require(unrelated != NULL); + + htimer_t* target = htimer_add(loop, case1_target_cb, 20000, 1); + require(target != NULL); + + require(hloop_ntimers(loop) == 2); + require(hloop_nactives(loop) == 2); + uint64_t old_target_deadline = target->next_timeout; + uint64_t unrelated_deadline = unrelated->next_timeout; + + // Reset target to a new, further deadline (40000 ms) + htimer_reset(target, 40000); + + // Reset does not create an additional timer or change unrelated timers + require(hloop_ntimers(loop) == 2); + require(hloop_nactives(loop) == 2); + // Unrelated timer deadline remains unchanged + require(unrelated->next_timeout == unrelated_deadline); + // Target replacement deadline matches the newly requested timeout + require(((htimeout_t*)target)->timeout == 40000); + require(target->next_timeout > old_target_deadline); + require(target->next_timeout >= old_target_deadline + 20000000ULL - 100000ULL); + require(!target->destroy); + require(!target->pending); + require(!target->pending_reset); + + // Step 1: Check non-delivery while deadline is in the future. + // Neither timer is due. + hloop_process_events(loop, 0); + require(s_case1_target_calls == 0); + require(s_case1_unrelated_calls == 0); + require(hloop_ntimers(loop) == 2); + require(hloop_nactives(loop) == 2); + + // Step 2: Advance clock to replacement deadline; target fires while unrelated does not. + make_timer_due_now(loop, target); + hloop_process_events(loop, 0); + require(s_case1_target_calls == 1); + require(s_case1_unrelated_calls == 0); + require(hloop_ntimers(loop) == 1); + require(hloop_nactives(loop) == 1); + + // Step 3: Advance clock for unrelated timer; unrelated fires. + make_timer_due_now(loop, unrelated); + hloop_process_events(loop, 0); + require(s_case1_unrelated_calls == 1); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 2: Reset an expiration already waiting for dispatch */ +/* ========================================================================= */ + +struct case2_ctx { + htimer_t* ctrl; + htimer_t* target; + htimer_t* unrelated; + int ctrl_calls; + int target_calls; + int unrelated_calls; + bool target_was_pending_in_ctrl; + bool target_pending_after_reset; + bool target_pending_reset_after_reset; +}; + +static void case2_ctrl_cb(htimer_t* timer) { + struct case2_ctx* ctx = (struct case2_ctx*)timer->userdata; + ctx->ctrl_calls++; + + // Target expiration is waiting for dispatch + ctx->target_was_pending_in_ctrl = ctx->target->pending; + require(ctx->target->pending == 1); + require(ctx->unrelated->pending == 1); + + // Reset target timer to a future deadline + htimer_reset(ctx->target, 10000); + + // Lifetime reference is preserved; pending_reset flag is set + ctx->target_pending_after_reset = ctx->target->pending; + ctx->target_pending_reset_after_reset = ctx->target->pending_reset; + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + require(ctx->target->active == 1); + require(ctx->target->destroy == 0); +} + +static void case2_target_cb(htimer_t* timer) { + struct case2_ctx* ctx = (struct case2_ctx*)timer->userdata; + ctx->target_calls++; +} + +static void case2_unrelated_cb(htimer_t* timer) { + struct case2_ctx* ctx = (struct case2_ctx*)timer->userdata; + ctx->unrelated_calls++; +} + +static void test_reset_pending_expiration(void) { + struct case2_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + ctx.ctrl = htimer_add(loop, case2_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case2_target_cb, 1000, 1); + ctx.unrelated = htimer_add(loop, case2_unrelated_cb, 1000, 1); + require(ctx.ctrl && ctx.target && ctx.unrelated); + + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + ctx.unrelated->userdata = &ctx; + + // Priorities ensure ctrl runs first, then target would run, then unrelated + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + hevent_set_priority(ctx.unrelated, HEVENT_LOWEST_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + make_timer_due_now(loop, ctx.unrelated); + + // First dispatch + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_was_pending_in_ctrl); + require(ctx.target_pending_after_reset); + require(ctx.target_pending_reset_after_reset); + require(ctx.target_calls == 0); // old due occurrence suppressed + require(ctx.unrelated_calls == 1); // unrelated due timer ran exactly once + + // Old pending reference released after dispatch completes + require(ctx.target->pending == 0); + require(ctx.target->pending_reset == 0); + require(ctx.target->active == 1); // target remains scheduled + require(ctx.target->destroy == 0); + require(hloop_ntimers(loop) == 1); + require(hloop_nactives(loop) == 1); + + // Advance clock to replacement deadline + make_timer_due_now(loop, ctx.target); + hloop_process_events(loop, 0); + + require(ctx.target_calls == 1); // fires at replacement deadline + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 3: Rearm from inside the timer's own callback */ +/* ========================================================================= */ + +struct case3_ctx { + int calls; + bool pending_in_first_cb; + bool pending_reset_after_rearm; + bool pending_after_rearm; +}; + +static void case3_cb(htimer_t* timer) { + struct case3_ctx* ctx = (struct case3_ctx*)timer->userdata; + ctx->calls++; + if (ctx->calls == 1) { + ctx->pending_in_first_cb = timer->pending; + require(timer->pending == 1); + htimer_reset(timer, 5000); + ctx->pending_after_rearm = timer->pending; + ctx->pending_reset_after_rearm = timer->pending_reset; + require(timer->pending == 1); + require(timer->pending_reset == 1); + require(timer->active == 1); + require(timer->destroy == 0); + } +} + +static void test_rearm_inside_own_callback(void) { + struct case3_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + uint32_t init_timers = hloop_ntimers(loop); + uint32_t init_actives = hloop_nactives(loop); + + htimer_t* timer = htimer_add(loop, case3_cb, 1000, 1); + require(timer != NULL); + timer->userdata = &ctx; + + make_timer_due_now(loop, timer); + + // First dispatch: callback fires, rearms itself + hloop_process_events(loop, 0); + + require(ctx.calls == 1); // first callback completes once + require(ctx.pending_in_first_cb); + require(ctx.pending_after_rearm); + require(ctx.pending_reset_after_rearm); + + // After dispatch finishes, old occurrence marker must be cleared + require(timer->pending == 0); + require(timer->pending_reset == 0); + require(timer->active == 1); + require(timer->destroy == 0); + require(hloop_ntimers(loop) == init_timers + 1); + require(hloop_nactives(loop) == init_actives + 1); + + // Second occurrence: make due and dispatch + make_timer_due_now(loop, timer); + hloop_process_events(loop, 0); + + require(ctx.calls == 2); // next occurrence delivered once + require(hloop_ntimers(loop) == init_timers); + require(hloop_nactives(loop) == init_actives); + require(loop->npendings == 0); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 4: Cancel a timer after resetting it in its own callback */ +/* ========================================================================= */ + +struct case4_ctx { + int calls; + long alloc_before; + long alloc_in_cb_after_del; + bool pending_before_del; +}; + +static void case4_cb(htimer_t* timer) { + struct case4_ctx* ctx = (struct case4_ctx*)timer->userdata; + ctx->calls++; + + require(timer->pending == 1); + htimer_reset(timer, 5000); + require(timer->pending == 1); + require(timer->pending_reset == 1); + + ctx->pending_before_del = timer->pending; + + // Delete timer after resetting it + htimer_del(timer); + + // Memory must still be valid; dispatch still owns the pending reference + ctx->alloc_in_cb_after_del = hv_alloc_cnt() - hv_free_cnt(); + // Application does NOT access timer after htimer_del +} + +static void test_cancel_after_rearm_in_own_callback(void) { + struct case4_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + ctx.alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + htimer_t* timer = htimer_add(loop, case4_cb, 1000, 1); + require(timer != NULL); + timer->userdata = &ctx; + + require(hv_alloc_cnt() - hv_free_cnt() == ctx.alloc_before + 1); + + make_timer_due_now(loop, timer); + + hloop_process_events(loop, 0); + + require(ctx.calls == 1); + require(ctx.pending_before_del); + // In callback, alloc count was still ctx.alloc_before + 1 (not freed prematurely) + require(ctx.alloc_in_cb_after_del == ctx.alloc_before + 1); + + // Now dispatch has finished: allocation released exactly once + require(hv_alloc_cnt() - hv_free_cnt() == ctx.alloc_before); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(loop->npendings == 0); + + // No later callback is delivered + hloop_process_events(loop, 0); + require(ctx.calls == 1); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 5: Cancel another pending timer after resetting it */ +/* ========================================================================= */ + +struct case5_ctx { + htimer_t* ctrl; + htimer_t* target; + htimer_t* unrelated; + int ctrl_calls; + int target_calls; + int unrelated_calls; + long alloc_before; + long alloc_in_ctrl_after_del; +}; + +static void case5_ctrl_cb(htimer_t* timer) { + struct case5_ctx* ctx = (struct case5_ctx*)timer->userdata; + ctx->ctrl_calls++; + + require(ctx->target->pending == 1); + htimer_reset(ctx->target, 5000); + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + + htimer_del(ctx->target); + // Target is still referenced by dispatch; must not be freed yet + ctx->alloc_in_ctrl_after_del = hv_alloc_cnt() - hv_free_cnt(); +} + +static void case5_target_cb(htimer_t* timer) { + struct case5_ctx* ctx = (struct case5_ctx*)timer->userdata; + ctx->target_calls++; +} + +static void case5_unrelated_cb(htimer_t* timer) { + struct case5_ctx* ctx = (struct case5_ctx*)timer->userdata; + ctx->unrelated_calls++; +} + +// 5a: Target in a later priority queue +static void test_cancel_pending_timer_different_priority(void) { + struct case5_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + ctx.alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.ctrl = htimer_add(loop, case5_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case5_target_cb, 1000, 1); + ctx.unrelated = htimer_add(loop, case5_unrelated_cb, 1000, 1); + require(ctx.ctrl && ctx.target && ctx.unrelated); + + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + ctx.unrelated->userdata = &ctx; + + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + hevent_set_priority(ctx.unrelated, HEVENT_LOWEST_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + make_timer_due_now(loop, ctx.unrelated); + + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); // neither old nor replacement runs + require(ctx.unrelated_calls == 1); // other pending callbacks remain intact + + require(ctx.alloc_in_ctrl_after_del == ctx.alloc_before + 3); + require(hv_alloc_cnt() - hv_free_cnt() == ctx.alloc_before); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(loop->npendings == 0); + + hloop_process_events(loop, 0); + require(ctx.target_calls == 0); + + hloop_free(&loop); +} + +// 5b: Target in the SAME priority queue (dispatcher already cached next pointer) +static void test_cancel_pending_timer_same_priority(void) { + struct case5_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + ctx.alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.unrelated = htimer_add(loop, case5_unrelated_cb, 1000, 1); + ctx.target = htimer_add(loop, case5_target_cb, 1000, 1); + ctx.ctrl = htimer_add(loop, case5_ctrl_cb, 1000, 1); + require(ctx.ctrl && ctx.target && ctx.unrelated); + + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + ctx.unrelated->userdata = &ctx; + + hevent_set_priority(ctx.unrelated, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + + // Heap order: unrelated dequeued first, target second, ctrl third. + // Resulting pendings order (LIFO): ctrl -> target -> unrelated. + make_timer_due_ordered(loop, ctx.unrelated, 1); + make_timer_due_ordered(loop, ctx.target, 2); + make_timer_due_ordered(loop, ctx.ctrl, 3); + + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); + require(ctx.unrelated_calls == 1); + + require(ctx.alloc_in_ctrl_after_del == ctx.alloc_before + 3); + require(hv_alloc_cnt() - hv_free_cnt() == ctx.alloc_before); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(loop->npendings == 0); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 6: Repeat reset operations before dispatch settles */ +/* ========================================================================= */ + +struct case6_ctx { + htimer_t* ctrl; + htimer_t* target; + int ctrl_calls; + int target_calls; + bool cancel_at_end; +}; + +static void case6_ctrl_cb(htimer_t* timer) { + struct case6_ctx* ctx = (struct case6_ctx*)timer->userdata; + ctx->ctrl_calls++; + + require(ctx->target->pending == 1); + + // Multiple resets + htimer_reset(ctx->target, 2000); + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + + htimer_reset(ctx->target, 4000); + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + + htimer_reset(ctx->target, 8000); + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + + if (ctx->cancel_at_end) { + htimer_del(ctx->target); + } +} + +static void case6_target_cb(htimer_t* timer) { + struct case6_ctx* ctx = (struct case6_ctx*)timer->userdata; + ctx->target_calls++; +} + +static void test_repeated_resets_keep_rearm(void) { + struct case6_ctx ctx = {0}; + ctx.cancel_at_end = false; + + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.ctrl = htimer_add(loop, case6_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case6_target_cb, 1000, 1); + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); // old occurrence suppressed + require(ctx.target->pending == 0); + require(ctx.target->pending_reset == 0); + require(ctx.target->active == 1); + require(((htimeout_t*)ctx.target)->timeout == 8000); // final requested deadline + + // Exactly 1 timer in heap (no duplicate queue membership) + require(loop->timers.nelts == 1); + require(hloop_ntimers(loop) == 1); + require(hloop_nactives(loop) == 1); + + // Fire at replacement deadline + make_timer_due_now(loop, ctx.target); + hloop_process_events(loop, 0); + + require(ctx.target_calls == 1); // exactly one occurrence delivered + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +static void test_repeated_resets_cancel(void) { + struct case6_ctx ctx = {0}; + ctx.cancel_at_end = true; + + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.ctrl = htimer_add(loop, case6_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case6_target_cb, 1000, 1); + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 7: Repeat-timer behavior */ +/* ========================================================================= */ + +struct case7_ctx { + htimer_t* ctrl; + htimer_t* target; + int ctrl_calls; + int target_calls; + int repeat_remaining_after_suppress; +}; + +static void case7_ctrl_cb(htimer_t* timer) { + struct case7_ctx* ctx = (struct case7_ctx*)timer->userdata; + ctx->ctrl_calls++; + + require(ctx->target->pending == 1); + htimer_reset(ctx->target, 5000); + require(ctx->target->pending == 1); + require(ctx->target->pending_reset == 1); + ctx->repeat_remaining_after_suppress = ctx->target->repeat; +} + +static void case7_target_cb(htimer_t* timer) { + struct case7_ctx* ctx = (struct case7_ctx*)timer->userdata; + ctx->target_calls++; +} + +static void test_repeat_timer_multiple_repetitions(void) { + struct case7_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.ctrl = htimer_add(loop, case7_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case7_target_cb, 1000, 3); + require(ctx.target->repeat == 3); + + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + + // Iteration 1: ctrl resets target; 1st occurrence suppressed + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); // suppressed + require(ctx.repeat_remaining_after_suppress == 2); + require(ctx.target->repeat == 2); + require(ctx.target->active == 1); + require(ctx.target->destroy == 0); + require(hloop_ntimers(loop) == 1); + + // Iteration 2: 2nd scheduled occurrence + make_timer_due_now(loop, ctx.target); + hloop_process_events(loop, 0); + + require(ctx.target_calls == 1); + require(ctx.target->repeat == 1); + require(hloop_ntimers(loop) == 1); + + // Iteration 3: 3rd scheduled occurrence + make_timer_due_now(loop, ctx.target); + hloop_process_events(loop, 0); + + require(ctx.target_calls == 2); // exactly 2 delivered occurrences total + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +struct case7_self_rearm_ctx { + int calls; + int repeat_after_rearm; + bool was_pending; +}; + +static void case7_self_rearm_cb(htimer_t* timer) { + struct case7_self_rearm_ctx* ctx = (struct case7_self_rearm_ctx*)timer->userdata; + ctx->calls++; + if (ctx->calls == 1) { + ctx->was_pending = timer->pending; + require(timer->pending == 1); + // Reset repeating timer to a new deadline from inside its own callback + htimer_reset(timer, 5000); + ctx->repeat_after_rearm = timer->repeat; + require(timer->repeat == 2); // repeat was decremented for this occurrence, preserved across reset + require(timer->pending == 1); + require(timer->pending_reset == 1); + } +} + +static void test_repeat_timer_self_rearm(void) { + struct case7_self_rearm_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + htimer_t* timer = htimer_add(loop, case7_self_rearm_cb, 1000, 3); + timer->userdata = &ctx; + + // Occurrence 1 fires and rearms itself from inside the callback + make_timer_due_now(loop, timer); + hloop_process_events(loop, 0); + require(ctx.calls == 1); + require(ctx.was_pending); + require(ctx.repeat_after_rearm == 2); + require(timer->repeat == 2); + require(timer->pending == 0); + require(timer->pending_reset == 0); + require(timer->active == 1); + require(hloop_ntimers(loop) == 1); + + // Occurrence 2 fires + make_timer_due_now(loop, timer); + hloop_process_events(loop, 0); + require(ctx.calls == 2); + require(timer->repeat == 1); + require(hloop_ntimers(loop) == 1); + + // Occurrence 3 fires + make_timer_due_now(loop, timer); + hloop_process_events(loop, 0); + require(ctx.calls == 3); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Case 8: Replacement allocation and cleanup */ +/* ========================================================================= */ + +struct case8a_ctx { + hloop_t* loop; + htimer_t* timer_a; + htimer_t* timer_b; + int a_calls; + int b_calls; + uintptr_t addr_a; + uintptr_t addr_b; + bool addresses_differ; +}; + +static void case8a_b_cb(htimer_t* timer) { + struct case8a_ctx* ctx = (struct case8a_ctx*)timer->userdata; + ctx->b_calls++; +} + +static void case8a_a_cb(htimer_t* timer) { + struct case8a_ctx* ctx = (struct case8a_ctx*)timer->userdata; + ctx->a_calls++; + ctx->addr_a = (uintptr_t)(void*)timer; + + htimer_reset(timer, 2000); + htimer_del(timer); + + // While timer_a is still owned by dispatch, create replacement timer_b + ctx->timer_b = htimer_add(ctx->loop, case8a_b_cb, 3000, 1); + require(ctx->timer_b != NULL); + ctx->timer_b->userdata = ctx; + ctx->addr_b = (uintptr_t)(void*)ctx->timer_b; + + // Storage still owned by dispatch is not offered for reuse + ctx->addresses_differ = (ctx->addr_a != ctx->addr_b); + require_msg(ctx->addresses_differ, "replacement timer must not reuse executing timer storage"); + // Replacement timer retains its own callback and deadline + require(((hevent_t*)ctx->timer_b)->cb == (hevent_cb)case8a_b_cb); + require(((htimeout_t*)ctx->timer_b)->timeout == 3000); +} + +static void test_replacement_allocation_in_executing_callback(void) { + struct case8a_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + ctx.loop = loop; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.timer_a = htimer_add(loop, case8a_a_cb, 1000, 1); + ctx.timer_a->userdata = &ctx; + + make_timer_due_now(loop, ctx.timer_a); + + // Dispatch timer_a + hloop_process_events(loop, 0); + + require(ctx.a_calls == 1); + require(ctx.addr_a != 0); + require(ctx.addr_b != 0); + // Verified while both allocations were alive inside the callback + require(ctx.addresses_differ); + + // timer_a freed by dispatch settlement; timer_b active in loop + require(hloop_ntimers(loop) == 1); + require(hloop_nactives(loop) == 1); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before + 1); + + // Dispatch replacement timer_b + make_timer_due_now(loop, ctx.timer_b); + hloop_process_events(loop, 0); + + require(ctx.b_calls == 1); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +struct case8b_ctx { + hloop_t* loop; + htimer_t* ctrl; + htimer_t* target; + htimer_t* replacement; + int ctrl_calls; + int target_calls; + int replacement_calls; + uintptr_t target_addr; + uintptr_t replacement_addr; + bool addresses_differ; +}; + +static void case8b_target_cb(htimer_t* timer) { + struct case8b_ctx* ctx = (struct case8b_ctx*)timer->userdata; + ctx->target_calls++; +} + +static void case8b_replacement_cb(htimer_t* timer) { + struct case8b_ctx* ctx = (struct case8b_ctx*)timer->userdata; + ctx->replacement_calls++; +} + +static void case8b_ctrl_cb(htimer_t* timer) { + struct case8b_ctx* ctx = (struct case8b_ctx*)timer->userdata; + ctx->ctrl_calls++; + ctx->target_addr = (uintptr_t)(void*)ctx->target; + + htimer_reset(ctx->target, 2000); + htimer_del(ctx->target); + + ctx->replacement = htimer_add(ctx->loop, case8b_replacement_cb, 4000, 1); + require(ctx->replacement != NULL); + ctx->replacement->userdata = ctx; + ctx->replacement_addr = (uintptr_t)(void*)ctx->replacement; + + ctx->addresses_differ = (ctx->target_addr != ctx->replacement_addr); + require_msg(ctx->addresses_differ, "replacement timer must not reuse pending timer storage"); + require(((hevent_t*)ctx->replacement)->cb == (hevent_cb)case8b_replacement_cb); + require(((htimeout_t*)ctx->replacement)->timeout == 4000); +} + +static void test_replacement_allocation_in_controller_callback(void) { + struct case8b_ctx ctx = {0}; + hloop_t* loop = hloop_new(0); + require(loop != NULL); + loop->status = HLOOP_STATUS_RUNNING; + ctx.loop = loop; + + long alloc_before = hv_alloc_cnt() - hv_free_cnt(); + + ctx.ctrl = htimer_add(loop, case8b_ctrl_cb, 1000, 1); + ctx.target = htimer_add(loop, case8b_target_cb, 1000, 1); + ctx.ctrl->userdata = &ctx; + ctx.target->userdata = &ctx; + + hevent_set_priority(ctx.ctrl, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(ctx.target, HEVENT_NORMAL_PRIORITY); + + make_timer_due_now(loop, ctx.ctrl); + make_timer_due_now(loop, ctx.target); + + hloop_process_events(loop, 0); + + require(ctx.ctrl_calls == 1); + require(ctx.target_calls == 0); + require(ctx.target_addr != 0); + require(ctx.replacement_addr != 0); + // Verified while both allocations were alive inside the callback + require(ctx.addresses_differ); + + // ctrl and target are freed; replacement is scheduled + require(hloop_ntimers(loop) == 1); + require(hloop_nactives(loop) == 1); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before + 1); + + // Dispatch replacement + make_timer_due_now(loop, ctx.replacement); + hloop_process_events(loop, 0); + + require(ctx.replacement_calls == 1); + require(hloop_ntimers(loop) == 0); + require(hloop_nactives(loop) == 0); + require(hv_alloc_cnt() - hv_free_cnt() == alloc_before); + + hloop_free(&loop); +} + +/* ========================================================================= */ +/* Main test runner */ +/* ========================================================================= */ + +int main(void) { + printf("Running timer_reset_test cases...\n"); + + test_reset_timer_not_due(); + test_reset_pending_expiration(); + test_rearm_inside_own_callback(); + test_cancel_after_rearm_in_own_callback(); + test_cancel_pending_timer_different_priority(); + test_cancel_pending_timer_same_priority(); + test_repeated_resets_keep_rearm(); + test_repeated_resets_cancel(); + test_repeat_timer_multiple_repetitions(); + test_repeat_timer_self_rearm(); + test_replacement_allocation_in_executing_callback(); + test_replacement_allocation_in_controller_callback(); + + printf("timer_reset_test passed\n"); + return 0; +} From a80e2bc922cc14e3d1887f5a132dca42a9e8c524 Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 20:54:10 +0000 Subject: [PATCH 3/7] Strengthened timer reset deadline assertions --- unittest/timer_reset_test.c | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/unittest/timer_reset_test.c b/unittest/timer_reset_test.c index 5049eaf2e..c898d652a 100644 --- a/unittest/timer_reset_test.c +++ b/unittest/timer_reset_test.c @@ -92,12 +92,16 @@ static void test_reset_timer_not_due(void) { require(hloop_ntimers(loop) == 2); require(hloop_nactives(loop) == 2); - uint64_t old_target_deadline = target->next_timeout; uint64_t unrelated_deadline = unrelated->next_timeout; + // Capture cached clock before reset + uint64_t reset_base_us = loop->cur_hrtime; + // Reset target to a new, further deadline (40000 ms) htimer_reset(target, 40000); + uint64_t expected_deadline = ((reset_base_us + UINT64_C(40000000)) / UINT64_C(100000)) * UINT64_C(100000); + // Reset does not create an additional timer or change unrelated timers require(hloop_ntimers(loop) == 2); require(hloop_nactives(loop) == 2); @@ -105,8 +109,7 @@ static void test_reset_timer_not_due(void) { require(unrelated->next_timeout == unrelated_deadline); // Target replacement deadline matches the newly requested timeout require(((htimeout_t*)target)->timeout == 40000); - require(target->next_timeout > old_target_deadline); - require(target->next_timeout >= old_target_deadline + 20000000ULL - 100000ULL); + require(target->next_timeout == expected_deadline); require(!target->destroy); require(!target->pending); require(!target->pending_reset); @@ -119,7 +122,7 @@ static void test_reset_timer_not_due(void) { require(hloop_ntimers(loop) == 2); require(hloop_nactives(loop) == 2); - // Step 2: Advance clock to replacement deadline; target fires while unrelated does not. + // Step 2: Force target due at its replacement deadline; target fires while unrelated does not. make_timer_due_now(loop, target); hloop_process_events(loop, 0); require(s_case1_target_calls == 1); @@ -127,7 +130,7 @@ static void test_reset_timer_not_due(void) { require(hloop_ntimers(loop) == 1); require(hloop_nactives(loop) == 1); - // Step 3: Advance clock for unrelated timer; unrelated fires. + // Step 3: Force unrelated timer due; unrelated fires. make_timer_due_now(loop, unrelated); hloop_process_events(loop, 0); require(s_case1_unrelated_calls == 1); From 8e3174965647ce83d39b0b6a38ab9dc801799c4e Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:37:01 +0000 Subject: [PATCH 4/7] Fixed C++ timer repeat accounting after reset --- Makefile | 1 + event/hloop.c | 4 + event/hloop.h | 1 + evpp/EventLoop.h | 6 +- scripts/unittest.sh | 3 + unittest/CMakeLists.txt | 7 +- unittest/eventloop_timer_test.cpp | 498 ++++++++++++++++++++++++++++++ 7 files changed, 515 insertions(+), 5 deletions(-) create mode 100644 unittest/eventloop_timer_test.cpp diff --git a/Makefile b/Makefile index 2b1ecd50c..0e08a103c 100644 --- a/Makefile +++ b/Makefile @@ -414,6 +414,7 @@ unittest: prepare libhv $(CC) -g -Wall -O0 -std=c99 -I. -Ibase -Issl -Ievent -o bin/hdns_benchmark unittest/hdns_benchmark.c -Llib -lhv -pthread ifeq ($(WITH_EVPP), yes) $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -o bin/tcpclient_dns_test unittest/tcpclient_dns_test.cpp -Llib -lhv -pthread + $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Issl -Ievent -Icpputil -Ievpp -o bin/eventloop_timer_test unittest/eventloop_timer_test.cpp -Llib -lhv -pthread endif ifeq ($(WITH_HTTP), yes) $(CXX) -g -Wall -O0 -std=c++11 -I. -Ibase -Ihttp -o bin/websocket_parser_test unittest/websocket_parser_test.cpp -Llib -lhv -pthread diff --git a/event/hloop.c b/event/hloop.c index e7156d6e3..cc031fd6b 100644 --- a/event/hloop.c +++ b/event/hloop.c @@ -806,6 +806,10 @@ void htimer_del(htimer_t* timer) { EVENT_DEL(timer); } +uint32_t htimer_repeat(const htimer_t* timer) { + return timer->repeat; +} + const char* hio_engine() { #ifdef EVENT_SELECT return "select"; diff --git a/event/hloop.h b/event/hloop.h index b074d8ab5..e9e349780 100644 --- a/event/hloop.h +++ b/event/hloop.h @@ -236,6 +236,7 @@ HV_EXPORT htimer_t* htimer_add_period(hloop_t* loop, htimer_cb cb, HV_EXPORT void htimer_del(htimer_t* timer); HV_EXPORT void htimer_reset(htimer_t* timer, uint32_t timeout_ms DEFAULT(0)); +HV_EXPORT uint32_t htimer_repeat(const htimer_t* timer); // io //-----------------------low-level apis--------------------------------------- diff --git a/evpp/EventLoop.h b/evpp/EventLoop.h index 483fd34c7..a40d69d67 100644 --- a/evpp/EventLoop.h +++ b/evpp/EventLoop.h @@ -159,13 +159,11 @@ class EventLoop : public Status, public std::enable_shared_from_this // resetTimer thread-safe void resetTimer(TimerID timerID, int timeout_ms = 0) { - runInLoop([timerID, timeout_ms, this](){ + runInLoop([timerID, timeout_ms, this]() { auto iter = timers.find(timerID); if (iter != timers.end()) { htimer_reset(iter->second->timer, timeout_ms); - if (iter->second->repeat == 0) { - iter->second->repeat = 1; - } + iter->second->repeat = htimer_repeat(iter->second->timer); } }); } diff --git a/scripts/unittest.sh b/scripts/unittest.sh index 6c8c58091..6a8999f8f 100755 --- a/scripts/unittest.sh +++ b/scripts/unittest.sh @@ -80,6 +80,9 @@ fi if [ -x bin/tcpclient_dns_test ]; then bin/tcpclient_dns_test fi +if [ -x bin/eventloop_timer_test ]; then + bin/eventloop_timer_test || exit $? +fi for redis_test in redis_async_client_test redis_client_test redis_batch_test redis_subscriber_test lua_redis_test; do if [ -x bin/${redis_test} ]; then if [ "${redis_test}" = "lua_redis_test" ]; then diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 42a829fff..0e9dbd02a 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -209,7 +209,12 @@ if(WITH_EVPP) add_executable(tcpclient_dns_test tcpclient_dns_test.cpp) target_include_directories(tcpclient_dns_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp) target_link_libraries(tcpclient_dns_test ${HV_LIBRARIES}) -set(EVPP_DNS_UNITTEST_TARGETS tcpclient_dns_test) +list(APPEND EVPP_DNS_UNITTEST_TARGETS tcpclient_dns_test) + +add_executable(eventloop_timer_test eventloop_timer_test.cpp) +target_include_directories(eventloop_timer_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp) +target_link_libraries(eventloop_timer_test ${HV_LIBRARIES}) +list(APPEND EVPP_DNS_UNITTEST_TARGETS eventloop_timer_test) endif() if(UNIX) diff --git a/unittest/eventloop_timer_test.cpp b/unittest/eventloop_timer_test.cpp new file mode 100644 index 000000000..51b5fb2dd --- /dev/null +++ b/unittest/eventloop_timer_test.cpp @@ -0,0 +1,498 @@ +/* + * Unit tests for C++ EventLoop timer wrapper repeat accounting and lifetime. + * + * Covers: + * Case 1: Finite count two, pending expiration suppressed via controller reset. + * Case 2: Finite count three, repeated resets of the same pending expiration. + * Case 3: Reset a finite timer between delivered occurrences. + * Case 4: One-shot self-rearm inside callback (zero-to-one revival). + * Case 5: Infinite timer reset and cancellation (INFINITE sentinel preserved). + */ + +#include "EventLoop.h" +#include "hevent.h" + +#include +#include +#include + +#define require(expr) \ + do { \ + if (!(expr)) { \ + fprintf(stderr, "REQUIRE FAILED at %s:%d: %s\n", __FILE__, __LINE__, #expr); \ + exit(EXIT_FAILURE); \ + } \ + } while (0) + +#define require_msg(expr, msg) \ + do { \ + if (!(expr)) { \ + fprintf(stderr, "REQUIRE FAILED at %s:%d: %s (%s)\n", __FILE__, __LINE__, #expr, msg); \ + exit(EXIT_FAILURE); \ + } \ + } while (0) + +using namespace hv; + +static htimer_t* find_timer_in_heap(struct heap_node* node, TimerID timerID) { + if (!node) return NULL; + htimer_t* timer = TIMER_ENTRY(node); + if ((TimerID)hevent_id(timer) == timerID) return timer; + htimer_t* found = find_timer_in_heap(node->left, timerID); + if (found) return found; + return find_timer_in_heap(node->right, timerID); +} + +static htimer_t* find_native_timer(hloop_t* loop, TimerID timerID) { + if (!loop) return NULL; + return find_timer_in_heap(loop->timers.root, timerID); +} + +static void make_timer_due(hloop_t* loop, htimer_t* timer, uint64_t due_time) { + heap_remove(&loop->timers, &timer->node); + timer->next_timeout = due_time; + heap_insert(&loop->timers, &timer->node); +} + +static void make_timer_due_now(hloop_t* loop, htimer_t* timer) { + hloop_update_time(loop); + uint64_t now = loop->cur_hrtime; + uint64_t due = now > 1000 ? now - 1000 : 0; + make_timer_due(loop, timer, due); +} + +static void make_timer_due_ordered(hloop_t* loop, htimer_t* timer, uint64_t offset_us) { + hloop_update_time(loop); + uint64_t now = loop->cur_hrtime; + uint64_t base = now > 10000 ? now - 10000 : 0; + make_timer_due(loop, timer, base + offset_us); +} + +/* + * Case 1: Finite count two, pending expiration suppressed. + * Create a target with count two. A higher-priority controller resets it + * after it becomes pending but before its callback runs. No target callback + * runs in that dispatch. Its next forced expiration runs the target callback + * once, then releases both the native schedule and wrapper callback record. + * The probe expires. Later wrapper reset and cancellation through the old ID + * perform no work. + */ +static void test_case1_finite_count_two_pending_suppressed() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int target_cb_count = 0; + int controller_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + TimerID controller_id = INVALID_TIMER_ID; + + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(100); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&target_cb_count, probe](TimerID id) { + (void)id; + target_cb_count++; + }, + 2); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 2); + + controller_id = evloop.setTimer( + 40000, + [&controller_cb_count, &evloop, target_id](TimerID id) { + (void)id; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + evloop.resetTimer(target_id, 40000); + controller_cb_count++; + }, + 1); + require(controller_id != INVALID_TIMER_ID); + + htimer_t* controller_timer = find_native_timer(loop, controller_id); + require(controller_timer != NULL); + hevent_set_priority(controller_timer, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(target_timer, HEVENT_NORMAL_PRIORITY); + + make_timer_due_ordered(loop, controller_timer, 0); + make_timer_due_ordered(loop, target_timer, 100); + + // Dispatch 1: Controller resets pending target; target callback suppressed + evloop.run(); + + require(controller_cb_count == 1); + require(target_cb_count == 0); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + // After pending reset, native repeat remains 1 and wrapper is synchronized + require(htimer_repeat(target_timer) == 1); + + // Dispatch 2: Next forced expiration runs target callback once and retires + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 1); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + + // Later wrapper reset and cancellation through old ID perform no work + evloop.resetTimer(target_id, 40000); + evloop.killTimer(target_id); + evloop.run(); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +/* + * Case 2: Finite count three, repeated resets of the same pending expiration. + * Reset the pending target more than once before its old delivery is settled. + * Native remaining count stays two; the resets do not consume extra repeats. + * Two later expirations deliver two callbacks and release the wrapper. + */ +static void test_case2_finite_count_three_repeated_resets() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int target_cb_count = 0; + int controller_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + TimerID controller_id = INVALID_TIMER_ID; + + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(200); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&target_cb_count, probe](TimerID id) { + (void)id; + target_cb_count++; + }, + 3); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 3); + + controller_id = evloop.setTimer( + 40000, + [&controller_cb_count, &evloop, target_id](TimerID id) { + (void)id; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + // Reset pending target twice before old delivery settles + evloop.resetTimer(target_id, 40000); + evloop.resetTimer(target_id, 40000); + controller_cb_count++; + }, + 1); + require(controller_id != INVALID_TIMER_ID); + + htimer_t* controller_timer = find_native_timer(loop, controller_id); + require(controller_timer != NULL); + hevent_set_priority(controller_timer, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(target_timer, HEVENT_NORMAL_PRIORITY); + + make_timer_due_ordered(loop, controller_timer, 0); + make_timer_due_ordered(loop, target_timer, 100); + + // Dispatch 1: Controller suppresses pending target delivery + evloop.run(); + + require(controller_cb_count == 1); + require(target_cb_count == 0); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + // Native remaining count stays 2; repeated resets did not consume extra repeats + require(htimer_repeat(target_timer) == 2); + + // Dispatch 2: First remaining expiration + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 1); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 1); + + // Dispatch 3: Second remaining expiration; delivers callback 2 and retires + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 2); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +/* + * Case 3: Reset a finite timer between delivered occurrences. + * Deliver the first occurrence of a count-three timer, reset it while it + * is scheduled but not pending, then complete the two remaining occurrences. + * All three callbacks should be delivered overall, with normal retirement. + */ +static void test_case3_reset_between_delivered_occurrences() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int target_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(300); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&target_cb_count, probe](TimerID id) { + (void)id; + target_cb_count++; + }, + 3); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 3); + + // Occurrence 1: Deliver first occurrence + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 1); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 2); + + // Reset while scheduled but not pending + evloop.resetTimer(target_id, 40000); + evloop.run(); // drive queued work + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 2); + + // Occurrence 2: Deliver second occurrence + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 2); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 1); + + // Occurrence 3: Deliver third occurrence and complete normal retirement + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 3); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +/* + * Case 4: One-shot self-rearm. + * In its first callback, reset the same ID. The wrapper must remain + * registered after dispatch; the next forced expiration must deliver once + * more and then release the probe. + * Also tests zero-to-one revival inside a final live callback. + */ +static void test_case4_oneshot_self_rearm() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int target_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(400); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&target_cb_count, &evloop, probe](TimerID id) { + target_cb_count++; + if (target_cb_count == 1) { + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + + // Reset performs zero-to-one revival and reinserts into heap + evloop.resetTimer(id, 40000); + + // Inside live callback after reset, repeat is revived from 0 to 1 + htimer_t* t = find_native_timer(evloop.loop(), id); + require(t != NULL); + require(htimer_repeat(t) == 1); + } + }, + 1); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 1); + + // Dispatch 1: Fires callback, callback self-rearms via resetTimer + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 1); + // The wrapper must remain registered after dispatch + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == 1); + + // Dispatch 2: Next forced expiration delivers once more and releases probe + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 2); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +/* + * Case 5: Infinite timer reset and cancellation. + * Reset a pending infinite timer, suppress its old delivery, allow a later + * delivery, then cancel it. The INFINITE sentinel must stay unchanged through + * reset; cancellation must release the wrapper and native schedule. + */ +static void test_case5_infinite_reset_and_cancellation() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int target_cb_count = 0; + int controller_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + TimerID controller_id = INVALID_TIMER_ID; + + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(500); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&target_cb_count, probe](TimerID id) { + (void)id; + target_cb_count++; + }, + INFINITE); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == INFINITE); + + controller_id = evloop.setTimer( + 40000, + [&controller_cb_count, &evloop, target_id](TimerID id) { + (void)id; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + evloop.resetTimer(target_id, 40000); + controller_cb_count++; + }, + 1); + require(controller_id != INVALID_TIMER_ID); + + htimer_t* controller_timer = find_native_timer(loop, controller_id); + require(controller_timer != NULL); + hevent_set_priority(controller_timer, HEVENT_HIGHEST_PRIORITY); + hevent_set_priority(target_timer, HEVENT_NORMAL_PRIORITY); + + make_timer_due_ordered(loop, controller_timer, 0); + make_timer_due_ordered(loop, target_timer, 100); + + // Dispatch 1: Controller suppresses pending infinite delivery + evloop.run(); + + require(controller_cb_count == 1); + require(target_cb_count == 0); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + // INFINITE sentinel must stay unchanged through reset + require(htimer_repeat(target_timer) == INFINITE); + + // Dispatch 2: Allow a later delivery + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(target_cb_count == 1); + require(!weak_probe.expired()); + + target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + require(htimer_repeat(target_timer) == INFINITE); + + // Cancel it + evloop.killTimer(target_id); + evloop.run(); // drive queued cancellation + + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +int main() { + printf("Running test_case1_finite_count_two_pending_suppressed...\n"); + test_case1_finite_count_two_pending_suppressed(); + + printf("Running test_case2_finite_count_three_repeated_resets...\n"); + test_case2_finite_count_three_repeated_resets(); + + printf("Running test_case3_reset_between_delivered_occurrences...\n"); + test_case3_reset_between_delivered_occurrences(); + + printf("Running test_case4_oneshot_self_rearm...\n"); + test_case4_oneshot_self_rearm(); + + printf("Running test_case5_infinite_reset_and_cancellation...\n"); + test_case5_infinite_reset_and_cancellation(); + + printf("All EventLoop timer tests (Cases 1-5) passed!\n"); + return 0; +} From 27fa24c06ca3296fc18951123d33a6b50dc9a785 Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 21:43:48 +0000 Subject: [PATCH 5/7] Preserved replacement timers during callback cleanup --- evpp/EventLoop.h | 5 +- unittest/eventloop_timer_test.cpp | 235 +++++++++++++++++++++++++++++- 2 files changed, 238 insertions(+), 2 deletions(-) diff --git a/evpp/EventLoop.h b/evpp/EventLoop.h index a40d69d67..c1e22dbec 100644 --- a/evpp/EventLoop.h +++ b/evpp/EventLoop.h @@ -265,7 +265,10 @@ class EventLoop : public Status, public std::enable_shared_from_this if (timer->cb) timer->cb(timerID); if (timer->repeat == 0) { // htimer_t alloc and free by hloop, but timers[timerID] managed by EventLoop. - loop->timers.erase(timerID); + auto current = loop->timers.find(timerID); + if (current != loop->timers.end() && current->second == timer) { + loop->timers.erase(current); + } } } } diff --git a/unittest/eventloop_timer_test.cpp b/unittest/eventloop_timer_test.cpp index 51b5fb2dd..7249831f6 100644 --- a/unittest/eventloop_timer_test.cpp +++ b/unittest/eventloop_timer_test.cpp @@ -7,6 +7,9 @@ * Case 3: Reset a finite timer between delivered occurrences. * Case 4: One-shot self-rearm inside callback (zero-to-one revival). * Case 5: Infinite timer reset and cancellation (INFINITE sentinel preserved). + * Case 6: Completing callback cancels itself and creates same-ID replacement. + * Case 7: Replacement remains cancellable via killTimer(id). + * Case 8: Callback cancels itself without replacement, plus normal control. */ #include "EventLoop.h" @@ -477,6 +480,227 @@ static void test_case5_infinite_reset_and_cancellation() { hloop_free(&loop); } +/* + * Case 6: Completing callback cancels itself and creates a one-shot replacement + * with the same ID. + * After old dispatch returns, the old callback probe must expire and the + * replacement probe must remain alive. Its next expiration must invoke the + * replacement callback exactly once and then release that probe. + * This is the direct regression for Bug B. + */ +static void test_case6_replacement_with_same_id() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int old_cb_count = 0; + int new_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + + std::weak_ptr old_weak; + std::weak_ptr new_weak; + { + auto old_probe = std::make_shared(601); + old_weak = old_probe; + target_id = evloop.setTimer( + 40000, + [&old_cb_count, &new_cb_count, &evloop, &new_weak, old_probe](TimerID id) { + old_cb_count++; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + evloop.killTimer(id); + + auto new_probe = std::make_shared(602); + new_weak = new_probe; + TimerID rep_id = evloop.setTimer( + 40000, + [&new_cb_count, new_probe](TimerID rid) { + (void)rid; + new_cb_count++; + }, + 1, id); + require(rep_id == id); + }, + 1); + } + require(target_id != INVALID_TIMER_ID); + require(!old_weak.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + + // Dispatch 1: Old callback executes, cancels old, creates replacement with same ID + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(old_cb_count == 1); + require(new_cb_count == 0); + // Old callback probe must expire and replacement probe must remain alive + require(old_weak.expired()); + require(!new_weak.expired()); + + htimer_t* rep_timer = find_native_timer(loop, target_id); + require(rep_timer != NULL); + require(htimer_repeat(rep_timer) == 1); + + // Dispatch 2: Replacement callback executes once and releases probe + make_timer_due_now(loop, rep_timer); + evloop.run(); + + require(new_cb_count == 1); + require(new_weak.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + hloop_free(&loop); +} + +/* + * Case 7: Replacement remains cancellable. + * Repeat case 6, but cancel the new timer by its ID before it becomes due. + * The replacement callback must not execute; its probe must expire and its + * native schedule must be gone. This proves that the replacement was retained + * in the map, not merely kept in a heap. + */ +static void test_case7_replacement_cancellable() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + int old_cb_count = 0; + int new_cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + + std::weak_ptr old_weak; + std::weak_ptr new_weak; + { + auto old_probe = std::make_shared(701); + old_weak = old_probe; + target_id = evloop.setTimer( + 40000, + [&old_cb_count, &new_cb_count, &evloop, &new_weak, old_probe](TimerID id) { + old_cb_count++; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + evloop.killTimer(id); + + auto new_probe = std::make_shared(702); + new_weak = new_probe; + TimerID rep_id = evloop.setTimer( + 40000, + [&new_cb_count, new_probe](TimerID rid) { + (void)rid; + new_cb_count++; + }, + 1, id); + require(rep_id == id); + }, + 1); + } + require(target_id != INVALID_TIMER_ID); + require(!old_weak.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + + // Dispatch 1: Old callback executes and creates replacement + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(old_cb_count == 1); + require(new_cb_count == 0); + require(old_weak.expired()); + require(!new_weak.expired()); + require(find_native_timer(loop, target_id) != NULL); + + // Cancel the replacement by its ID before it becomes due + evloop.killTimer(target_id); + evloop.run(); // drive queued cancellation + + // Replacement probe must expire and native schedule must be gone + require(new_weak.expired()); + require(find_native_timer(loop, target_id) == NULL); + require(new_cb_count == 0); + } + hloop_free(&loop); +} + +/* + * Case 8: Callback cancels itself without replacement, and normal completion control. + * Completion must tolerate the missing map entry, release the retained callback + * record after return, and avoid double deletion or reuse of an invalidated iterator. + * Also includes a normal completion control case. + */ +static void test_case8_cancel_without_replacement_and_control() { + hloop_t* loop = hloop_new(HLOOP_FLAG_RUN_ONCE); + require(loop != NULL); + { + EventLoop evloop(loop); + + // Subcase 8a: Callback cancels itself without replacement + { + int cb_count = 0; + TimerID target_id = INVALID_TIMER_ID; + std::weak_ptr weak_probe; + { + auto probe = std::make_shared(801); + weak_probe = probe; + target_id = evloop.setTimer( + 40000, + [&cb_count, &evloop, probe](TimerID id) { + cb_count++; + require(evloop.isRunning()); + require(evloop.isInLoopThread()); + evloop.killTimer(id); + }, + 1); + } + require(target_id != INVALID_TIMER_ID); + require(!weak_probe.expired()); + + htimer_t* target_timer = find_native_timer(loop, target_id); + require(target_timer != NULL); + + make_timer_due_now(loop, target_timer); + evloop.run(); + + require(cb_count == 1); + require(weak_probe.expired()); + require(find_native_timer(loop, target_id) == NULL); + } + + // Subcase 8b: Normal completion control case + { + int control_cb_count = 0; + TimerID control_id = INVALID_TIMER_ID; + std::weak_ptr control_weak; + { + auto control_probe = std::make_shared(802); + control_weak = control_probe; + control_id = evloop.setTimer( + 40000, + [&control_cb_count, control_probe](TimerID id) { + (void)id; + control_cb_count++; + }, + 1); + } + require(control_id != INVALID_TIMER_ID); + require(!control_weak.expired()); + + htimer_t* control_timer = find_native_timer(loop, control_id); + require(control_timer != NULL); + + make_timer_due_now(loop, control_timer); + evloop.run(); + + require(control_cb_count == 1); + require(control_weak.expired()); + require(find_native_timer(loop, control_id) == NULL); + } + } + hloop_free(&loop); +} + int main() { printf("Running test_case1_finite_count_two_pending_suppressed...\n"); test_case1_finite_count_two_pending_suppressed(); @@ -493,6 +717,15 @@ int main() { printf("Running test_case5_infinite_reset_and_cancellation...\n"); test_case5_infinite_reset_and_cancellation(); - printf("All EventLoop timer tests (Cases 1-5) passed!\n"); + printf("Running test_case6_replacement_with_same_id...\n"); + test_case6_replacement_with_same_id(); + + printf("Running test_case7_replacement_cancellable...\n"); + test_case7_replacement_cancellable(); + + printf("Running test_case8_cancel_without_replacement_and_control...\n"); + test_case8_cancel_without_replacement_and_control(); + + printf("All EventLoop timer tests (Cases 1-8) passed!\n"); return 0; } From c298d316c0cad4ae4885b6116743ea37eba3b9e2 Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:09:17 +0000 Subject: [PATCH 6/7] Documented timer repeat accessor --- event/hloop.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/event/hloop.h b/event/hloop.h index e9e349780..2af29a569 100644 --- a/event/hloop.h +++ b/event/hloop.h @@ -236,6 +236,20 @@ HV_EXPORT htimer_t* htimer_add_period(hloop_t* loop, htimer_cb cb, HV_EXPORT void htimer_del(htimer_t* timer); HV_EXPORT void htimer_reset(htimer_t* timer, uint32_t timeout_ms DEFAULT(0)); +/* + * Returns the native remaining expiration count of a timer, or INFINITE. + * + * Contract: + * - The caller must supply a valid, live timer handle; the function does not + * validate pointers or make using a freed timer safe. + * - Must be read under the timer's normal owner-loop/thread rules. The getter + * provides no cross-thread synchronization. + * - A finite repetition is consumed when its expiration becomes pending, + * before callback delivery. Resetting away that pending callback does not + * refund it. + * - A final callback can observe zero while dispatch still keeps the timer + * alive. Resetting that live timer can revive its count from zero to one. + */ HV_EXPORT uint32_t htimer_repeat(const htimer_t* timer); // io From 53ffe3d4041c53ca839c378312e876d793fd36ed Mon Sep 17 00:00:00 2001 From: radkesvat <134321679+radkesvat@users.noreply.github.com> Date: Tue, 20 Oct 2026 22:52:04 +0000 Subject: [PATCH 7/7] Clarified timer tests and EVPP test grouping --- unittest/CMakeLists.txt | 8 ++++---- unittest/timer_reset_test.c | 6 +++--- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 0e9dbd02a..61e1f9464 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -204,17 +204,17 @@ endforeach() list(APPEND REDIS_UNITTEST_TARGETS ${REDIS_LINKED_UNITTEST_TARGETS}) endif() -# ------evpp: async dns in connect path (connect/reconnect, lifetime, resolve-fail)------ +# ------evpp: async dns and timer lifetime------ if(WITH_EVPP) add_executable(tcpclient_dns_test tcpclient_dns_test.cpp) target_include_directories(tcpclient_dns_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp) target_link_libraries(tcpclient_dns_test ${HV_LIBRARIES}) -list(APPEND EVPP_DNS_UNITTEST_TARGETS tcpclient_dns_test) +list(APPEND EVPP_UNITTEST_TARGETS tcpclient_dns_test) add_executable(eventloop_timer_test eventloop_timer_test.cpp) target_include_directories(eventloop_timer_test PRIVATE .. ../base ../ssl ../event ../cpputil ../evpp) target_link_libraries(eventloop_timer_test ${HV_LIBRARIES}) -list(APPEND EVPP_DNS_UNITTEST_TARGETS eventloop_timer_test) +list(APPEND EVPP_UNITTEST_TARGETS eventloop_timer_test) endif() if(UNIX) @@ -254,5 +254,5 @@ add_custom_target(unittest DEPENDS hdns_test hdns_benchmark ${REDIS_UNITTEST_TARGETS} - ${EVPP_DNS_UNITTEST_TARGETS} + ${EVPP_UNITTEST_TARGETS} ) diff --git a/unittest/timer_reset_test.c b/unittest/timer_reset_test.c index c898d652a..2173fb5f2 100644 --- a/unittest/timer_reset_test.c +++ b/unittest/timer_reset_test.c @@ -122,7 +122,7 @@ static void test_reset_timer_not_due(void) { require(hloop_ntimers(loop) == 2); require(hloop_nactives(loop) == 2); - // Step 2: Force target due at its replacement deadline; target fires while unrelated does not. + // Step 2: Move the target deadline into the past; target fires while unrelated does not. make_timer_due_now(loop, target); hloop_process_events(loop, 0); require(s_case1_target_calls == 1); @@ -229,11 +229,11 @@ static void test_reset_pending_expiration(void) { require(hloop_ntimers(loop) == 1); require(hloop_nactives(loop) == 1); - // Advance clock to replacement deadline + // Move the reset timer's deadline into the past to force delivery. make_timer_due_now(loop, ctx.target); hloop_process_events(loop, 0); - require(ctx.target_calls == 1); // fires at replacement deadline + require(ctx.target_calls == 1); // reset timer fires once when forced due require(hloop_ntimers(loop) == 0); require(hloop_nactives(loop) == 0);