Remove explicit app closing function (#618)

Modules/app-module/include/app/loader.h

@@ -23,7 +23,8 @@ extern "C" {
  * firmware binary. Called on the dedicated task app-module's scheduler spawns for this instance,
  * blocking for the app's whole lifetime - same contract as an external app's main(), plus
  * @a app_instance_id identifying this running instance (use it with
- * app_event_subscribe()/window_manager_create()/app_manager_finish()/etc.).
+ * app_event_subscribe()/window_manager_create()/etc.). The instance closes when this function
+ * returns - no separate call is needed.
  * AppManifest::location.location holds this cast to void*.
  */
 typedef int32_t (*AppMainFn)(uint32_t app_instance_id, int argc, char* argv[]);

Modules/app-module/include/app/manager.h

@@ -90,23 +90,11 @@ error_t app_manager_start_for_result(const char* id, AppInstanceId parent_instan
  * Stop an app instance permanently. Emits APP_EVENT_CLOSE and bound-waits for its task to exit
  * if it was running.
  * @warning Must not be called from the instance's own task (it bound-waits via thread_join(),
- * which asserts against joining yourself) - an app closing itself must call app_manager_finish()
- * instead, right before returning from its own AppMainFn/AppLoaderApi::run().
+ * which asserts against joining yourself) - an app closes itself by returning from its own
+ * AppMainFn/AppLoaderApi::run(), not by calling this on itself.
  */
 error_t app_manager_stop(AppInstanceId app_instance_id);
 
-/**
- * Called by an app instance, from its own task, right before it returns in response to
- * APP_EVENT_CLOSE - whether that close was self-initiated (e.g. its own back button) or came
- * from someone else. Marks this instance Stopped immediately (rather than waiting for its task
- * to actually exit) so app_manager_get_state()/app_manager_get_topmost_instance_id() reflect the
- * closure as soon as the app has decided to close, not just once its task has fully unwound.
- * @warning Does not join or free this instance's own task/ledger entry (can't - this runs on
- * that very task); those are cleaned up on a later app_manager_stop() call, same as any
- * self-terminating instance.
- */
-error_t app_manager_finish(AppInstanceId app_instance_id);
-
 /** @return the instance's current state, or APP_INSTANCE_STATE_STOPPED if the id is unknown. */
 AppInstanceState app_manager_get_state(AppInstanceId app_instance_id);
 

Modules/app-module/source/app_scheduler.cpp

@@ -156,9 +156,9 @@ void app_task_main(void* context) {
 
     deliver_result_to_parent_if_any(ctx->app_instance_id, result);
 
-    // A safe default terminal marker for CLOSE (and any other exit): an app that calls
-    // app_manager_finish() already marked itself Stopped before returning, so this is a no-op
-    // for it - but it's still needed as the terminal marker for any other exit path.
+    // The terminal marker for every exit path: an app instance is Stopped exactly when its
+    // AppMainFn/AppLoaderApi::run() has returned, whether that return was self-initiated or in
+    // response to APP_EVENT_CLOSE.
     set_state(ctx->app_instance_id, APP_INSTANCE_STATE_STOPPED);
 
     app_ledger_free_arguments(ctx->argc, ctx->argv);
@@ -269,6 +269,11 @@ error_t app_scheduler_stop(AppInstanceId app_instance_id, TickType_t join_timeou
         AppEvent event { .type = APP_EVENT_CLOSE, .timestamp = 0, .result = {} };
         app_event_emit(app_instance_id, &event);
 
+        // Marked as soon as the app has been told to close, not once its task has actually
+        // unwound - so app_manager_get_state()/app_manager_get_topmost_instance_id() reflect the
+        // closure immediately, without waiting on whatever teardown the app still has left to do.
+        set_state(app_instance_id, APP_INSTANCE_STATE_STOPPING);
+
         // Blocks until app_task_main() gives this dedicated semaphore as the literal last thing it does before vTaskDelete().
         // Uses aa dedicated semaphore rather than this task's default FreeRTOS notification because app_event.cpp's AppEventSubscription also uses that shared slot.
         // An unrelated event (e.g. a different child's APP_EVENT_RESULT) delivered to this same task could otherwise unblock this early.

Modules/app-module/source/manager.cpp

@@ -138,17 +138,6 @@ error_t app_manager_stop(AppInstanceId app_instance_id) {
     return app_scheduler_stop(app_instance_id, pdMS_TO_TICKS(2000));
 }
 
-error_t app_manager_finish(AppInstanceId app_instance_id) {
-    auto& ledger = app_ledger();
-    mutex_lock(&ledger.mutex);
-    auto iterator = ledger.instances.find(app_instance_id);
-    if (iterator != ledger.instances.end()) {
-        iterator->second.state = APP_INSTANCE_STATE_STOPPED;
-    }
-    mutex_unlock(&ledger.mutex);
-    return ERROR_NONE;
-}
-
 AppInstanceState app_manager_get_state(AppInstanceId app_instance_id) {
     auto& ledger = app_ledger();
     mutex_lock(&ledger.mutex);

Modules/app-module/source/symbols.cpp

@@ -30,7 +30,6 @@ const ModuleSymbol app_module_symbols[] = {
     DEFINE_MODULE_SYMBOL(app_manager_start_with_parameters),
     DEFINE_MODULE_SYMBOL(app_manager_start_for_result),
     DEFINE_MODULE_SYMBOL(app_manager_stop),
-    DEFINE_MODULE_SYMBOL(app_manager_finish),
     DEFINE_MODULE_SYMBOL(app_manager_get_state),
     DEFINE_MODULE_SYMBOL(app_manager_find_manifest),
     DEFINE_MODULE_SYMBOL(app_manager_for_each_manifest),

Modules/app-module/tests/source/app_manager_test.cpp

@@ -76,7 +76,6 @@ int32_t fake_run(void*, uint32_t app_instance_id, int argc, char* argv[]) {
             break; // safety net so a bug here can't hang the test suite
         }
         if (event.type == APP_EVENT_CLOSE) {
-            app_manager_finish(app_instance_id);
             break;
         }
     }