Furi: Fix EventLoop state persisting on same thread after free (#3711)

* Furi: Fix EventLoop state persisting on same thread after free * Furi: clear event loop notification state and value on allocation, report unprocessed events on free * UnitTests: add multiple event loop runs in one thread test Co-authored-by: Aleksandr Kutuzov <alleteam@gmail.com>
flipperdevices · Jun 14, 2024 · 12c1d10 · 12c1d10
1 parent ca8517a
commit 12c1d10
Show file tree

Hide file tree

Showing 3 changed files with 70 additions and 4 deletions.
diff --git a/applications/debug/unit_tests/tests/furi/furi_event_loop.c b/applications/debug/unit_tests/tests/furi/furi_event_loop.c
@@ -2,6 +2,9 @@
 #include <furi.h>
 #include <furi_hal.h>
 
+#include <FreeRTOS.h>
+#include <task.h>
+
 #define TAG "TestFuriEventLoop"
 
 #define EVENT_LOOP_EVENT_COUNT (256u)
@@ -53,11 +56,31 @@ bool test_furi_event_loop_producer_mq_callback(FuriMessageQueue* queue, void* co
 int32_t test_furi_event_loop_producer(void* p) {
     furi_check(p);
 
-    FURI_LOG_I(TAG, "producer start");
-
     TestFuriData* data = p;
 
+    FURI_LOG_I(TAG, "producer start 1st run");
+
+    data->producer_event_loop = furi_event_loop_alloc();
+    furi_event_loop_message_queue_subscribe(
+        data->producer_event_loop,
+        data->mq,
+        FuriEventLoopEventOut,
+        test_furi_event_loop_producer_mq_callback,
+        data);
+
+    furi_event_loop_run(data->producer_event_loop);
+
+    // 2 EventLoop index, 0xFFFFFFFF - all possible flags, emulate uncleared flags
+    xTaskNotifyIndexed(xTaskGetCurrentTaskHandle(), 2, 0xFFFFFFFF, eSetBits);
+
+    furi_event_loop_message_queue_unsubscribe(data->producer_event_loop, data->mq);
+    furi_event_loop_free(data->producer_event_loop);
+
+    FURI_LOG_I(TAG, "producer start 2nd run");
+
+    data->producer_counter = 0;
     data->producer_event_loop = furi_event_loop_alloc();
+
     furi_event_loop_message_queue_subscribe(
         data->producer_event_loop,
         data->mq,
@@ -109,10 +132,29 @@ bool test_furi_event_loop_consumer_mq_callback(FuriMessageQueue* queue, void* co
 int32_t test_furi_event_loop_consumer(void* p) {
     furi_check(p);
 
-    FURI_LOG_I(TAG, "consumer start");
-
     TestFuriData* data = p;
 
+    FURI_LOG_I(TAG, "consumer start 1st run");
+
+    data->consumer_event_loop = furi_event_loop_alloc();
+    furi_event_loop_message_queue_subscribe(
+        data->consumer_event_loop,
+        data->mq,
+        FuriEventLoopEventIn,
+        test_furi_event_loop_consumer_mq_callback,
+        data);
+
+    furi_event_loop_run(data->consumer_event_loop);
+
+    // 2 EventLoop index, 0xFFFFFFFF - all possible flags, emulate uncleared flags
+    xTaskNotifyIndexed(xTaskGetCurrentTaskHandle(), 2, 0xFFFFFFFF, eSetBits);
+
+    furi_event_loop_message_queue_unsubscribe(data->consumer_event_loop, data->mq);
+    furi_event_loop_free(data->consumer_event_loop);
+
+    FURI_LOG_I(TAG, "consumer start 2nd run");
+
+    data->consumer_counter = 0;
     data->consumer_event_loop = furi_event_loop_alloc();
     furi_event_loop_message_queue_subscribe(
         data->consumer_event_loop,
@@ -156,6 +198,7 @@ void test_furi_event_loop(void) {
 
     // The test itself
     mu_assert_int_eq(data.producer_counter, data.consumer_counter);
+    mu_assert_int_eq(data.producer_counter, EVENT_LOOP_EVENT_COUNT);
 
     // Release memory
     furi_thread_free(consumer_thread);

diff --git a/applications/debug/unit_tests/unit_test_api_table_i.h b/applications/debug/unit_tests/unit_test_api_table_i.h
@@ -3,6 +3,7 @@
 #include <nfc/protocols/iso15693_3/iso15693_3_poller_i.h>
 #include <FreeRTOS.h>
 #include <FreeRTOS-Kernel/include/queue.h>
+#include <task.h>
 
 #include <rpc/rpc_i.h>
 #include <flipper.pb.h>
@@ -18,6 +19,11 @@ static constexpr auto unit_tests_api_table = sort(create_array_t<sym_entry>(
     API_METHOD(iso15693_3_poller_get_data, const Iso15693_3Data*, (Iso15693_3Poller*)),
     API_METHOD(rpc_system_storage_get_error, PB_CommandStatus, (FS_Error)),
     API_METHOD(xQueueSemaphoreTake, BaseType_t, (QueueHandle_t, TickType_t)),
+    API_METHOD(
+        xTaskGenericNotify,
+        BaseType_t,
+        (TaskHandle_t, UBaseType_t, uint32_t, eNotifyAction, uint32_t*)),
+    API_METHOD(xTaskGetCurrentTaskHandle, TaskHandle_t, ()),
     API_METHOD(vQueueDelete, void, (QueueHandle_t)),
     API_METHOD(
         xQueueGenericCreateStatic,

diff --git a/furi/core/event_loop.c b/furi/core/event_loop.c
@@ -1,6 +1,7 @@
 #include "event_loop_i.h"
 #include "message_queue_i.h"
 
+#include "log.h"
 #include "check.h"
 #include "thread.h"
 
@@ -10,6 +11,8 @@
 #include <FreeRTOS.h>
 #include <task.h>
 
+#define TAG "FuriEventLoop"
+
 struct FuriEventLoopItem {
     // Source
     FuriEventLoop* owner;
@@ -99,9 +102,15 @@ FuriEventLoop* furi_event_loop_alloc(void) {
     FuriEventLoop* instance = malloc(sizeof(FuriEventLoop));
 
     instance->thread_id = furi_thread_get_current_id();
+
     FuriEventLoopTree_init(instance->tree);
     WaitingList_init(instance->waiting_list);
 
+    // Clear notification state and value
+    xTaskNotifyStateClearIndexed(instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX);
+    ulTaskNotifyValueClearIndexed(
+        instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, 0xFFFFFFFF);
+
     return instance;
 }
 
@@ -110,6 +119,14 @@ void furi_event_loop_free(FuriEventLoop* instance) {
     furi_check(instance->thread_id == furi_thread_get_current_id());
 
     FuriEventLoopTree_clear(instance->tree);
+
+    uint32_t flags = 0;
+    BaseType_t ret = xTaskNotifyWaitIndexed(
+        FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, 0, FuriEventLoopFlagAll, &flags, 0);
+    if(ret == pdTRUE) {
+        FURI_LOG_D(TAG, "Some events was not processed: 0x%lx", flags);
+    }
+
     free(instance);
 }