Fix etl::message_timer firing following timers early on unregister (#1484)

active_list.remove(id, has_expired) adjusts the next timer's delta only
when has_expired is false. When unregistering an active, non-expired timer,
unregister_timer() incorrectly passed true, skipping that adjustment, so
every timer after the removed one in the active list fired early by the
removed timer's delta.

Pass false instead, matching the etl::callback_timer family. tick() still
passes true, since the timer has genuinely expired there. The bug affected
all four variants: message_timer, message_timer_interrupt,
message_timer_atomic and message_timer_locked. Existing tests missed it
because they only ever unregistered the tail timer, where the delta
adjustment is a no-op.

Add a regression test that unregisters a non-tail active timer and checks
the following timer still fires at its original absolute time.
This commit is contained in:
Roland Reichwein 2026-07-06 17:32:26 +02:00 committed by GitHub
parent ceb21914fb
commit a47c0aae81
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
5 changed files with 50 additions and 4 deletions

View File

@ -379,7 +379,7 @@ namespace etl
if (timer.is_active())
{
ETL_DISABLE_TIMER_UPDATES;
active_list.remove(timer.id, true);
active_list.remove(timer.id, false);
remove_callback.call_if(timer.id);
ETL_ENABLE_TIMER_UPDATES;
}

View File

@ -108,7 +108,7 @@ namespace etl
if (timer.is_active())
{
++process_semaphore;
active_list.remove(timer.id, true);
active_list.remove(timer.id, false);
remove_callback.call_if(timer.id);
--process_semaphore;
}

View File

@ -114,7 +114,7 @@ namespace etl
TInterruptGuard guard;
(void)guard; // Silence 'unused variable warnings.
active_list.remove(timer.id, true);
active_list.remove(timer.id, false);
remove_callback.call_if(timer.id);
}

View File

@ -111,7 +111,7 @@ namespace etl
if (timer.is_active())
{
lock();
active_list.remove(timer.id, true);
active_list.remove(timer.id, false);
remove_callback.call_if(timer.id);
unlock();
}

View File

@ -433,6 +433,52 @@ namespace
CHECK_ARRAY_EQUAL(compare3.data(), router1.message3.data(), compare3.size());
}
//*************************************************************************
// Unregistering an active timer that is not at the tail of the active list
// must preserve the absolute timeout of the timers that follow it.
// Regression test: unregister_timer used to pass has_expired = true, which
// skipped the delta adjustment of the next timer, making it fire early.
//*************************************************************************
TEST(message_timer_unregister_active_timer_preserves_following_timer_timing)
{
etl::message_timer<2> timer_controller;
// id1 (period 10) becomes the active list head, id2 (period 20) follows it.
etl::timer::id::type id1 = timer_controller.register_timer(message1, router1, 10, etl::timer::mode::Single_Shot);
etl::timer::id::type id2 = timer_controller.register_timer(message2, router1, 20, etl::timer::mode::Single_Shot);
router1.clear();
timer_controller.start(id1);
timer_controller.start(id2);
timer_controller.enable(true);
ticks = 0;
const uint32_t step = 1UL;
while (ticks <= 30U)
{
if (ticks == 5U)
{
// id1 is the active head and still has a 'next' (id2).
timer_controller.unregister_timer(id1);
}
ticks += step;
timer_controller.tick(step);
}
// id1 was unregistered before it could fire.
CHECK_EQUAL(0U, router1.message1.size());
// id2 must still fire at its original absolute time of 20, not earlier.
std::vector<uint64_t> compare2 = {20ULL};
CHECK_EQUAL(compare2.size(), router1.message2.size());
CHECK_ARRAY_EQUAL(compare2.data(), router1.message2.data(), compare2.size());
}
//*************************************************************************
TEST(message_timer_repeating_clear)
{