From 8c2223df5dfc9c5b1119adee634e53610ca504d1 Mon Sep 17 00:00:00 2001 From: Silent Date: Sat, 7 Sep 2024 20:18:51 +0200 Subject: [PATCH] Threading, Timers improvements (#3865) * FuriTimer: Use a local variable to wait for deletion This combines the current synchronous behaviour (as we could have deferred the free call too) with a smaller FuriTimer - it's safe to pass a pointer to a local variable to this pending timer call, because we know it'll be finished before the caller returns * Tighten the use of FuriThread* vs FuriThreadId Event loop and Loader mixed those two, but the fact those are aliases should be an implementation detail. For this reason, thread.c is still allowed to mix them freely. --- applications/services/loader/loader.c | 2 +- furi/core/event_loop.c | 34 +++++++++++++++++---------- furi/core/event_loop_timer.c | 5 +++- furi/core/thread.c | 21 +++++++++-------- furi/core/timer.c | 14 ++++++----- 5 files changed, 46 insertions(+), 30 deletions(-) diff --git a/applications/services/loader/loader.c b/applications/services/loader/loader.c index 0f4cc4a0c24..b76b38c25c9 100644 --- a/applications/services/loader/loader.c +++ b/applications/services/loader/loader.c @@ -717,7 +717,7 @@ static bool loader_do_signal(Loader* loader, uint32_t signal, void* arg) { static bool loader_do_get_application_name(Loader* loader, FuriString* name) { if(loader_is_application_running(loader)) { - furi_string_set(name, furi_thread_get_name(loader->app.thread)); + furi_string_set(name, furi_thread_get_name(furi_thread_get_id(loader->app.thread))); return true; } diff --git a/furi/core/event_loop.c b/furi/core/event_loop.c index 2a6cd51d32e..f4f008a71b6 100644 --- a/furi/core/event_loop.c +++ b/furi/core/event_loop.c @@ -71,9 +71,9 @@ FuriEventLoop* furi_event_loop_alloc(void) { PendingQueue_init(instance->pending_queue); // 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); + TaskHandle_t task = (TaskHandle_t)instance->thread_id; + xTaskNotifyStateClearIndexed(task, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX); + ulTaskNotifyValueClearIndexed(task, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, 0xFFFFFFFF); return instance; } @@ -178,7 +178,7 @@ static void furi_event_loop_process_waiting_list(FuriEventLoop* instance) { static void furi_event_loop_restore_flags(FuriEventLoop* instance, uint32_t flags) { if(flags) { xTaskNotifyIndexed( - instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, flags, eSetBits); + (TaskHandle_t)instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, flags, eSetBits); } } @@ -186,10 +186,11 @@ void furi_event_loop_run(FuriEventLoop* instance) { furi_check(instance); furi_check(instance->thread_id == furi_thread_get_current_id()); + FuriThread* thread = furi_thread_get_current(); + // Set the default signal callback if none was previously set - if(furi_thread_get_signal_callback(instance->thread_id) == NULL) { - furi_thread_set_signal_callback( - instance->thread_id, furi_event_loop_signal_callback, instance); + if(furi_thread_get_signal_callback(thread) == NULL) { + furi_thread_set_signal_callback(thread, furi_event_loop_signal_callback, instance); } furi_event_loop_init_tick(instance); @@ -233,8 +234,8 @@ void furi_event_loop_run(FuriEventLoop* instance) { } // Disable the default signal callback - if(furi_thread_get_signal_callback(instance->thread_id) == furi_event_loop_signal_callback) { - furi_thread_set_signal_callback(instance->thread_id, NULL, NULL); + if(furi_thread_get_signal_callback(thread) == furi_event_loop_signal_callback) { + furi_thread_set_signal_callback(thread, NULL, NULL); } } @@ -242,7 +243,10 @@ void furi_event_loop_stop(FuriEventLoop* instance) { furi_check(instance); xTaskNotifyIndexed( - instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, FuriEventLoopFlagStop, eSetBits); + (TaskHandle_t)instance->thread_id, + FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, + FuriEventLoopFlagStop, + eSetBits); } /* @@ -265,7 +269,10 @@ void furi_event_loop_pend_callback( PendingQueue_push_front(instance->pending_queue, item); xTaskNotifyIndexed( - instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, FuriEventLoopFlagPending, eSetBits); + (TaskHandle_t)instance->thread_id, + FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, + FuriEventLoopFlagPending, + eSetBits); } /* @@ -473,7 +480,10 @@ static void furi_event_loop_item_notify(FuriEventLoopItem* instance) { FURI_CRITICAL_EXIT(); xTaskNotifyIndexed( - owner->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, FuriEventLoopFlagEvent, eSetBits); + (TaskHandle_t)owner->thread_id, + FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, + FuriEventLoopFlagEvent, + eSetBits); } static bool furi_event_loop_item_is_waiting(FuriEventLoopItem* instance) { diff --git a/furi/core/event_loop_timer.c b/furi/core/event_loop_timer.c index 03b6c513234..f4a79bb4f17 100644 --- a/furi/core/event_loop_timer.c +++ b/furi/core/event_loop_timer.c @@ -65,7 +65,10 @@ static void furi_event_loop_timer_enqueue_request( TimerQueue_push_back(instance->timer_queue, timer); xTaskNotifyIndexed( - instance->thread_id, FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, FuriEventLoopFlagTimer, eSetBits); + (TaskHandle_t)instance->thread_id, + FURI_EVENT_LOOP_FLAG_NOTIFY_INDEX, + FuriEventLoopFlagTimer, + eSetBits); } /* diff --git a/furi/core/thread.c b/furi/core/thread.c index 69c6b0f04e8..60cc628acb3 100644 --- a/furi/core/thread.c +++ b/furi/core/thread.c @@ -97,7 +97,7 @@ static void furi_thread_body(void* context) { furi_thread_set_state(thread, FuriThreadStateRunning); if(thread->heap_trace_enabled == true) { - memmgr_heap_enable_thread_trace(thread); + memmgr_heap_enable_thread_trace((FuriThreadId)thread); } thread->ret = thread->callback(thread->context); @@ -106,14 +106,14 @@ static void furi_thread_body(void* context) { if(thread->heap_trace_enabled == true) { furi_delay_ms(33); - thread->heap_size = memmgr_heap_get_thread_memory(thread); + thread->heap_size = memmgr_heap_get_thread_memory((FuriThreadId)thread); furi_log_print_format( thread->heap_size ? FuriLogLevelError : FuriLogLevelInfo, TAG, "%s allocation balance: %zu", thread->name ? thread->name : "Thread", thread->heap_size); - memmgr_heap_disable_thread_trace(thread); + memmgr_heap_disable_thread_trace((FuriThreadId)thread); } furi_check(thread->state == FuriThreadStateRunning); @@ -275,7 +275,7 @@ void furi_thread_set_priority(FuriThread* thread, FuriThreadPriority priority) { FuriThreadPriority furi_thread_get_priority(FuriThread* thread) { furi_check(thread); - TaskHandle_t hTask = furi_thread_get_id(thread); + TaskHandle_t hTask = (TaskHandle_t)thread; return (FuriThreadPriority)uxTaskPriorityGet(hTask); } @@ -390,7 +390,7 @@ bool furi_thread_join(FuriThread* thread) { FuriThreadId furi_thread_get_id(FuriThread* thread) { furi_check(thread); - return thread; + return (FuriThreadId)thread; } void furi_thread_enable_heap_trace(FuriThread* thread) { @@ -418,7 +418,7 @@ int32_t furi_thread_get_return_code(FuriThread* thread) { } FuriThreadId furi_thread_get_current_id(void) { - return xTaskGetCurrentTaskHandle(); + return (FuriThreadId)xTaskGetCurrentTaskHandle(); } FuriThread* furi_thread_get_current(void) { @@ -624,15 +624,16 @@ bool furi_thread_enumerate(FuriThreadList* thread_list) { FuriThreadListItem* item = furi_thread_list_get_or_insert(thread_list, (FuriThread*)task[i].xHandle); - item->thread = (FuriThreadId)task[i].xHandle; - item->app_id = furi_thread_get_appid(item->thread); + FuriThreadId thread_id = (FuriThreadId)task[i].xHandle; + item->thread = (FuriThread*)thread_id; + item->app_id = furi_thread_get_appid(thread_id); item->name = task[i].pcTaskName; item->priority = task[i].uxCurrentPriority; item->stack_address = (uint32_t)tcb->pxStack; - size_t thread_heap = memmgr_heap_get_thread_memory(item->thread); + size_t thread_heap = memmgr_heap_get_thread_memory(thread_id); item->heap = thread_heap == MEMMGR_HEAP_UNKNOWN ? 0u : thread_heap; item->stack_size = (tcb->pxEndOfStack - tcb->pxStack + 1) * sizeof(StackType_t); - item->stack_min_free = furi_thread_get_stack_space(item->thread); + item->stack_min_free = furi_thread_get_stack_space(thread_id); item->state = furi_thread_state_name(task[i].eCurrentState); item->counter_previous = item->counter_current; item->counter_current = task[i].ulRunTimeCounter; diff --git a/furi/core/timer.c b/furi/core/timer.c index 1ca56f0fa45..4ed9139583b 100644 --- a/furi/core/timer.c +++ b/furi/core/timer.c @@ -9,7 +9,6 @@ struct FuriTimer { StaticTimer_t container; FuriTimerCallback cb_func; void* cb_context; - volatile bool can_be_removed; }; // IMPORTANT: container MUST be the FIRST struct member @@ -42,9 +41,8 @@ static void furi_timer_epilogue(void* context, uint32_t arg) { furi_assert(context); UNUSED(arg); - FuriTimer* instance = context; - - instance->can_be_removed = true; + volatile bool* can_be_removed = context; + *can_be_removed = true; } void furi_timer_free(FuriTimer* instance) { @@ -53,9 +51,13 @@ void furi_timer_free(FuriTimer* instance) { TimerHandle_t hTimer = (TimerHandle_t)instance; furi_check(xTimerDelete(hTimer, portMAX_DELAY) == pdPASS); - furi_check(xTimerPendFunctionCall(furi_timer_epilogue, instance, 0, portMAX_DELAY) == pdPASS); - while(!instance->can_be_removed) { + volatile bool can_be_removed = false; + furi_check( + xTimerPendFunctionCall(furi_timer_epilogue, (void*)&can_be_removed, 0, portMAX_DELAY) == + pdPASS); + + while(!can_be_removed) { furi_delay_tick(2); }