Fixes

Modules/app-module/private/app/private/app_fs.h

@@ -46,39 +46,76 @@ inline bool app_fs_delete_recursively(const std::string& path) {
         return true;
     }
 
-    if (app_fs_is_directory(path)) {
+    // Use lstat() so symbolic links are not followed: a symlink that points at
-        FileMutex file_mutex;
+    // an external directory must be removed as a leaf entry (unlink), not
-        file_mutex_get(&file_mutex, path.c_str());
+    // recursed into.  app_fs_is_directory() uses stat() and would follow the
-        file_mutex_lock(&file_mutex);
+    // link, potentially deleting files outside the target tree.
+    // ESP-IDF newlib has no lstat(); ESP32 filesystems (FAT/SPIFFS) don't
+    // support symlinks, so stat() is equivalent there.
+    struct stat st {};
+    FileMutex file_mutex;
+    file_mutex_get(&file_mutex, path.c_str());
+    file_mutex_lock(&file_mutex);
+#ifdef ESP_PLATFORM
+    int rc = stat(path.c_str(), &st);
+#else
+    int rc = lstat(path.c_str(), &st);
+#endif
+    file_mutex_unlock(&file_mutex);
 
+    if (rc != 0) {
+        return false;
+    }
+
+#ifndef ESP_PLATFORM
+    if (S_ISLNK(st.st_mode)) {
+        // Symlink — remove as a leaf regardless of its target.
+        file_mutex_lock(&file_mutex);
+        bool result = unlink(path.c_str()) == 0;
+        file_mutex_unlock(&file_mutex);
+        return result;
+    }
+#endif
+
+    if (S_ISDIR(st.st_mode)) {
+        // Collect child names while locked, then release before recursing —
+        // child paths can resolve to the same mount mutex (see
+        // app_fs_list_direct_subdirectories comment), so holding the parent
+        // lock across the recursive call would self-deadlock.
+        std::vector<std::string> children;
+
+        file_mutex_lock(&file_mutex);
         DIR* dir = opendir(path.c_str());
         if (dir == nullptr) {
             file_mutex_unlock(&file_mutex);
             return false;
         }
 
-        bool success = true;
         struct dirent* entry;
-        while (success && (entry = readdir(dir)) != nullptr) {
+        while ((entry = readdir(dir)) != nullptr) {
             if (std::strcmp(entry->d_name, ".") == 0 || std::strcmp(entry->d_name, "..") == 0) {
                 continue;
             }
-            success = app_fs_delete_recursively(path + "/" + entry->d_name);
+            children.push_back(path + "/" + entry->d_name);
         }
         closedir(dir);
+        file_mutex_unlock(&file_mutex);
 
-        if (!success) {
+        bool success = true;
-            file_mutex_unlock(&file_mutex);
+        for (const auto& child : children) {
-            return false;
+            success = app_fs_delete_recursively(child);
+            if (!success) {
+                return false;
+            }
         }
 
+        file_mutex_lock(&file_mutex);
         bool result = rmdir(path.c_str()) == 0;
         file_mutex_unlock(&file_mutex);
         return result;
     }
 
-    FileMutex file_mutex;
+    // Regular file or other — unlink.
-    file_mutex_get(&file_mutex, path.c_str());
     file_mutex_lock(&file_mutex);
     bool result = unlink(path.c_str()) == 0;
     file_mutex_unlock(&file_mutex);

Modules/app-module/source/manager.cpp

@@ -1,13 +1,12 @@
 // SPDX-License-Identifier: Apache-2.0
 #include <app/manager.h>
-
 #include <app/metadata.h>
-
 #include <app/private/app_fs.h>
 #include <app/private/app_ledger.h>
 #include <app/private/app_scheduler.h>
 
 #include <tactility/concurrent/mutex.h>
+#include <tactility/error.h>
 #include <tactility/log.h>
 
 #include <algorithm>
@@ -350,19 +349,44 @@ error_t app_manager_install_path_uninstall(const char* app_id) {
         return ERROR_NOT_FOUND;
     }
 
+    const AppManifest* manifest = &iterator->second->manifest;
     auto path = iterator->second->path;
     mutex_unlock(&registry.mutex);
 
-    // app_manager_remove takes ledger.mutex internally - call outside registry.mutex
+    // Stop every running instance that retains this manifest pointer, mirroring
-    // to match the lock ordering in app_manager_install_path_scan().
+    // stop_all_instances_of() in app_install.cpp.  Collect under ledger.mutex,
+    // then call app_manager_stop() outside it (that call bound-joins the
+    // instance's thread, which itself takes ledger.mutex in its thread_main).
+    std::vector<uint32_t> instance_ids;
+    auto& ledger = app_ledger();
+    mutex_lock(&ledger.mutex);
+    for (const auto& [id, record] : ledger.instances) {
+        if (record.manifest == manifest) {
+            instance_ids.push_back(id);
+        }
+    }
+    mutex_unlock(&ledger.mutex);
+
+    for (uint32_t id : instance_ids) {
+        app_manager_stop(id);
+    }
+
+    // app_manager_remove takes ledger.mutex internally - call outside both
+    // registry.mutex and ledger.mutex to match the lock ordering in
+    // app_manager_install_path_scan().
     app_manager_remove(app_id);
 
+    // Every instance has stopped and the manifest is unregistered — safe to
+    // delete the on-disk directory.  Delete before erasing the scan record so
+    // that a failed deletion leaves the entry discoverable for a retry.
+    if (!app_fs_delete_recursively(path)) {
+        return ERROR_RESOURCE;
+    }
+
     mutex_lock(&registry.mutex);
     registry.scanned.erase(app_id);
     mutex_unlock(&registry.mutex);
 
-    app_fs_delete_recursively(path);
-
     return ERROR_NONE;
 }
 

Modules/service-module/tests/source/service_paths_test.cpp

@@ -9,7 +9,7 @@
 
 TEST_CASE("paths_get_data_path returns a non-empty path") {
     char buffer[192];
-    CHECK_EQ(paths_get_data_path(buffer, sizeof(buffer)), ERROR_NONE);
+    REQUIRE_EQ(paths_get_data_path(buffer, sizeof(buffer)), ERROR_NONE);
     CHECK_GT(std::strlen(buffer), 0);
 }