diff --git a/components/esp_eth/test_apps/main/esp_eth_test_apps.c b/components/esp_eth/test_apps/main/esp_eth_test_apps.c index d2350a1fb2..392abe1058 100644 --- a/components/esp_eth/test_apps/main/esp_eth_test_apps.c +++ b/components/esp_eth/test_apps/main/esp_eth_test_apps.c @@ -434,7 +434,10 @@ cleanup: TEST_ESP_OK(esp_eth_driver_uninstall(eth_handle)); TEST_ESP_OK(phy->del(phy)); TEST_ESP_OK(mac->del(mac)); +#ifndef CONFIG_TARGET_ETH_PHY_DEVICE_W5500 +// only unregister events if the device != W5500, since w5500 doesn't support loopback and we don't register the event TEST_ESP_OK(esp_event_handler_unregister(ETH_EVENT, ESP_EVENT_ANY_ID, eth_event_handler)); +#endif TEST_ESP_OK(esp_event_loop_delete_default()); extra_cleanup(); vEventGroupDelete(eth_event_group); diff --git a/components/esp_event/esp_event.c b/components/esp_event/esp_event.c index 0d9be71409..9358d910e2 100644 --- a/components/esp_event/esp_event.c +++ b/components/esp_event/esp_event.c @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -22,7 +22,7 @@ /* ---------------------------- Definitions --------------------------------- */ #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING -// LOOP @ rx: dr: +// LOOP @ rx: dr: #define LOOP_DUMP_FORMAT "LOOP @%p,%s rx:%" PRIu32 " dr:%" PRIu32 "\n" // handler @
ev: inv: time: #define HANDLER_DUMP_FORMAT " HANDLER @%p ev:%s,%s inv:%" PRIu32 " time:%lld us\n" @@ -38,6 +38,7 @@ static const char* TAG = "event"; static const char* esp_event_any_base = "any"; +static const char* esp_event_handler_cleanup = "cleanup"; #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING static SLIST_HEAD(esp_event_loop_instance_list_t, esp_event_loop_instance) s_event_loops = @@ -134,29 +135,14 @@ static void handler_execute(esp_event_loop_instance_t* loop, esp_event_handler_n (*(handler->handler_ctx->handler))(handler->handler_ctx->arg, post.base, post.id, data_ptr); #else - (*(handler->handler_ctx->handler))(handler->handler_ctx->arg, post.base, post.id, post.data); + (*(handler->handler_ctx->handler))(handler->handler_ctx->arg, post.base, post.id, post.data.ptr); #endif #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING diff = esp_timer_get_time() - start; - xSemaphoreTake(loop->profiling_mutex, portMAX_DELAY); - - // At this point handler may be already unregistered. - // This happens in "handler instance can unregister itself" test case. - // To prevent memory corruption error it's necessary to check if pointer is still valid. - esp_event_loop_node_t* loop_node; - esp_event_handler_node_t* handler_node; - SLIST_FOREACH(loop_node, &(loop->loop_nodes), next) { - SLIST_FOREACH(handler_node, &(loop_node->handlers), next) { - if (handler_node == handler) { - handler->invoked++; - handler->time += diff; - } - } - } - - xSemaphoreGive(loop->profiling_mutex); + handler->invoked++; + handler->time += diff; #endif } @@ -355,8 +341,8 @@ static esp_err_t base_node_remove_handler(esp_event_base_node_t* base_node, int3 if (SLIST_EMPTY(&(it->handlers))) { SLIST_REMOVE(&(base_node->id_nodes), it, esp_event_id_node, next); free(it); - return ESP_OK; } + return ESP_OK; } } } @@ -379,8 +365,8 @@ static esp_err_t loop_node_remove_handler(esp_event_loop_node_t* loop_node, esp_ if (SLIST_EMPTY(&(it->handlers)) && SLIST_EMPTY(&(it->id_nodes))) { SLIST_REMOVE(&(loop_node->base_nodes), it, esp_event_base_node, next); free(it); - return ESP_OK; } + return ESP_OK; } } } @@ -389,6 +375,23 @@ static esp_err_t loop_node_remove_handler(esp_event_loop_node_t* loop_node, esp_ return ESP_ERR_NOT_FOUND; } +static esp_err_t loop_remove_handler(esp_event_remove_handler_context_t* ctx) +{ + esp_event_loop_node_t *it, *temp; + SLIST_FOREACH_SAFE(it, &(ctx->loop->loop_nodes), next, temp) { + esp_err_t res = loop_node_remove_handler(it, ctx->event_base, ctx->event_id, ctx->handler_ctx, ctx->legacy); + + if (res == ESP_OK) { + if (SLIST_EMPTY(&(it->base_nodes)) && SLIST_EMPTY(&(it->handlers))) { + SLIST_REMOVE(&(ctx->loop->loop_nodes), it, esp_event_loop_node, next); + free(it); + } + return ESP_OK; + } + } + return ESP_ERR_NOT_FOUND; +} + static void handler_instances_remove_all(esp_event_handler_nodes_t* handlers) { esp_event_handler_node_t *it, *temp; @@ -426,17 +429,112 @@ static void loop_node_remove_all_handler(esp_event_loop_node_t* loop_node) static void inline __attribute__((always_inline)) post_instance_delete(esp_event_post_instance_t* post) { #if CONFIG_ESP_EVENT_POST_FROM_ISR - if (post->data_allocated && post->data.ptr) { + if (post->data_allocated) +#endif + { free(post->data.ptr); } -#else - if (post->data) { - free(post->data); - } -#endif memset(post, 0, sizeof(*post)); } +static esp_err_t find_and_unregister_handler(esp_event_remove_handler_context_t* ctx) +{ + esp_event_handler_node_t *handler_to_unregister = NULL; + esp_event_handler_node_t *handler; + esp_event_loop_node_t *loop_node; + esp_event_base_node_t *base_node; + esp_event_id_node_t *id_node; + + SLIST_FOREACH(loop_node, &(ctx->loop->loop_nodes), next) { + // Execute loop level handlers + SLIST_FOREACH(handler, &(loop_node->handlers), next) { + if (ctx->legacy) { + if (handler->handler_ctx->handler == ctx->handler_ctx->handler) { + handler_to_unregister = handler; + break; + } + } else { + if (handler->handler_ctx == ctx->handler_ctx) { + handler_to_unregister = handler; + break; + } + } + } + + if (handler_to_unregister != NULL) { + break; + } + + SLIST_FOREACH(base_node, &(loop_node->base_nodes), next) { + if (base_node->base == ctx->event_base) { + // Execute base level handlers + SLIST_FOREACH(handler, &(base_node->handlers), next) { + if (ctx->legacy) { + if (handler->handler_ctx->handler == ctx->handler_ctx->handler) { + handler_to_unregister = handler; + break; + } + } else { + if (handler->handler_ctx == ctx->handler_ctx) { + handler_to_unregister = handler; + break; + } + } + } + + if (handler_to_unregister != NULL) { + break; + } + + SLIST_FOREACH(id_node, &(base_node->id_nodes), next) { + if (id_node->id == ctx->event_id) { + // Execute id level handlers + SLIST_FOREACH(handler, &(id_node->handlers), next) { + if (ctx->legacy) { + if (handler->handler_ctx->handler == ctx->handler_ctx->handler) { + handler_to_unregister = handler; + break; + } + } else { + if (handler->handler_ctx == ctx->handler_ctx) { + handler_to_unregister = handler; + break; + } + } + } + } + } + } + } + } + + if (handler_to_unregister == NULL) { + /* handler not found in the lists, return */ + return ESP_ERR_NOT_FOUND; + } + + if (handler_to_unregister->unregistered) { + /* the handler was found in a list but has already be marked + * as unregistered. It means an event was already created to + * remove from the list. return OK but do nothing */ + return ESP_OK; + } + /* handler found in the lists and not already marked as unregistered. Mark it as unregistered + * and post an event to remove it from the lists */ + handler_to_unregister->unregistered = true; + if (ctx->legacy) { + /* in case of legacy code, we have to copy the handler_ctx content since it was created in the calling function */ + esp_event_handler_instance_context_t *handler_ctx_copy = calloc(1, sizeof(esp_event_handler_instance_context_t)); + if (!handler_ctx_copy) { + return ESP_ERR_NO_MEM; + } + handler_ctx_copy->arg = ctx->handler_ctx->arg; + handler_ctx_copy->handler = ctx->handler_ctx->handler; + ctx->handler_ctx = handler_ctx_copy; + } + return esp_event_post_to(ctx->loop, esp_event_handler_cleanup, 0, ctx, sizeof(esp_event_remove_handler_context_t), portMAX_DELAY); +} + /* ---------------------------- Public API --------------------------------- */ esp_err_t esp_event_loop_create(const esp_event_loop_args_t* event_loop_args, esp_event_loop_handle_t* event_loop) @@ -472,14 +570,6 @@ esp_err_t esp_event_loop_create(const esp_event_loop_args_t* event_loop_args, es goto on_err; } -#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - loop->profiling_mutex = xSemaphoreCreateMutex(); - if (loop->profiling_mutex == NULL) { - ESP_LOGE(TAG, "create event loop profiling mutex failed"); - goto on_err; - } -#endif - SLIST_INIT(&(loop->loop_nodes)); // Create the loop task if requested @@ -525,12 +615,6 @@ on_err: vSemaphoreDelete(loop->mutex); } -#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - if (loop->profiling_mutex != NULL) { - vSemaphoreDelete(loop->profiling_mutex); - } -#endif - free(loop); return err; @@ -557,10 +641,25 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick int64_t remaining_ticks = ticks_to_run; #endif - while (xQueueReceive(loop->queue, &post, ticks_to_run) == pdTRUE) { + while (xQueueReceive(loop->queue, &post, remaining_ticks) == pdTRUE) { // The event has already been unqueued, so ensure it gets executed. xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY); + // check if the event retrieve from the queue is the internal event that is + // triggered when a handler needs to be removed.. + if (post.base == esp_event_handler_cleanup) { + assert(post.data.ptr != NULL); + esp_event_remove_handler_context_t* ctx = (esp_event_remove_handler_context_t*)post.data.ptr; + loop_remove_handler(ctx); + + // if the handler unregistration request came from legacy code, + // we have to free handler_ctx pointer since it points to memory + // allocated by esp_event_handler_unregister_with_internal + if (ctx->legacy) { + free(ctx->handler_ctx); + } + } + loop->running_task = xTaskGetCurrentTaskHandle(); bool exec = false; @@ -573,24 +672,30 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick SLIST_FOREACH_SAFE(loop_node, &(loop->loop_nodes), next, temp_node) { // Execute loop level handlers SLIST_FOREACH_SAFE(handler, &(loop_node->handlers), next, temp_handler) { - handler_execute(loop, handler, post); - exec |= true; + if (!handler->unregistered) { + handler_execute(loop, handler, post); + exec |= true; + } } SLIST_FOREACH_SAFE(base_node, &(loop_node->base_nodes), next, temp_base) { if (base_node->base == post.base) { // Execute base level handlers SLIST_FOREACH_SAFE(handler, &(base_node->handlers), next, temp_handler) { - handler_execute(loop, handler, post); - exec |= true; + if (!handler->unregistered) { + handler_execute(loop, handler, post); + exec |= true; + } } SLIST_FOREACH_SAFE(id_node, &(base_node->id_nodes), next, temp_id_node) { if (id_node->id == post.id) { // Execute id level handlers SLIST_FOREACH_SAFE(handler, &(id_node->handlers), next, temp_handler) { - handler_execute(loop, handler, post); - exec |= true; + if (!handler->unregistered) { + handler_execute(loop, handler, post); + exec |= true; + } } // Skip to next base node break; @@ -637,14 +742,10 @@ esp_err_t esp_event_loop_delete(esp_event_loop_handle_t event_loop) esp_event_loop_instance_t* loop = (esp_event_loop_instance_t*) event_loop; SemaphoreHandle_t loop_mutex = loop->mutex; -#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - SemaphoreHandle_t loop_profiling_mutex = loop->profiling_mutex; -#endif xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY); #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - xSemaphoreTake(loop->profiling_mutex, portMAX_DELAY); portENTER_CRITICAL(&s_event_loops_spinlock); SLIST_REMOVE(&s_event_loops, loop, esp_event_loop_instance, next); portEXIT_CRITICAL(&s_event_loops_spinlock); @@ -674,10 +775,6 @@ esp_err_t esp_event_loop_delete(esp_event_loop_handle_t event_loop) free(loop); // Free loop mutex before deleting xSemaphoreGiveRecursive(loop_mutex); -#ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - xSemaphoreGive(loop_profiling_mutex); - vSemaphoreDelete(loop_profiling_mutex); -#endif vSemaphoreDelete(loop_mutex); return ESP_OK; @@ -775,24 +872,21 @@ esp_err_t esp_event_handler_unregister_with_internal(esp_event_loop_handle_t eve } esp_event_loop_instance_t* loop = (esp_event_loop_instance_t*) event_loop; + esp_event_remove_handler_context_t remove_handler_ctx = {loop, event_base, event_id, handler_ctx, legacy}; - xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY); - - esp_event_loop_node_t *it, *temp; - - SLIST_FOREACH_SAFE(it, &(loop->loop_nodes), next, temp) { - esp_err_t res = loop_node_remove_handler(it, event_base, event_id, handler_ctx, legacy); - - if (res == ESP_OK && SLIST_EMPTY(&(it->base_nodes)) && SLIST_EMPTY(&(it->handlers))) { - SLIST_REMOVE(&(loop->loop_nodes), it, esp_event_loop_node, next); - free(it); - break; - } + /* remove the handler if the mutex is taken successfully. + * otherwise it will be removed from the list later */ + esp_err_t res = ESP_FAIL; + if (xSemaphoreTake(loop->mutex, 0) == pdTRUE) { + res = loop_remove_handler(&remove_handler_ctx); + xSemaphoreGive(loop->mutex); + } else { + xSemaphoreTakeRecursive(loop->mutex, portMAX_DELAY); + res = find_and_unregister_handler(&remove_handler_ctx); + xSemaphoreGiveRecursive(loop->mutex); } - xSemaphoreGiveRecursive(loop->mutex); - - return ESP_OK; + return res; } esp_err_t esp_event_handler_unregister_with(esp_event_loop_handle_t event_loop, esp_event_base_t event_base, @@ -838,12 +932,10 @@ esp_err_t esp_event_post_to(esp_event_loop_handle_t event_loop, esp_event_base_t } memcpy(event_data_copy, event_data, event_data_size); -#if CONFIG_ESP_EVENT_POST_FROM_ISR post.data.ptr = event_data_copy; +#if CONFIG_ESP_EVENT_POST_FROM_ISR post.data_allocated = true; post.data_set = true; -#else - post.data = event_data_copy; #endif } post.base = event_base; @@ -885,7 +977,7 @@ esp_err_t esp_event_post_to(esp_event_loop_handle_t event_loop, esp_event_base_t } #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - atomic_fetch_add(&loop->events_recieved, 1); + atomic_fetch_add(&loop->events_received, 1); #endif return ESP_OK; @@ -933,7 +1025,7 @@ esp_err_t esp_event_isr_post_to(esp_event_loop_handle_t event_loop, esp_event_ba } #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - atomic_fetch_add(&loop->events_recieved, 1); + atomic_fetch_add(&loop->events_received, 1); #endif return ESP_OK; @@ -962,13 +1054,13 @@ esp_err_t esp_event_dump(FILE* file) portENTER_CRITICAL(&s_event_loops_spinlock); SLIST_FOREACH(loop_it, &s_event_loops, next) { - uint32_t events_recieved, events_dropped; + uint32_t events_received, events_dropped; - events_recieved = atomic_load(&loop_it->events_recieved); + events_received = atomic_load(&loop_it->events_received); events_dropped = atomic_load(&loop_it->events_dropped); PRINT_DUMP_INFO(dst, sz, LOOP_DUMP_FORMAT, loop_it, loop_it->task != NULL ? loop_it->name : "none", - events_recieved, events_dropped); + events_received, events_dropped); int sz_bak = sz; diff --git a/components/esp_event/private_include/esp_event_internal.h b/components/esp_event/private_include/esp_event_internal.h index caa0f1614b..76304c689c 100644 --- a/components/esp_event/private_include/esp_event_internal.h +++ b/components/esp_event/private_include/esp_event_internal.h @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2018-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2018-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Apache-2.0 */ @@ -31,6 +31,7 @@ typedef struct esp_event_handler_node { int64_t time; /**< total runtime of this handler across all calls */ #endif SLIST_ENTRY(esp_event_handler_node) next; /**< next event handler in the list */ + bool unregistered; } esp_event_handler_node_t; typedef SLIST_HEAD(esp_event_handler_instances, esp_event_handler_node) esp_event_handler_nodes_t; @@ -76,21 +77,24 @@ typedef struct esp_event_loop_instance { esp_event_loop_nodes_t loop_nodes; /**< set of linked lists containing the registered handlers for the loop */ #ifdef CONFIG_ESP_EVENT_LOOP_PROFILING - atomic_uint_least32_t events_recieved; /**< number of events successfully posted to the loop */ + atomic_uint_least32_t events_received; /**< number of events successfully posted to the loop */ atomic_uint_least32_t events_dropped; /**< number of events dropped due to queue being full */ - SemaphoreHandle_t profiling_mutex; /**< mutex used for profiliing */ SLIST_ENTRY(esp_event_loop_instance) next; /**< next event loop in the list */ #endif } esp_event_loop_instance_t; -#if CONFIG_ESP_EVENT_POST_FROM_ISR +typedef struct esp_event_remove_handler_context_t { + esp_event_loop_instance_t* loop; /**< Instance of the event loop from which the handler has to be removed */ + esp_event_base_t event_base; /**< The event base identification of the handler that has to be removed */ + int32_t event_id; /**< The event identification value of the handler that has to be removed */ + esp_event_handler_instance_context_t* handler_ctx; /**< The handler context of the handler that has to be removed */ + bool legacy; /**< Set to true when the handler unregistration request was made from legacy code */ +} esp_event_remove_handler_context_t; + typedef union esp_event_post_data { uint32_t val; void *ptr; } esp_event_post_data_t; -#else -typedef void* esp_event_post_data_t; -#endif /// Event posted to the event queue typedef struct esp_event_post_instance { diff --git a/components/esp_event/test_apps/main/test_event_common.cpp b/components/esp_event/test_apps/main/test_event_common.cpp index bbac3490f1..cc3d665ba4 100644 --- a/components/esp_event/test_apps/main/test_event_common.cpp +++ b/components/esp_event/test_apps/main/test_event_common.cpp @@ -1,5 +1,5 @@ /* - * SPDX-FileCopyrightText: 2023-2024 Espressif Systems (Shanghai) CO LTD + * SPDX-FileCopyrightText: 2023-2025 Espressif Systems (Shanghai) CO LTD * * SPDX-License-Identifier: Unlicense OR CC0-1.0 */ @@ -1597,3 +1597,167 @@ TEST_CASE("default event loop: registering event handler instance without instan TEST_ESP_OK(esp_event_loop_delete_default()); } + +#if CONFIG_ESP_EVENT_LOOP_PROFILING +static void handler_all(void* arg, esp_event_base_t event_base, int32_t event_id, void* data) +{ + printf("base=%s, id=%" PRId32 ", data=%p\n", event_base, event_id, data); +} + +static void handler_base(void* arg, esp_event_base_t event_base, int32_t event_id, void* data) +{ + printf("base=%s, id=%" PRId32 ", data=%p\n", event_base, event_id, data); +} + +static void handler_id(void* arg, esp_event_base_t event_base, int32_t event_id, void* data) +{ + printf("base=%s, id=%" PRId32 ", data=%p\n", event_base, event_id, data); +} + +/* This test executes the following steps: + * 1) register handler_id for a event id A and event base B, + * 2) register handler_base for a given event base, + * 3) register handler_all for on the loop level, + * 4) post an event (A, B), + * 5) call esp_event_dump and make sure by parsing the string IN PYTEST ENVIRONMENT that + * all handlers profiling data is printed + * 6) unregister the handlers successfully */ +TEST_CASE("profiling reports valid values", "[event][default]") +{ + TEST_ESP_OK(esp_event_loop_create_default()); + + /* register handler for event base 1 and event id 1 */ + TEST_ESP_OK(esp_event_handler_register(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id, NULL)); + + /* register handler for event base 1 and all event ids */ + TEST_ESP_OK(esp_event_handler_register(s_test_base1, ESP_EVENT_ANY_ID, handler_base, NULL)); + + /* register handler for all event bases and all event ids */ + TEST_ESP_OK(esp_event_handler_register(ESP_EVENT_ANY_BASE, ESP_EVENT_ANY_ID, handler_all, NULL)); + + /* post an event on event base 1, event id 1 */ + TEST_ESP_OK(esp_event_post(s_test_base1, TEST_EVENT_BASE1_EV1, NULL, 0, pdMS_TO_TICKS(1000))); + + /* post an event 1 from base 1 and check the dump. + * - 3 handlers invoked, exec time is not 0 */ + esp_event_dump(stdout); + + /* unregister handlers */ + TEST_ESP_OK(esp_event_handler_unregister(ESP_EVENT_ANY_BASE, ESP_EVENT_ANY_ID, handler_all)); + TEST_ESP_OK(esp_event_handler_unregister(s_test_base1, ESP_EVENT_ANY_ID, handler_base)); + TEST_ESP_OK(esp_event_handler_unregister(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id)); + + /* delete loop */ + TEST_ESP_OK(esp_event_loop_delete_default()); +} + +static void handler_id2(void* arg, esp_event_base_t event_base, int32_t event_id, void* data) +{ + printf("event received: base=%s, id=%" PRId32 ", data=%p\n", event_base, event_id, data); + + /* self unregistering handler */ + TEST_ESP_OK(esp_event_handler_unregister(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id2)); + + /* register a new handler on id level */ + TEST_ESP_OK(esp_event_handler_register(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id, NULL)); +} + +/* This test executes the following steps: + * 1) register handler_id2 for a given event id and event base, + * 2) post an event to trigger the handler_id2, + * 3) unregister the handler_id2 during the execution of the handler_id2 and register + * handler_id instead. + * 4) call esp_event_dump and make sure by parsing the string IN PYTEST ENVIRONMENT that + * 1 handler profiling data is printed + * 5) unregister the handler_id successfully */ +TEST_CASE("esp_event_dump does not show self unregistered handler", "[event][default]") +{ + TEST_ESP_OK(esp_event_loop_create_default()); + + /* register handler for event base 1 and event id 1 */ + TEST_ESP_OK(esp_event_handler_register(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id2, NULL)); + + /* post an event on event base 1, event id 1 */ + TEST_ESP_OK(esp_event_post(s_test_base1, TEST_EVENT_BASE1_EV1, NULL, 0, pdMS_TO_TICKS(1000))); + + /* post an event 1 from base 1 and check the dump. + * - 1 handler invoked 0 times, exec time is 0 us */ + esp_event_dump(stdout); + + /* unregister handler id */ + TEST_ESP_OK(esp_event_handler_unregister(s_test_base1, TEST_EVENT_BASE1_EV1, handler_id)); + + /* delete loop */ + TEST_ESP_OK(esp_event_loop_delete_default()); +} + +static SemaphoreHandle_t s_event_mutex; +static StaticSemaphore_t s_event_mutex_buf; +static size_t s_handler_triggered = 0; + +static void self_unregistering_handler(void* arg, esp_event_base_t event_base, int32_t event_id, void* data) +{ + xSemaphoreTake(s_event_mutex, portMAX_DELAY); + + /* self unregistering handler */ + TEST_ESP_OK(esp_event_handler_unregister(s_test_base1, TEST_EVENT_BASE1_EV1, self_unregistering_handler)); + + s_handler_triggered++; + + xSemaphoreGive(s_event_mutex); +} + +/* This test makes sure that once a handler is unregistered, it is never called again. + * This test follows the update in esp event unregistration process where a handler can + * be marked as unregistered but not removed directly from the list it belongs to. + * The test creates a handler triggered by a specific combination of event base / event id + * and generates 2 consecutive events that should trigger the handler. The handler unregisters + * itself. Make sure that the second event does not trigger the handler again. + * + * Both event posts need to be queued before the unregistration happens, to make sure that event if + * the handler is still present in the handlers lists when the second event triggers, it will not + * be called. To make sure of that, the execution of the handler is blocked by a mutex released from + * test after the 2 events are posted. */ +TEST_CASE("self unregistered handlers are never called again after they return", "[event][default]") +{ + s_event_mutex = xSemaphoreCreateMutexStatic(&s_event_mutex_buf); + TEST_ASSERT_NOT_NULL(s_event_mutex); + + esp_err_t ret = esp_event_loop_create_default(); + printf("esp_event_loop_create_default %d\n", ret); + TEST_ESP_OK(ret); + + /* register handler for event base 1 and event id 1 */ + ret = esp_event_handler_register(s_test_base1, TEST_EVENT_BASE1_EV1, self_unregistering_handler, NULL); + printf("esp_event_handler_register %d\n", ret); + TEST_ESP_OK(ret); + + /* take the mutex to block the execution of the self_unregistering_handler */ + xSemaphoreTake(s_event_mutex, portMAX_DELAY); + + /* post 2 times the event on event base 1, event id 1 */ + ret = esp_event_post(s_test_base1, TEST_EVENT_BASE1_EV1, NULL, 0, pdMS_TO_TICKS(1000)); + printf("esp_event_post %d\n", ret); + TEST_ESP_OK(ret); + ret = esp_event_post(s_test_base1, TEST_EVENT_BASE1_EV1, NULL, 0, pdMS_TO_TICKS(1000)); + printf("esp_event_post %d\n", ret); + TEST_ESP_OK(ret); + + /* release the mutex to execute the self_unregistering_handler */ + xSemaphoreGive(s_event_mutex); + + /* make sure the handler was called only once */ + TEST_ASSERT(s_handler_triggered == 1); + + /* delete mutex */ + vSemaphoreDelete(s_event_mutex); + + /* reset the static variable in case the test gets called once more */ + s_handler_triggered = 0; + + /* delete loop */ + ret = esp_event_loop_delete_default(); + printf("esp_event_loop_delete_default %d\n", ret); + TEST_ESP_OK(ret); +} +#endif // CONFIG_ESP_EVENT_LOOP_PROFILING diff --git a/components/esp_event/test_apps/pytest_esp_event.py b/components/esp_event/test_apps/pytest_esp_event.py index 9057e2ff9f..ff04ca3a55 100644 --- a/components/esp_event/test_apps/pytest_esp_event.py +++ b/components/esp_event/test_apps/pytest_esp_event.py @@ -1,4 +1,4 @@ -# SPDX-FileCopyrightText: 2022-2023 Espressif Systems (Shanghai) CO LTD +# SPDX-FileCopyrightText: 2022-2025 Espressif Systems (Shanghai) CO LTD # SPDX-License-Identifier: CC0-1.0 import pytest from pytest_embedded import Dut @@ -29,3 +29,28 @@ def test_esp_event_posix_simulator(dut: Dut) -> None: dut.expect_exact('Press ENTER to see the list of tests.') dut.write('*') dut.expect(r'\d{2} Tests 0 Failures 0 Ignored', timeout=120) + + +@pytest.mark.esp32 +@pytest.mark.generic +def test_esp_event_profiling(dut: Dut) -> None: + dut.expect_exact('Press ENTER to see the list of tests.') + dut.write('"profiling reports valid values"') + # look for all references of handlers invoked at least 1 time + # with an execution time superior to 0 us + matches = dut.expect(r'HANDLER .+ inv:[1-9][0-9]{0,} time:[1-9][0-9]{0,} us', timeout=2) + matches_arr = matches.group().split(b'\r\n') + assert (len(matches_arr) == 3) + dut.expect('1 Tests 0 Failures 0 Ignored', timeout=120) + dut.expect_exact("Enter next test, or 'enter' to see menu") + + dut.write('"esp_event_dump does not show self unregistered handler"') + # look for 1 handlers never invoked + matches = dut.expect(r'HANDLER .+ inv:0 time:0 us', timeout=2) + matches_arr = matches.group().split(b'\r\n') + assert (len(matches_arr) == 1) + dut.expect('1 Tests 0 Failures 0 Ignored', timeout=120) + dut.expect_exact("Enter next test, or 'enter' to see menu") + + dut.write('"self unregistered handlers are never called again after they return"') + dut.expect('1 Tests 0 Failures 0 Ignored', timeout=120) diff --git a/components/esp_event/test_apps/sdkconfig.ci.no_isr_post b/components/esp_event/test_apps/sdkconfig.ci.no_isr_post new file mode 100644 index 0000000000..1b0464b258 --- /dev/null +++ b/components/esp_event/test_apps/sdkconfig.ci.no_isr_post @@ -0,0 +1,3 @@ +# This configuration checks the event loop if posting from ISR is disabled +CONFIG_ESP_TASK_WDT_INIT=n +CONFIG_POST_EVENTS_FROM_ISR=n