From 8b2dfea925835012f2e6b4a6eba4ce82fce7ca6c Mon Sep 17 00:00:00 2001 From: Sergey Gavrilov Date: Tue, 28 Mar 2023 00:34:49 -0700 Subject: [PATCH] Improved thread lifecycle (#2534) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Core, Thread: mark thread to join from prvDeleteTCB * USB HAL: move vars to MEM2 * Core, Thread: cleanup sources * Cli: add magic delays on rx pipe error, prevent cli from consuming processor time * Furi: update thread documentation Co-authored-by: あく --- applications/services/cli/cli.c | 2 ++ firmware/targets/f7/furi_hal/furi_hal_usb.c | 5 ++- firmware/targets/f7/inc/FreeRTOSConfig.h | 5 +++ furi/core/thread.c | 35 ++++++++++++++------- furi/core/thread.h | 5 +++ furi/flipper.c | 4 +-- 6 files changed, 39 insertions(+), 17 deletions(-) diff --git a/applications/services/cli/cli.c b/applications/services/cli/cli.c index b68505c51..ad3bbd665 100644 --- a/applications/services/cli/cli.c +++ b/applications/services/cli/cli.c @@ -37,9 +37,11 @@ char cli_getc(Cli* cli) { if(cli->session != NULL) { if(cli->session->rx((uint8_t*)&c, 1, FuriWaitForever) == 0) { cli_reset(cli); + furi_delay_tick(10); } } else { cli_reset(cli); + furi_delay_tick(10); } return c; } diff --git a/firmware/targets/f7/furi_hal/furi_hal_usb.c b/firmware/targets/f7/furi_hal/furi_hal_usb.c index fc679114f..011add953 100644 --- a/firmware/targets/f7/furi_hal/furi_hal_usb.c +++ b/firmware/targets/f7/furi_hal/furi_hal_usb.c @@ -73,12 +73,11 @@ typedef enum { #define USB_SRV_ALL_EVENTS (UsbEventReset | UsbEventRequest | UsbEventMessage) PLACE_IN_SECTION("MB_MEM2") static UsbSrv usb = {0}; +PLACE_IN_SECTION("MB_MEM2") static uint32_t ubuf[0x20]; +PLACE_IN_SECTION("MB_MEM2") usbd_device udev; static const struct usb_string_descriptor dev_lang_desc = USB_ARRAY_DESC(USB_LANGID_ENG_US); -static uint32_t ubuf[0x20]; -usbd_device udev; - static int32_t furi_hal_usb_thread(void* context); static usbd_respond usb_descriptor_get(usbd_ctlreq* req, void** address, uint16_t* length); static void reset_evt(usbd_device* dev, uint8_t event, uint8_t ep); diff --git a/firmware/targets/f7/inc/FreeRTOSConfig.h b/firmware/targets/f7/inc/FreeRTOSConfig.h index 69ef9406b..9486f501c 100644 --- a/firmware/targets/f7/inc/FreeRTOSConfig.h +++ b/firmware/targets/f7/inc/FreeRTOSConfig.h @@ -58,6 +58,7 @@ extern uint32_t SystemCoreClock; #define configTIMER_SERVICE_TASK_NAME "TimersSrv" #define configIDLE_TASK_NAME "(-_-)" +#define configIDLE_TASK_STACK_DEPTH 128 /* Set the following definitions to 1 to include the API function, or zero to exclude the API function. */ @@ -138,3 +139,7 @@ standard names. */ #define traceTASK_SWITCHED_IN() \ extern void furi_hal_mpu_set_stack_protection(uint32_t* stack); \ furi_hal_mpu_set_stack_protection((uint32_t*)pxCurrentTCB->pxStack) + +#define portCLEAN_UP_TCB(pxTCB) \ + extern void furi_thread_cleanup_tcb_event(TaskHandle_t task); \ + furi_thread_cleanup_tcb_event(pxTCB) diff --git a/furi/core/thread.c b/furi/core/thread.c index b45651c29..d78070d61 100644 --- a/furi/core/thread.c +++ b/furi/core/thread.c @@ -24,7 +24,6 @@ struct FuriThreadStdout { }; struct FuriThread { - bool is_service; FuriThreadState state; int32_t ret; @@ -37,14 +36,19 @@ struct FuriThread { char* name; char* appid; - configSTACK_DEPTH_TYPE stack_size; FuriThreadPriority priority; TaskHandle_t task_handle; - bool heap_trace_enabled; size_t heap_size; FuriThreadStdout output; + + // Keep all non-alignable byte types in one place, + // this ensures that the size of this structure is minimal + bool is_service; + bool heap_trace_enabled; + + configSTACK_DEPTH_TYPE stack_size; }; static size_t __furi_thread_stdout_write(FuriThread* thread, const char* data, size_t size); @@ -107,14 +111,8 @@ static void furi_thread_body(void* context) { // flush stdout __furi_thread_stdout_flush(thread); - // from here we can't use thread pointer furi_thread_set_state(thread, FuriThreadStateStopped); - // clear thread local storage - furi_assert(pvTaskGetThreadLocalStoragePointer(NULL, 0) != NULL); - vTaskSetThreadLocalStoragePointer(NULL, 0, NULL); - - thread->task_handle = NULL; vTaskDelete(NULL); furi_thread_catch(); } @@ -249,11 +247,11 @@ void furi_thread_start(FuriThread* thread) { furi_assert(thread); furi_assert(thread->callback); furi_assert(thread->state == FuriThreadStateStopped); - furi_assert(thread->stack_size > 0 && thread->stack_size < 0xFFFF * 4); + furi_assert(thread->stack_size > 0 && thread->stack_size < (UINT16_MAX * sizeof(StackType_t))); furi_thread_set_state(thread, FuriThreadStateStarting); - uint32_t stack = thread->stack_size / 4; + uint32_t stack = thread->stack_size / sizeof(StackType_t); UBaseType_t priority = thread->priority ? thread->priority : FuriThreadPriorityNormal; if(thread->is_service) { thread->task_handle = xTaskCreateStatic( @@ -273,12 +271,25 @@ void furi_thread_start(FuriThread* thread) { furi_check(thread->task_handle); } +void furi_thread_cleanup_tcb_event(TaskHandle_t task) { + FuriThread* thread = pvTaskGetThreadLocalStoragePointer(task, 0); + if(thread) { + // clear thread local storage + vTaskSetThreadLocalStoragePointer(task, 0, NULL); + + thread->task_handle = NULL; + } +} + bool furi_thread_join(FuriThread* thread) { furi_assert(thread); furi_check(furi_thread_get_current() != thread); - // Wait for thread to stop + // !!! IMPORTANT NOTICE !!! + // + // If your thread exited, but your app stuck here: some other thread uses + // all cpu time, which delays kernel from releasing task handle while(thread->task_handle) { furi_delay_ms(10); } diff --git a/furi/core/thread.h b/furi/core/thread.h index 8f4398419..b11a225b5 100644 --- a/furi/core/thread.h +++ b/furi/core/thread.h @@ -75,6 +75,8 @@ FuriThread* furi_thread_alloc_ex( void* context); /** Release FuriThread + * + * @warning see furi_thread_join * * @param thread FuriThread instance */ @@ -173,6 +175,9 @@ FuriThreadState furi_thread_get_state(FuriThread* thread); void furi_thread_start(FuriThread* thread); /** Join FuriThread + * + * @warning Use this method only when CPU is not busy(Idle task receives + * control), otherwise it will wait forever. * * @param thread FuriThread instance * diff --git a/furi/flipper.c b/furi/flipper.c index 5c2ad8138..8806ce27f 100644 --- a/furi/flipper.c +++ b/furi/flipper.c @@ -54,8 +54,8 @@ void vApplicationGetIdleTaskMemory( StackType_t** stack_ptr, uint32_t* stack_size) { *tcb_ptr = memmgr_alloc_from_pool(sizeof(StaticTask_t)); - *stack_ptr = memmgr_alloc_from_pool(sizeof(StackType_t) * configMINIMAL_STACK_SIZE); - *stack_size = configMINIMAL_STACK_SIZE; + *stack_ptr = memmgr_alloc_from_pool(sizeof(StackType_t) * configIDLE_TASK_STACK_DEPTH); + *stack_size = configIDLE_TASK_STACK_DEPTH; } void vApplicationGetTimerTaskMemory(