Skip to content

Commit

Permalink
Threading, Timers improvements (#3865)
Browse files Browse the repository at this point in the history 
* 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.
  • Loading branch information
@CookiePLMonster
CookiePLMonster authored Sep 7, 2024
1 parent 8672a1d commit 8c2223d
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 30 deletions.
2 changes: 1 addition & 1 deletion applications/services/loader/loader.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
34 changes: 22 additions & 12 deletions furi/core/event_loop.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down Expand Up @@ -178,18 +178,19 @@ 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);
}
}

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);
Expand Down Expand Up @@ -233,16 +234,19 @@ 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);
}
}

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);
}

/*
Expand All @@ -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);
}

/*
Expand Down Expand Up @@ -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) {
Expand Down
5 changes: 4 additions & 1 deletion furi/core/event_loop_timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

/*
Expand Down
21 changes: 11 additions & 10 deletions furi/core/thread.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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;
Expand Down
14 changes: 8 additions & 6 deletions furi/core/timer.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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) {
Expand All @@ -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);
}

Expand Down

0 comments on commit 8c2223d

Please sign in to comment.