Improved thread lifecycle (#2534)

* 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: あく <[email protected]>

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

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

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)

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

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
  *

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(