[FL-3289] Various Furi/FuriHal bug fixes and improvements (#2637)

* Furi: properly handle thread free before TCB scrapping, add furi_free - more invasive version of free to memmgr. FuriHal: add DWT comparator api to cortex. Updater, RPC: refactor various thread shanenigans. Code cleanup.
* Rollback free macros and related changes

applications/services/loader/loader.c

@@ -248,6 +248,7 @@ static void loader_do_app_closed(Loader* loader) {
     free(loader->app.name);
     loader->app.name = NULL;
 
+    furi_thread_join(loader->app.thread);
     furi_thread_free(loader->app.thread);
     loader->app.thread = NULL;
 }

applications/services/rpc/rpc.c

@@ -326,31 +326,35 @@ static int32_t rpc_session_worker(void* context) {
     return 0;
 }
 
-static void rpc_session_free_callback(FuriThreadState thread_state, void* context) {
-    furi_assert(context);
-
+static void rpc_session_thread_pending_callback(void* context, uint32_t arg) {
+    UNUSED(arg);
     RpcSession* session = (RpcSession*)context;
 
-    if(thread_state == FuriThreadStateStopped) {
-        for(size_t i = 0; i < COUNT_OF(rpc_systems); ++i) {
-            if(rpc_systems[i].free) {
-                rpc_systems[i].free(session->system_contexts[i]);
-            }
-        }
-        free(session->system_contexts);
-        free(session->decoded_message);
-        RpcHandlerDict_clear(session->handlers);
-        furi_stream_buffer_free(session->stream);
-
-        furi_mutex_acquire(session->callbacks_mutex, FuriWaitForever);
-        if(session->terminated_callback) {
-            session->terminated_callback(session->context);
+    for(size_t i = 0; i < COUNT_OF(rpc_systems); ++i) {
+        if(rpc_systems[i].free) {
+            (rpc_systems[i].free)(session->system_contexts[i]);
         }
-        furi_mutex_release(session->callbacks_mutex);
+    }
+    free(session->system_contexts);
+    free(session->decoded_message);
+    RpcHandlerDict_clear(session->handlers);
+    furi_stream_buffer_free(session->stream);
 
-        furi_mutex_free(session->callbacks_mutex);
-        furi_thread_free(session->thread);
-        free(session);
+    furi_mutex_acquire(session->callbacks_mutex, FuriWaitForever);
+    if(session->terminated_callback) {
+        session->terminated_callback(session->context);
+    }
+    furi_mutex_release(session->callbacks_mutex);
+
+    furi_mutex_free(session->callbacks_mutex);
+    furi_thread_join(session->thread);
+    furi_thread_free(session->thread);
+    free(session);
+}
+
+static void rpc_session_thread_state_callback(FuriThreadState thread_state, void* context) {
+    if(thread_state == FuriThreadStateStopped) {
+        furi_timer_pending_callback(rpc_session_thread_pending_callback, context, 0);
     }
 }
 
@@ -385,7 +389,7 @@ RpcSession* rpc_session_open(Rpc* rpc, RpcOwner owner) {
     session->thread = furi_thread_alloc_ex("RpcSessionWorker", 3072, rpc_session_worker, session);
 
     furi_thread_set_state_context(session->thread, session);
-    furi_thread_set_state_callback(session->thread, rpc_session_free_callback);
+    furi_thread_set_state_callback(session->thread, rpc_session_thread_state_callback);
 
     furi_thread_start(session->thread);
 

applications/services/storage/storage_external_api.c

@@ -803,6 +803,7 @@ void storage_file_free(File* file) {
 }
 
 FuriPubSub* storage_get_pubsub(Storage* storage) {
+    furi_assert(storage);
     return storage->pubsub;
 }
 

applications/services/storage/storages/storage_ext.c

@@ -337,6 +337,7 @@ static bool storage_ext_file_close(void* ctx, File* file) {
     file->internal_error_id = f_close(file_data);
     file->error_id = storage_ext_parse_error(file->internal_error_id);
     free(file_data);
+    storage_set_storage_file_data(file, NULL, storage);
     return (file->error_id == FSE_OK);
 }
 

applications/settings/power_settings_app/views/battery_info.c

@@ -53,7 +53,9 @@ static void draw_battery(Canvas* canvas, BatteryInfoModel* data, int x, int y) {
             (uint32_t)(data->vbus_voltage),
             (uint32_t)(data->vbus_voltage * 10) % 10,
             current);
-    } else if(current < 0) {
+    } else if(current < -5) {
+        // Often gauge reports anything in the range 1~5ma as 5ma
+        // That brings confusion, so we'll treat it as Napping
         snprintf(
             emote,
             sizeof(emote),

applications/system/updater/cli/updater_cli.c

@@ -85,22 +85,10 @@ static void updater_cli_ep(Cli* cli, FuriString* args, void* context) {
     updater_cli_help(args);
 }
 
-static int32_t updater_spawner_thread_worker(void* arg) {
+static void updater_start_app(void* context, uint32_t arg) {
+    UNUSED(context);
     UNUSED(arg);
-    Loader* loader = furi_record_open(RECORD_LOADER);
-    loader_start(loader, "UpdaterApp", NULL);
-    furi_record_close(RECORD_LOADER);
-    return 0;
-}
 
-static void updater_spawner_thread_cleanup(FuriThreadState state, void* context) {
-    FuriThread* thread = context;
-    if(state == FuriThreadStateStopped) {
-        furi_thread_free(thread);
-    }
-}
-
-static void updater_start_app() {
     FuriHalRtcBootMode mode = furi_hal_rtc_get_boot_mode();
     if((mode != FuriHalRtcBootModePreUpdate) && (mode != FuriHalRtcBootModePostUpdate)) {
         return;
@@ -110,11 +98,9 @@ static void updater_start_app() {
      * inside loader process, at startup. 
      * So, accessing its record would cause a deadlock 
      */
-    FuriThread* thread =
-        furi_thread_alloc_ex("UpdateAppSpawner", 768, updater_spawner_thread_worker, NULL);
-    furi_thread_set_state_callback(thread, updater_spawner_thread_cleanup);
-    furi_thread_set_state_context(thread, thread);
-    furi_thread_start(thread);
+    Loader* loader = furi_record_open(RECORD_LOADER);
+    loader_start(loader, "UpdaterApp", NULL);
+    furi_record_close(RECORD_LOADER);
 }
 
 void updater_on_system_start() {
@@ -126,7 +112,7 @@ void updater_on_system_start() {
     UNUSED(updater_cli_ep);
 #endif
 #ifndef FURI_RAM_EXEC
-    updater_start_app();
+    furi_timer_pending_callback(updater_start_app, NULL, 0);
 #else
     UNUSED(updater_start_app);
 #endif

applications/system/updater/util/update_task_worker_flasher.c

@@ -346,7 +346,11 @@ int32_t update_task_worker_flash_writer(void* context) {
         furi_hal_rtc_set_boot_mode(FuriHalRtcBootModePostUpdate);
         // Format LFS before restoring backup on next boot
         furi_hal_rtc_set_flag(FuriHalRtcFlagFactoryReset);
-
+#ifdef FURI_NDEBUG
+        // Production
+        furi_hal_rtc_set_log_level(FuriLogLevelDefault);
+        furi_hal_rtc_reset_flag(FuriHalRtcFlagDebug);
+#endif
         update_task_set_progress(update_task, UpdateTaskStageCompleted, 100);
         success = true;
     } while(false);

firmware/targets/f18/api_symbols.csv

@@ -1,5 +1,5 @@
 entry,status,name,type,params
-Version,+,24.0,,
+Version,+,26.0,,
 Header,+,applications/services/bt/bt_service/bt.h,,
 Header,+,applications/services/cli/cli.h,,
 Header,+,applications/services/cli/cli_vcp.h,,
@@ -892,6 +892,8 @@ Function,+,furi_hal_console_puts,void,const char*
 Function,+,furi_hal_console_set_tx_callback,void,"FuriHalConsoleTxCallback, void*"
 Function,+,furi_hal_console_tx,void,"const uint8_t*, size_t"
 Function,+,furi_hal_console_tx_with_new_line,void,"const uint8_t*, size_t"
+Function,+,furi_hal_cortex_comp_enable,void,"FuriHalCortexComp, FuriHalCortexCompFunction, uint32_t, uint32_t, FuriHalCortexCompSize"
+Function,+,furi_hal_cortex_comp_reset,void,FuriHalCortexComp
 Function,+,furi_hal_cortex_delay_us,void,uint32_t
 Function,-,furi_hal_cortex_init_early,void,
 Function,+,furi_hal_cortex_instructions_per_microsecond,uint32_t,
@@ -1278,7 +1280,7 @@ Function,+,furi_thread_set_priority,void,"FuriThread*, FuriThreadPriority"
 Function,+,furi_thread_set_stack_size,void,"FuriThread*, size_t"
 Function,+,furi_thread_set_state_callback,void,"FuriThread*, FuriThreadStateCallback"
 Function,+,furi_thread_set_state_context,void,"FuriThread*, void*"
-Function,+,furi_thread_set_stdout_callback,_Bool,FuriThreadStdoutWriteCallback
+Function,+,furi_thread_set_stdout_callback,void,FuriThreadStdoutWriteCallback
 Function,+,furi_thread_start,void,FuriThread*
 Function,+,furi_thread_stdout_flush,int32_t,
 Function,+,furi_thread_stdout_write,size_t,"const char*, size_t"
@@ -1287,6 +1289,7 @@ Function,+,furi_thread_yield,void,
 Function,+,furi_timer_alloc,FuriTimer*,"FuriTimerCallback, FuriTimerType, void*"
 Function,+,furi_timer_free,void,FuriTimer*
 Function,+,furi_timer_is_running,uint32_t,FuriTimer*
+Function,+,furi_timer_pending_callback,void,"FuriTimerPendigCallback, void*, uint32_t"
 Function,+,furi_timer_start,FuriStatus,"FuriTimer*, uint32_t"
 Function,+,furi_timer_stop,FuriStatus,FuriTimer*
 Function,-,fwrite,size_t,"const void*, size_t, size_t, FILE*"

firmware/targets/f7/api_symbols.csv

@@ -1,5 +1,5 @@
 entry,status,name,type,params
-Version,+,24.0,,
+Version,+,26.0,,
 Header,+,applications/services/bt/bt_service/bt.h,,
 Header,+,applications/services/cli/cli.h,,
 Header,+,applications/services/cli/cli_vcp.h,,
@@ -1073,6 +1073,8 @@ Function,+,furi_hal_console_puts,void,const char*
 Function,+,furi_hal_console_set_tx_callback,void,"FuriHalConsoleTxCallback, void*"
 Function,+,furi_hal_console_tx,void,"const uint8_t*, size_t"
 Function,+,furi_hal_console_tx_with_new_line,void,"const uint8_t*, size_t"
+Function,+,furi_hal_cortex_comp_enable,void,"FuriHalCortexComp, FuriHalCortexCompFunction, uint32_t, uint32_t, FuriHalCortexCompSize"
+Function,+,furi_hal_cortex_comp_reset,void,FuriHalCortexComp
 Function,+,furi_hal_cortex_delay_us,void,uint32_t
 Function,-,furi_hal_cortex_init_early,void,
 Function,+,furi_hal_cortex_instructions_per_microsecond,uint32_t,
@@ -1562,7 +1564,7 @@ Function,+,furi_thread_set_priority,void,"FuriThread*, FuriThreadPriority"
 Function,+,furi_thread_set_stack_size,void,"FuriThread*, size_t"
 Function,+,furi_thread_set_state_callback,void,"FuriThread*, FuriThreadStateCallback"
 Function,+,furi_thread_set_state_context,void,"FuriThread*, void*"
-Function,+,furi_thread_set_stdout_callback,_Bool,FuriThreadStdoutWriteCallback
+Function,+,furi_thread_set_stdout_callback,void,FuriThreadStdoutWriteCallback
 Function,+,furi_thread_start,void,FuriThread*
 Function,+,furi_thread_stdout_flush,int32_t,
 Function,+,furi_thread_stdout_write,size_t,"const char*, size_t"
@@ -1571,6 +1573,7 @@ Function,+,furi_thread_yield,void,
 Function,+,furi_timer_alloc,FuriTimer*,"FuriTimerCallback, FuriTimerType, void*"
 Function,+,furi_timer_free,void,FuriTimer*
 Function,+,furi_timer_is_running,uint32_t,FuriTimer*
+Function,+,furi_timer_pending_callback,void,"FuriTimerPendigCallback, void*, uint32_t"
 Function,+,furi_timer_start,FuriStatus,"FuriTimer*, uint32_t"
 Function,+,furi_timer_stop,FuriStatus,FuriTimer*
 Function,-,fwrite,size_t,"const void*, size_t, size_t, FILE*"

firmware/targets/f7/furi_hal/furi_hal_cortex.c

@@ -1,11 +1,12 @@
 #include <furi_hal_cortex.h>
+#include <furi.h>
 
 #include <stm32wbxx.h>
 
 #define FURI_HAL_CORTEX_INSTRUCTIONS_PER_MICROSECOND (SystemCoreClock / 1000000)
 
 void furi_hal_cortex_init_early() {
-    CoreDebug->DEMCR |= CoreDebug_DEMCR_TRCENA_Msk;
+    CoreDebug->DEMCR |= (CoreDebug_DEMCR_TRCENA_Msk | CoreDebug_DEMCR_MON_EN_Msk);
     DWT->CTRL |= DWT_CTRL_CYCCNTENA_Msk;
     DWT->CYCCNT = 0U;
 
@@ -38,4 +39,71 @@ bool furi_hal_cortex_timer_is_expired(FuriHalCortexTimer cortex_timer) {
 void furi_hal_cortex_timer_wait(FuriHalCortexTimer cortex_timer) {
     while(!furi_hal_cortex_timer_is_expired(cortex_timer))
         ;
-}
+}
+
+// Duck ST
+#undef COMP0
+#undef COMP1
+#undef COMP2
+#undef COMP3
+
+void furi_hal_cortex_comp_enable(
+    FuriHalCortexComp comp,
+    FuriHalCortexCompFunction function,
+    uint32_t value,
+    uint32_t mask,
+    FuriHalCortexCompSize size) {
+    uint32_t function_reg = (uint32_t)function | ((uint32_t)size << 10);
+
+    switch(comp) {
+    case FuriHalCortexComp0:
+        (DWT->COMP0) = value;
+        (DWT->MASK0) = mask;
+        (DWT->FUNCTION0) = function_reg;
+        break;
+    case FuriHalCortexComp1:
+        (DWT->COMP1) = value;
+        (DWT->MASK1) = mask;
+        (DWT->FUNCTION1) = function_reg;
+        break;
+    case FuriHalCortexComp2:
+        (DWT->COMP2) = value;
+        (DWT->MASK2) = mask;
+        (DWT->FUNCTION2) = function_reg;
+        break;
+    case FuriHalCortexComp3:
+        (DWT->COMP3) = value;
+        (DWT->MASK3) = mask;
+        (DWT->FUNCTION3) = function_reg;
+        break;
+    default:
+        furi_crash("Invalid parameter");
+    }
+}
+
+void furi_hal_cortex_comp_reset(FuriHalCortexComp comp) {
+    switch(comp) {
+    case FuriHalCortexComp0:
+        (DWT->COMP0) = 0;
+        (DWT->MASK0) = 0;
+        (DWT->FUNCTION0) = 0;
+        break;
+    case FuriHalCortexComp1:
+        (DWT->COMP1) = 0;
+        (DWT->MASK1) = 0;
+        (DWT->FUNCTION1) = 0;
+        break;
+    case FuriHalCortexComp2:
+        (DWT->COMP2) = 0;
+        (DWT->MASK2) = 0;
+        (DWT->FUNCTION2) = 0;
+        break;
+    case FuriHalCortexComp3:
+        (DWT->COMP3) = 0;
+        (DWT->MASK3) = 0;
+        (DWT->FUNCTION3) = 0;
+        break;
+    default:
+        furi_crash("Invalid parameter");
+    }
+}

firmware/targets/furi_hal_include/furi_hal_cortex.h

@@ -56,6 +56,53 @@ bool furi_hal_cortex_timer_is_expired(FuriHalCortexTimer cortex_timer);
  */
 void furi_hal_cortex_timer_wait(FuriHalCortexTimer cortex_timer);
 
+typedef enum {
+    FuriHalCortexComp0,
+    FuriHalCortexComp1,
+    FuriHalCortexComp2,
+    FuriHalCortexComp3,
+} FuriHalCortexComp;
+
+typedef enum {
+    FuriHalCortexCompSizeWord = 0b10,
+    FuriHalCortexCompSizeHalfWord = 0b01,
+    FuriHalCortexCompSizeByte = 0b00,
+} FuriHalCortexCompSize;
+
+typedef enum {
+    FuriHalCortexCompFunctionPC = 0b100,
+    FuriHalCortexCompFunctionRead = 0b101,
+    FuriHalCortexCompFunctionWrite = 0b110,
+    FuriHalCortexCompFunctionReadWrite = 0b110,
+} FuriHalCortexCompFunction;
+
+/** Enable DWT comparator
+ * 
+ * Allows to programmatically set instruction/data breakpoints.
+ * 
+ * More details on how it works can be found in armv7m official documentation:
+ * https://developer.arm.com/documentation/ddi0403/d/Debug-Architecture/ARMv7-M-Debug/The-Data-Watchpoint-and-Trace-unit/The-DWT-comparators
+ * https://developer.arm.com/documentation/ddi0403/d/Debug-Architecture/ARMv7-M-Debug/The-Data-Watchpoint-and-Trace-unit/Comparator-Function-registers--DWT-FUNCTIONn
+ *
+ * @param[in]  comp      The Comparator
+ * @param[in]  function  The Comparator Function to use
+ * @param[in]  value     The value
+ * @param[in]  mask      The mask
+ * @param[in]  size      The size
+ */
+void furi_hal_cortex_comp_enable(
+    FuriHalCortexComp comp,
+    FuriHalCortexCompFunction function,
+    uint32_t value,
+    uint32_t mask,
+    FuriHalCortexCompSize size);
+
+/** Reset DWT comparator
+ *
+ * @param[in]  comp  The Comparator
+ */
+void furi_hal_cortex_comp_reset(FuriHalCortexComp comp);
+
 #ifdef __cplusplus
 }
 #endif

furi/core/thread.c

@@ -164,10 +164,13 @@ FuriThread* furi_thread_alloc_ex(
 
 void furi_thread_free(FuriThread* thread) {
     furi_assert(thread);
+
+    // Ensure that use join before free
     furi_assert(thread->state == FuriThreadStateStopped);
+    furi_assert(thread->task_handle == NULL);
 
-    if(thread->name) free((void*)thread->name);
-    if(thread->appid) free((void*)thread->appid);
+    if(thread->name) free(thread->name);
+    if(thread->appid) free(thread->appid);
     furi_string_free(thread->output.buffer);
 
     free(thread);
@@ -176,14 +179,14 @@ void furi_thread_free(FuriThread* thread) {
 void furi_thread_set_name(FuriThread* thread, const char* name) {
     furi_assert(thread);
     furi_assert(thread->state == FuriThreadStateStopped);
-    if(thread->name) free((void*)thread->name);
+    if(thread->name) free(thread->name);
     thread->name = name ? strdup(name) : NULL;
 }
 
 void furi_thread_set_appid(FuriThread* thread, const char* appid) {
     furi_assert(thread);
     furi_assert(thread->state == FuriThreadStateStopped);
-    if(thread->appid) free((void*)thread->appid);
+    if(thread->appid) free(thread->appid);
     thread->appid = appid ? strdup(appid) : NULL;
 }
 
@@ -276,7 +279,7 @@ void furi_thread_cleanup_tcb_event(TaskHandle_t task) {
     if(thread) {
         // clear thread local storage
         vTaskSetThreadLocalStoragePointer(task, 0, NULL);
-
+        furi_assert(thread->task_handle == task);
         thread->task_handle = NULL;
     }
 }
@@ -332,7 +335,6 @@ FuriThreadId furi_thread_get_current_id() {
 
 FuriThread* furi_thread_get_current() {
     FuriThread* thread = pvTaskGetThreadLocalStoragePointer(NULL, 0);
-    furi_assert(thread != NULL);
     return thread;
 }
 
@@ -579,24 +581,22 @@ static int32_t __furi_thread_stdout_flush(FuriThread* thread) {
     return 0;
 }
 
-bool furi_thread_set_stdout_callback(FuriThreadStdoutWriteCallback callback) {
+void furi_thread_set_stdout_callback(FuriThreadStdoutWriteCallback callback) {
     FuriThread* thread = furi_thread_get_current();
-
+    furi_assert(thread);
     __furi_thread_stdout_flush(thread);
     thread->output.write_callback = callback;
-
-    return true;
 }
 
 FuriThreadStdoutWriteCallback furi_thread_get_stdout_callback() {
     FuriThread* thread = furi_thread_get_current();
-
+    furi_assert(thread);
     return thread->output.write_callback;
 }
 
 size_t furi_thread_stdout_write(const char* data, size_t size) {
     FuriThread* thread = furi_thread_get_current();
-
+    furi_assert(thread);
     if(size == 0 || data == NULL) {
         return __furi_thread_stdout_flush(thread);
     } else {
@@ -619,7 +619,9 @@ size_t furi_thread_stdout_write(const char* data, size_t size) {
 }
 
 int32_t furi_thread_stdout_flush() {
-    return __furi_thread_stdout_flush(furi_thread_get_current());
+    FuriThread* thread = furi_thread_get_current();
+    furi_assert(thread);
+    return __furi_thread_stdout_flush(thread);
 }
 
 void furi_thread_suspend(FuriThreadId thread_id) {

furi/core/thread.h

@@ -233,7 +233,7 @@ FuriThreadId furi_thread_get_current_id();
 
 /** Get FuriThread instance for current thread
  * 
- * @return FuriThread* 
+ * @return pointer to FuriThread or NULL if this thread doesn't belongs to Furi
  */
 FuriThread* furi_thread_get_current();
 
@@ -288,12 +288,10 @@ uint32_t furi_thread_get_stack_space(FuriThreadId thread_id);
 FuriThreadStdoutWriteCallback furi_thread_get_stdout_callback();
 
 /** Set STDOUT callback for thread
- * 
+ *
  * @param      callback  callback or NULL to clear
- * 
- * @return     true on success, otherwise fail
  */
-bool furi_thread_set_stdout_callback(FuriThreadStdoutWriteCallback callback);
+void furi_thread_set_stdout_callback(FuriThreadStdoutWriteCallback callback);
 
 /** Write data to buffered STDOUT
  * 

furi/core/timer.c

@@ -124,3 +124,13 @@ uint32_t furi_timer_is_running(FuriTimer* instance) {
     /* Return 0: not running, 1: running */
     return (uint32_t)xTimerIsTimerActive(hTimer);
 }
+
+void furi_timer_pending_callback(FuriTimerPendigCallback callback, void* context, uint32_t arg) {
+    BaseType_t ret = pdFAIL;
+    if(furi_kernel_is_irq_or_masked()) {
+        ret = xTimerPendFunctionCallFromISR(callback, context, arg, NULL);
+    } else {
+        ret = xTimerPendFunctionCall(callback, context, arg, FuriWaitForever);
+    }
+    furi_check(ret == pdPASS);
+}

furi/core/timer.h

@@ -56,6 +56,10 @@ FuriStatus furi_timer_stop(FuriTimer* instance);
  */
 uint32_t furi_timer_is_running(FuriTimer* instance);
 
+typedef void (*FuriTimerPendigCallback)(void* context, uint32_t arg);
+
+void furi_timer_pending_callback(FuriTimerPendigCallback callback, void* context, uint32_t arg);
+
 #ifdef __cplusplus
 }
 #endif