From 0da6fc569ab6fae77f7ef64fc5aa49c9e7b21623 Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Wed, 15 Jul 2026 16:13:51 +0200 Subject: [PATCH 1/3] fix(esp_timer): Fix esp_timer task dispatch stall The task dispatch method for the esp_timer could stall even if it is armed if ther ISR dispatch alarm triggers close to the task dispatch. This MR fixes a bug where the esp_timer cleared the incorrect cached array timer index and subsequently the timer task is never woken up. Closes https://github.com/espressif/esp-idf/issues/18808 --- .../private_include/esp_timer_impl.h | 15 ++++++-- components/esp_timer/src/esp_timer.c | 37 ++++++++----------- .../esp_timer/src/esp_timer_impl_common.c | 37 +++++++++---------- 3 files changed, 46 insertions(+), 43 deletions(-) diff --git a/components/esp_timer/private_include/esp_timer_impl.h b/components/esp_timer/private_include/esp_timer_impl.h index 2e32f43d903..4e3cde20a28 100644 --- a/components/esp_timer/private_include/esp_timer_impl.h +++ b/components/esp_timer/private_include/esp_timer_impl.h @@ -142,9 +142,18 @@ void esp_timer_impl_init_system_time(void); #if CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD /** - * @brief Set the next alarm if there is such an alarm in the cached array. + * @brief Check which alarm slots have fired and disarm them. * - * @note Available only when CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD is enabled. + * For each dispatch method (TASK and ISR), checks if the cached alarm deadline + * has expired. If so, clears the deadline (resets to UINT64_MAX) and sets the + * corresponding bit in the returned mask. After processing both slots, + * reprograms the hardware alarm to the earliest remaining cached deadline. + * + * Must be called only from ISR context. + * + * @return Bitmask of the dispatch methods whose alarm was due: + * bit 0 set means the TASK-dispatch alarm fired, + * bit 1 set means the ISR-dispatch alarm fired. */ -void esp_timer_impl_try_to_set_next_alarm(void); +uint32_t esp_timer_impl_claim_due_alarms(void); #endif diff --git a/components/esp_timer/src/esp_timer.c b/components/esp_timer/src/esp_timer.c index 9d0c2db5f2c..ec254378816 100644 --- a/components/esp_timer/src/esp_timer.c +++ b/components/esp_timer/src/esp_timer.c @@ -476,13 +476,11 @@ static ESP_TIMER_IRAM_ATTR void timer_list_unlock(esp_timer_dispatch_t timer_typ } #ifdef CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD -static ESP_TIMER_IRAM_ATTR bool timer_process_alarm(esp_timer_dispatch_t dispatch_method) -#else -static bool timer_process_alarm(esp_timer_dispatch_t dispatch_method) +ESP_TIMER_IRAM_ATTR #endif +static void timer_process_alarm(esp_timer_dispatch_t dispatch_method) { timer_list_lock(dispatch_method); - bool processed = false; esp_timer_handle_t it; while (1) { it = LIST_FIRST(&s_timers[dispatch_method]); @@ -492,7 +490,6 @@ static bool timer_process_alarm(esp_timer_dispatch_t dispatch_method) break; } ESP_COMPILER_DIAGNOSTIC_POP("-Wanalyzer-use-after-free") - processed = true; LIST_REMOVE(it, list_entry); if (it->event_id == EVENT_ID_DELETE_TIMER) { // It is handled only by ESP_TIMER_TASK (see esp_timer_delete()). @@ -534,17 +531,9 @@ static bool timer_process_alarm(esp_timer_dispatch_t dispatch_method) #endif } } // while(1) - if (it) { - if (dispatch_method == ESP_TIMER_TASK || (dispatch_method != ESP_TIMER_TASK && processed == true)) { - esp_timer_impl_set_alarm_id(it->alarm, dispatch_method); - } - } else { - if (processed) { - esp_timer_impl_set_alarm_id(UINT64_MAX, dispatch_method); - } - } + uint64_t next_alarm = (it != NULL) ? it->alarm : UINT64_MAX; + esp_timer_impl_set_alarm_id(next_alarm, dispatch_method); timer_list_unlock(dispatch_method); - return processed; } static void timer_task(void* arg) @@ -562,24 +551,30 @@ ESP_TIMER_IRAM_ATTR void esp_timer_isr_dispatch_need_yield(void) assert(xPortInIsrContext()); s_isr_dispatch_need_yield = pdTRUE; } + #endif static void ESP_TIMER_IRAM_ATTR timer_alarm_handler(void* arg) { BaseType_t xHigherPriorityTaskWoken = pdFALSE; - bool isr_timers_processed = false; #ifdef CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD - esp_timer_impl_try_to_set_next_alarm(); - // process timers with ISR dispatch method - isr_timers_processed = timer_process_alarm(ESP_TIMER_ISR); + uint32_t due_alarm_mask = esp_timer_impl_claim_due_alarms(); + + if (due_alarm_mask & (1U << ESP_TIMER_ISR)) { + timer_process_alarm(ESP_TIMER_ISR); + } + xHigherPriorityTaskWoken = s_isr_dispatch_need_yield; s_isr_dispatch_need_yield = pdFALSE; -#endif - if (isr_timers_processed == false) { + if (due_alarm_mask & (1U << ESP_TIMER_TASK)) { vTaskNotifyGiveFromISR(s_timer_task, &xHigherPriorityTaskWoken); } +#else + vTaskNotifyGiveFromISR(s_timer_task, &xHigherPriorityTaskWoken); +#endif + if (xHigherPriorityTaskWoken == pdTRUE) { portYIELD_FROM_ISR(); } diff --git a/components/esp_timer/src/esp_timer_impl_common.c b/components/esp_timer/src/esp_timer_impl_common.c index 94eef0bfc92..4e6cae03c18 100644 --- a/components/esp_timer/src/esp_timer_impl_common.c +++ b/components/esp_timer/src/esp_timer_impl_common.c @@ -38,29 +38,28 @@ void ESP_TIMER_IRAM_ATTR esp_timer_impl_set_alarm(uint64_t timestamp) } #ifdef CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD -void ESP_TIMER_IRAM_ATTR esp_timer_impl_try_to_set_next_alarm(void) +/* Reprogram the hardware alarm to the nearest pending deadline, + * i.e. MIN(timestamp_id[0], timestamp_id[1]). The timestamp_id[] cache is not modified. + * Must be called with s_time_update_lock held. */ +static inline void ESP_TIMER_IRAM_ATTR esp_timer_impl_rearm_alarm(void) +{ + esp_timer_impl_set_alarm_id(timestamp_id[ESP_TIMER_TASK], ESP_TIMER_TASK); +} + +uint32_t ESP_TIMER_IRAM_ATTR esp_timer_impl_claim_due_alarms(void) { portENTER_CRITICAL_ISR(&s_time_update_lock); - unsigned now_alarm_idx; // ISR is called due to this current alarm - unsigned next_alarm_idx; // The following alarm after now_alarm_idx - if (timestamp_id[0] < timestamp_id[1]) { - now_alarm_idx = 0; - next_alarm_idx = 1; - } else { - now_alarm_idx = 1; - next_alarm_idx = 0; - } - - if (timestamp_id[next_alarm_idx] != UINT64_MAX) { - // The following alarm is valid and can be used. - // Remove the current alarm from consideration. - esp_timer_impl_set_alarm_id(UINT64_MAX, now_alarm_idx); - } else { - // There is no the following alarm. - // Remove the current alarm from consideration as well. - timestamp_id[now_alarm_idx] = UINT64_MAX; + uint64_t now = esp_timer_impl_get_time(); + uint32_t due_mask = 0; + for (unsigned alarm_id = 0; alarm_id < sizeof(timestamp_id) / sizeof(timestamp_id[0]); ++alarm_id) { + if (timestamp_id[alarm_id] <= now) { + due_mask |= 1U << alarm_id; + timestamp_id[alarm_id] = UINT64_MAX; + } } + esp_timer_impl_rearm_alarm(); portEXIT_CRITICAL_ISR(&s_time_update_lock); + return due_mask; } #endif From 6685c636b359b1d4b4e223d78e202f3a7b1b078f Mon Sep 17 00:00:00 2001 From: Sudeep Mohanty Date: Wed, 15 Jul 2026 16:47:02 +0200 Subject: [PATCH 2/3] test(esp_timer): Add regression test for esp_timer task dispatch stall --- .../esp_timer/test_apps/main/test_esp_timer.c | 71 +++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/components/esp_timer/test_apps/main/test_esp_timer.c b/components/esp_timer/test_apps/main/test_esp_timer.c index 993273c9a06..b12f18a0fb2 100644 --- a/components/esp_timer/test_apps/main/test_esp_timer.c +++ b/components/esp_timer/test_apps/main/test_esp_timer.c @@ -22,6 +22,7 @@ #include "test_utils.h" #include "esp_freertos_hooks.h" #include "esp_rom_sys.h" +#include "esp_task_wdt.h" /* include performance pass standards header file */ #include "esp_timer_performance.h" @@ -1372,6 +1373,76 @@ TEST_CASE("Test ISR dispatch callbacks are not blocked even if TASK callbacks ta vTaskDelay(3); // wait for the esp_timer task to delete all timers } +static volatile uint32_t task_timer_count; +static volatile uint32_t isr_timer_count; + +static void task_timer_count_cb(void *arg) +{ + task_timer_count++; +} + +static void IRAM_ATTR isr_timer_count_cb(void *arg) +{ + isr_timer_count++; +} + +#define NUM_ISR_TIMERS 5 + +TEST_CASE("TASK dispatch timers not stalled by ISR dispatch timers on shared alarm", "[esp_timer][isr_dispatch][timeout=120]") +{ + task_timer_count = 0; + isr_timer_count = 0; + + esp_timer_handle_t task_timer; + esp_timer_handle_t isr_timers[NUM_ISR_TIMERS]; + + const esp_timer_create_args_t task_args = { + .callback = task_timer_count_cb, + .dispatch_method = ESP_TIMER_TASK, + .name = "task_timer", + }; + + TEST_ESP_OK(esp_timer_create(&task_args, &task_timer)); + + for (int i = 0; i < NUM_ISR_TIMERS; i++) { + const esp_timer_create_args_t isr_args = { + .callback = isr_timer_count_cb, + .dispatch_method = ESP_TIMER_ISR, + .name = "isr_timer", + }; + TEST_ESP_OK(esp_timer_create(&isr_args, &isr_timers[i])); + } + + /* One TASK timer at 1s. Multiple ISR timers at harmonic periods that + * frequently collide with the TASK alarm (1s, 500ms, 333ms, 250ms, 200ms). + * This maximizes the shared-alarm race window that triggers the stall. */ + TEST_ESP_OK(esp_timer_start_periodic(task_timer, 1 * SEC)); + TEST_ESP_OK(esp_timer_start_periodic(isr_timers[0], 1 * SEC)); + TEST_ESP_OK(esp_timer_start_periodic(isr_timers[1], SEC / 2)); + TEST_ESP_OK(esp_timer_start_periodic(isr_timers[2], SEC / 3)); + TEST_ESP_OK(esp_timer_start_periodic(isr_timers[3], SEC / 4)); + TEST_ESP_OK(esp_timer_start_periodic(isr_timers[4], SEC / 5)); + + for (int i = 0; i < 20; i++) { + uint32_t prev_task = task_timer_count; + vTaskDelay(pdMS_TO_TICKS(5000)); + esp_task_wdt_reset(); + uint32_t cur_task = task_timer_count; + uint32_t cur_isr = isr_timer_count; + printf("check %d/20: task_timer_count=%" PRIu32 " isr_timer_count=%" PRIu32 "\n", + i + 1, cur_task, cur_isr); + // Verify TASK-dispatch callback is still being invoked (not stalled) + TEST_ASSERT_GREATER_THAN_UINT32(prev_task, cur_task); + } + + TEST_ESP_OK(esp_timer_stop(task_timer)); + TEST_ESP_OK(esp_timer_delete(task_timer)); + for (int i = 0; i < NUM_ISR_TIMERS; i++) { + TEST_ESP_OK(esp_timer_stop(isr_timers[i])); + TEST_ESP_OK(esp_timer_delete(isr_timers[i])); + } +} + #endif // CONFIG_ESP_TIMER_SUPPORTS_ISR_DISPATCH_METHOD typedef struct { From 2f8ef67a2d6db22a55477bb63e8e848656e0e082 Mon Sep 17 00:00:00 2001 From: Konstantin Kondrashov Date: Fri, 28 Aug 2026 16:52:22 +0300 Subject: [PATCH 3/3] test(esp_timer): Restrict LAC-timer alarm test to ESP32 only The "set timer < now_time" test targets the ESP32 LAC timer (magic constant 0xefffffff/80 and a tick-based tolerance) and is not valid on systimer-based targets, where it fails under the ISR-dispatch configs. On master it is already guarded with CONFIG_IDF_TARGET_ESP32; this guard was not backported to v6.1. Add the same guard here. --- components/esp_timer/test_apps/main/test_esp_timer.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/components/esp_timer/test_apps/main/test_esp_timer.c b/components/esp_timer/test_apps/main/test_esp_timer.c index b12f18a0fb2..a2aaf6c88d2 100644 --- a/components/esp_timer/test_apps/main/test_esp_timer.c +++ b/components/esp_timer/test_apps/main/test_esp_timer.c @@ -875,6 +875,7 @@ TEST_CASE("esp_timer_impl_set_alarm and using start_once do not lead that the Sy #endif // !defined(CONFIG_FREERTOS_UNICORE) && SOC_DPORT_WORKAROUND +#ifdef CONFIG_IDF_TARGET_ESP32 TEST_CASE("Test case when esp_timer_impl_set_alarm needs set timer < now_time", "[esp_timer]") { esp_timer_impl_advance(50331648); // 0xefffffff/80 = 50331647 @@ -892,6 +893,7 @@ TEST_CASE("Test case when esp_timer_impl_set_alarm needs set timer < now_time", printf("alarm_reg = 0x%llx, count_reg 0x%llx\n", alarm_reg, count_reg); TEST_ASSERT(alarm_reg <= (count_reg + offset)); } +#endif // CONFIG_IDF_TARGET_ESP32 static void timer_callback5(void* arg) {