From a47c0aae81b886259918961d5fcee20faa06fdd0 Mon Sep 17 00:00:00 2001
From: Roland Reichwein <Roland.Reichwein@bmw.de>
Date: Mon, 6 Jul 2026 17:32:26 +0200
Subject: [PATCH] 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.
---
 include/etl/message_timer.h           |  2 +-
 include/etl/message_timer_atomic.h    |  2 +-
 include/etl/message_timer_interrupt.h |  2 +-
 include/etl/message_timer_locked.h    |  2 +-
 test/test_message_timer.cpp           | 46 +++++++++++++++++++++++++++
 5 files changed, 50 insertions(+), 4 deletions(-)

diff --git a/include/etl/message_timer.h b/include/etl/message_timer.h
index eaf506c8..a9c0ce31 100644
--- a/include/etl/message_timer.h
+++ b/include/etl/message_timer.h
@@ -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;
           }
diff --git a/include/etl/message_timer_atomic.h b/include/etl/message_timer_atomic.h
index 93e328c3..8d716d8b 100644
--- a/include/etl/message_timer_atomic.h
+++ b/include/etl/message_timer_atomic.h
@@ -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;
           }
diff --git a/include/etl/message_timer_interrupt.h b/include/etl/message_timer_interrupt.h
index 5aaeb424..33ac248c 100644
--- a/include/etl/message_timer_interrupt.h
+++ b/include/etl/message_timer_interrupt.h
@@ -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);
           }
 
diff --git a/include/etl/message_timer_locked.h b/include/etl/message_timer_locked.h
index 4eb689ee..92b41488 100644
--- a/include/etl/message_timer_locked.h
+++ b/include/etl/message_timer_locked.h
@@ -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();
           }
diff --git a/test/test_message_timer.cpp b/test/test_message_timer.cpp
index 712d13c1..4b05f74b 100644
--- a/test/test_message_timer.cpp
+++ b/test/test_message_timer.cpp
@@ -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)
     {