From 9c0cf3c28a0808163df58292566789a2d1529890 Mon Sep 17 00:00:00 2001 From: Rusty Eddy Date: Tue, 4 Feb 2020 07:38:44 -0800 Subject: [PATCH 1/5] Added semi-colon to esp_event_loop_create(...) Title sums it up. --- docs/en/api-reference/system/esp_event.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/en/api-reference/system/esp_event.rst b/docs/en/api-reference/system/esp_event.rst index dd969be70..958b0fe7a 100644 --- a/docs/en/api-reference/system/esp_event.rst +++ b/docs/en/api-reference/system/esp_event.rst @@ -54,7 +54,7 @@ In code, the flow above may look like as follows: esp_event_loop_handle_t loop_handle; - esp_event_loop_create(&loop_args, &loop_handle) + esp_event_loop_create(&loop_args, &loop_handle); // 3. Register event handler defined in (1). MY_EVENT_BASE and MY_EVENT_ID specifies a hypothetical // event that handler run_on_event should execute on when it gets posted to the loop. From 9b16cb75f863b4e4161e45a6ef97548edc4c5841 Mon Sep 17 00:00:00 2001 From: Xentec Date: Tue, 1 Oct 2019 01:31:09 +0200 Subject: [PATCH 2/5] esp_event: fix crash when unregistering a handler instance in itself When a handler instance is the last one in the list und unregisters itself, the handler iterator will be invalidated by entering free'd memory. Same applies for event base and id, if they become empty. Merges https://github.com/espressif/esp-idf/pull/4139 --- components/esp_event/esp_event.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/components/esp_event/esp_event.c b/components/esp_event/esp_event.c index 08e7cd250..f4628e694 100644 --- a/components/esp_event/esp_event.c +++ b/components/esp_event/esp_event.c @@ -526,30 +526,30 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick bool exec = false; - esp_event_handler_instance_t *handler; + esp_event_handler_instance_t *handler, *temp_handler; esp_event_loop_node_t *loop_node; - esp_event_base_node_t *base_node; - esp_event_id_node_t *id_node; + esp_event_base_node_t *base_node, *temp_base; + esp_event_id_node_t *id_node, *temp_id_node; SLIST_FOREACH(loop_node, &(loop->loop_nodes), next) { // Execute loop level handlers - SLIST_FOREACH(handler, &(loop_node->handlers), next) { + SLIST_FOREACH_SAFE(handler, &(loop_node->handlers), next, temp_handler) { handler_execute(loop, handler, post); exec |= true; } - SLIST_FOREACH(base_node, &(loop_node->base_nodes), next) { + SLIST_FOREACH_SAFE(base_node, &(loop_node->base_nodes), next, temp_base) { if (base_node->base == post.base) { // Execute base level handlers - SLIST_FOREACH(handler, &(base_node->handlers), next) { + SLIST_FOREACH_SAFE(handler, &(base_node->handlers), next, temp_handler) { handler_execute(loop, handler, post); exec |= true; } - SLIST_FOREACH(id_node, &(base_node->id_nodes), next) { - if(id_node->id == post.id) { + 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(handler, &(id_node->handlers), next) { + SLIST_FOREACH_SAFE(handler, &(id_node->handlers), next, temp_handler) { handler_execute(loop, handler, post); exec |= true; } From 0890ce91150134c40f519609a0afd29b5ff57091 Mon Sep 17 00:00:00 2001 From: Renz Christian Bagaporo Date: Fri, 18 Oct 2019 12:54:42 +0800 Subject: [PATCH 3/5] esp_event: iterate loop nodes safely as well --- components/esp_event/esp_event.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/components/esp_event/esp_event.c b/components/esp_event/esp_event.c index f4628e694..22774a880 100644 --- a/components/esp_event/esp_event.c +++ b/components/esp_event/esp_event.c @@ -527,11 +527,11 @@ esp_err_t esp_event_loop_run(esp_event_loop_handle_t event_loop, TickType_t tick bool exec = false; esp_event_handler_instance_t *handler, *temp_handler; - esp_event_loop_node_t *loop_node; + esp_event_loop_node_t *loop_node, *temp_node; esp_event_base_node_t *base_node, *temp_base; esp_event_id_node_t *id_node, *temp_id_node; - SLIST_FOREACH(loop_node, &(loop->loop_nodes), next) { + 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); From 2e3a949f3ae63c42472b0be88acdfdc06cc12363 Mon Sep 17 00:00:00 2001 From: Renz Christian Bagaporo Date: Tue, 8 Oct 2019 15:32:51 +0800 Subject: [PATCH 4/5] esp_event: test that handlers can unregister themselves --- components/esp_event/test/test_event.c | 67 ++++++++++++++++++++++++++ 1 file changed, 67 insertions(+) diff --git a/components/esp_event/test/test_event.c b/components/esp_event/test/test_event.c index d706db942..e5507865a 100644 --- a/components/esp_event/test/test_event.c +++ b/components/esp_event/test/test_event.c @@ -200,6 +200,7 @@ static void test_event_simple_handler_registration_task(void* args) vTaskDelete(NULL); } + static void test_handler_post_w_task(void* event_handler_arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { simple_arg_t* arg = (simple_arg_t*) event_handler_arg; @@ -251,6 +252,17 @@ static void test_handler_post_wo_task(void* event_handler_arg, esp_event_base_t } } +static void test_handler_unregister_itself(void* event_handler_arg, esp_event_base_t event_base, int32_t event_id, void* event_data) +{ + esp_event_loop_handle_t* loop = (esp_event_loop_handle_t*) event_data; + int* unregistered = (int*) event_handler_arg; + + (*unregistered) += (event_base == s_test_base1 ? 0 : 10) + event_id + 1; + + // Unregister this handler for this event + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_unregister_with(*loop, event_base, event_id, test_handler_unregister_itself)); +} + static void test_post_from_handler_loop_task(void* args) { esp_event_loop_handle_t event_loop = (esp_event_loop_handle_t) args; @@ -427,6 +439,61 @@ TEST_CASE("can unregister handler", "[event]") TEST_TEARDOWN(); } +TEST_CASE("handler can unregister itself", "[event]") +{ + /* this test aims to verify that handlers can unregister themselves */ + + TEST_SETUP(); + + esp_event_loop_handle_t loop; + esp_event_loop_args_t loop_args = test_event_get_default_loop_args(); + + loop_args.task_name = NULL; + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_create(&loop_args, &loop)); + + int unregistered = 0; + + /* + * s_test_base1, ev1 = 1 + * s_test_base1, ev2 = 2 + * s_test_base2, ev1 = 11 + * s_test_base2, ev2 = 12 + */ + int expected_unregistered = 0; + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base1, TEST_EVENT_BASE1_EV1, test_handler_unregister_itself, &unregistered)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base1, TEST_EVENT_BASE1_EV2, test_handler_unregister_itself, &unregistered)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base2, TEST_EVENT_BASE2_EV1, test_handler_unregister_itself, &unregistered)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_handler_register_with(loop, s_test_base2, TEST_EVENT_BASE2_EV2, test_handler_unregister_itself, &unregistered)); + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base1, TEST_EVENT_BASE1_EV2, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_run(loop, pdMS_TO_TICKS(10))); + expected_unregistered = 2; // base1, ev2 + TEST_ASSERT_EQUAL(expected_unregistered, unregistered); + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base1, TEST_EVENT_BASE1_EV1, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base2, TEST_EVENT_BASE2_EV1, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_run(loop, pdMS_TO_TICKS(10))); + expected_unregistered += 1 + 11; // base1, ev1 + base2, ev1 + TEST_ASSERT_EQUAL(expected_unregistered, unregistered); + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base2, TEST_EVENT_BASE2_EV2, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_run(loop, pdMS_TO_TICKS(10))); + expected_unregistered += 12; // base2, ev2 + TEST_ASSERT_EQUAL(expected_unregistered, unregistered); + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base1, TEST_EVENT_BASE1_EV1, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base1, TEST_EVENT_BASE1_EV2, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base2, TEST_EVENT_BASE2_EV1, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_post_to(loop, s_test_base2, TEST_EVENT_BASE2_EV2, &loop, sizeof(loop), portMAX_DELAY)); + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_run(loop, pdMS_TO_TICKS(10))); + TEST_ASSERT_EQUAL(expected_unregistered, unregistered); // all handlers unregistered + + TEST_ASSERT_EQUAL(ESP_OK, esp_event_loop_delete(loop)); + + TEST_TEARDOWN(); +} + TEST_CASE("can exit running loop at approximately the set amount of time", "[event]") { /* this test aims to verify that running loop does not block indefinitely in cases where From 84227aeac0aa5838f8ede9d899505bec71a35505 Mon Sep 17 00:00:00 2001 From: Renz Christian Bagaporo Date: Fri, 25 Oct 2019 13:14:05 +0800 Subject: [PATCH 5/5] esp_event: remove extra line from source file --- components/esp_event/test/test_event.c | 1 - 1 file changed, 1 deletion(-) diff --git a/components/esp_event/test/test_event.c b/components/esp_event/test/test_event.c index e5507865a..d0a06c737 100644 --- a/components/esp_event/test/test_event.c +++ b/components/esp_event/test/test_event.c @@ -200,7 +200,6 @@ static void test_event_simple_handler_registration_task(void* args) vTaskDelete(NULL); } - static void test_handler_post_w_task(void* event_handler_arg, esp_event_base_t event_base, int32_t event_id, void* event_data) { simple_arg_t* arg = (simple_arg_t*) event_handler_arg;