QVAC-23767 abot: destroy ModelManager before the runners that own its tensors

src/abot_world.hpp

@@ -974,9 +974,16 @@ class AbotWalkSession {
     // return false to fall back to the RNG.
     std::function<bool(int block, int step, float* dst, size_t n)> noise_override;
 
-    std::shared_ptr<ModelManager> model_manager;
     std::unique_ptr<AbotWorldRunner> runner;
     std::shared_ptr<AbotTinyVideoAutoEncoder> tae;
+    // Declared after the runners on purpose (mirrors StableDiffusionGGML):
+    // ~ModelManager force-frees the param storage blocks and writes through
+    // the registered ggml tensors (state->tensor->buffer = nullptr), which
+    // live in the runner/tae contexts above. Members destroy in reverse
+    // declaration order and the runners hold only weak_ptr refs to the
+    // manager, so this ordering runs ~ModelManager first, while every
+    // registered tensor is still alive.
+    std::shared_ptr<ModelManager> model_manager;
 
     // finalized walk state (per-frame ggml {W,H,C} latents, torch [C,H,W] flat)
     std::vector<std::vector<float>> history;

src/core/ggml_extend.hpp

@@ -3194,6 +3194,90 @@ struct GGMLRunner {
         return ggml_get_tensor(cache_ctx, name.c_str());
     }
 
+    // Shared VAE-CPU-fallback reroute used by both entry points (the
+    // preflight route and the reactive retry after a failed
+    // execute_graph). prepare_params stages weights to each tensor state's
+    // registration-time compute backend, which does not follow a switched
+    // runtime: a CPU graph would read device-staged memory (SIGSEGV in
+    // ggml_vec_dot on a Vulkan-staged weight). So: collect the graph's
+    // params BEFORE switching (switch_runtime_backend frees the compute
+    // ctx that owns gf; the param pointers themselves are stable model
+    // tensors), quiesce this runner's prepared params, re-point the params
+    // at the fallback backend — with compute and params on the same
+    // backend staging is skipped and the CPU graph reads them in place —
+    // run the graph, then restore the runtime backend and the params on
+    // every exit path. A failed restore leaves the runner's weight
+    // bindings inconsistent with its runtime backend (the next graph would
+    // hand a host pointer to a GPU kernel), so it is fatal to the feature:
+    // disable the fallback and fail the call instead of returning output.
+    template <typename T>
+    std::optional<sd::Tensor<T>> compute_on_vae_fallback_backend(
+        ggml_cgraph* gf,
+        get_graph_cb_t get_graph,
+        int n_threads,
+        bool free_compute_buffer,
+        bool free_compute_params,
+        bool no_return) {
+        std::vector<ggml_tensor*> fallback_params = collect_used_param_tensors(gf);
+        ggml_backend_t previous_backend           = runtime_backend;
+        const std::string previous_backend_name   = ggml_backend_name(previous_backend);
+        const std::string cpu_backend_name        = ggml_backend_name(vae_fallback_backend);
+        auto repoint_params = [&](ggml_backend_t target) -> bool {
+            runner_done();
+            auto manager = weight_manager.lock();
+            if (manager == nullptr || fallback_params.empty()) {
+                return true;
+            }
+            return manager->assign_compute_backend(fallback_params, target);
+        };
+        switch_runtime_backend(vae_fallback_backend);
+        if (!repoint_params(vae_fallback_backend)) {
+            // assign_compute_backend validates before it commits, so a
+            // refusal here means nothing moved; switching back restores
+            // the exact pre-call state.
+            LOG_ERROR("%s VAE CPU fallback failed to re-point graph params to %s",
+                      get_desc().c_str(),
+                      cpu_backend_name.c_str());
+            switch_runtime_backend(previous_backend);
+            free_compute_ctx();
+            return std::nullopt;
+        }
+        auto restore = [&]() -> bool {
+            switch_runtime_backend(previous_backend);
+            return repoint_params(previous_backend);
+        };
+        auto on_failed_restore = [&]() {
+            LOG_ERROR(
+                "%s VAE CPU fallback could not restore graph params to %s; "
+                "weight bindings no longer match the runtime backend — "
+                "disabling VAE auto CPU fallback and failing this call",
+                get_desc().c_str(),
+                previous_backend_name.c_str());
+            vae_auto_cpu_fallback_enabled = false;
+        };
+        try {
+            auto output = compute<T>(get_graph,
+                                     n_threads,
+                                     false,
+                                     free_compute_buffer,
+                                     free_compute_params,
+                                     no_return);
+            if (!restore()) {
+                on_failed_restore();
+                return std::nullopt;
+            }
+            LOG_INFO("%s VAE CPU fallback complete; restored runtime backend %s",
+                     get_desc().c_str(),
+                     previous_backend_name.c_str());
+            return output;
+        } catch (...) {
+            if (!restore()) {
+                on_failed_restore();
+            }
+            throw;
+        }
+    }
+
     template <typename T>
     std::optional<sd::Tensor<T>> compute(get_graph_cb_t get_graph,
                                          int n_threads,
@@ -3242,27 +3326,18 @@ struct GGMLRunner {
                 has_stateful_cache) {
                 return std::nullopt;
             }
-            ggml_backend_t previous_backend = runtime_backend;
-            const std::string previous_backend_name =
-                ggml_backend_name(previous_backend);
             LOG_WARN("%s VAE %s on %s; retrying stateless graph on CPU",
                      get_desc().c_str(),
                      failure,
-                     previous_backend_name.c_str());
-            switch_runtime_backend(vae_fallback_backend);
-            try {
-                auto output = compute<T>(get_graph,
-                                         n_threads,
-                                         false,
-                                         free_compute_buffer,
-                                         free_compute_params,
-                                         no_return);
-                switch_runtime_backend(previous_backend);
-                return output;
-            } catch (...) {
-                switch_runtime_backend(previous_backend);
-                throw;
-            }
+                     ggml_backend_name(runtime_backend));
+            // gf is still owned by the (unfreed) compute ctx here; the
+            // shared reroute collects its params before switching.
+            return compute_on_vae_fallback_backend<T>(gf,
+                                                      get_graph,
+                                                      n_threads,
+                                                      free_compute_buffer,
+                                                      free_compute_params,
+                                                      no_return);
         };
 
         if (vae_auto_cpu_fallback_enabled &&
@@ -3295,8 +3370,7 @@ struct GGMLRunner {
                 capacity.free_memory_ratio);
 
             if (decision.use_cpu_fallback()) {
-                ggml_backend_t previous_backend         = runtime_backend;
-                const std::string previous_backend_name = ggml_backend_name(previous_backend);
+                const std::string previous_backend_name = ggml_backend_name(runtime_backend);
                 const std::string cpu_backend_name      = ggml_backend_name(vae_fallback_backend);
 
                 LOG_WARN(
@@ -3315,23 +3389,12 @@ struct GGMLRunner {
                     previous_backend_name.c_str(),
                     cpu_backend_name.c_str());
 
-                switch_runtime_backend(vae_fallback_backend);
-                try {
-                    auto output = compute<T>(get_graph,
-                                             n_threads,
-                                             false,
-                                             free_compute_buffer,
-                                             free_compute_params,
-                                             no_return);
-                    switch_runtime_backend(previous_backend);
-                    LOG_INFO("%s VAE CPU fallback complete; restored runtime backend %s",
-                             get_desc().c_str(),
-                             previous_backend_name.c_str());
-                    return output;
-                } catch (...) {
-                    switch_runtime_backend(previous_backend);
-                    throw;
-                }
+                return compute_on_vae_fallback_backend<T>(gf,
+                                                          get_graph,
+                                                          n_threads,
+                                                          free_compute_buffer,
+                                                          free_compute_params,
+                                                          no_return);
             }
 
             if (decision.reason == sd::VaeGraphRouteReason::STATEFUL_GRAPH) {

src/model_manager.cpp

@@ -1142,30 +1142,51 @@ bool ModelManager::assign_compute_backend(const std::vector<ggml_tensor*>& tenso
         return false;
     }
 
-    for (TensorState* state : required_states) {
-        if (state == nullptr || state->tensor == nullptr) {
-            continue;
-        }
-
+    // Two-phase: validate every state first, then commit. A mid-loop
+    // refusal must not leave a prefix of the tensors re-pointed at the new
+    // backend — a partially-moved set silently skips staging on the next
+    // graph (compute == params backend) and hands host pointers to device
+    // kernels, and the caller has no way to roll it back.
+    auto needs_move = [&](const TensorState* state, bool* params_follow_out) {
         const bool params_follow_compute = state->params_follow_compute_backend ||
                                            state->residency_mode == ResidencyMode::Disk;
+        if (params_follow_out != nullptr) {
+            *params_follow_out = params_follow_compute;
+        }
         const bool compute_changes = state->compute_backend != compute_backend;
         const bool params_changes  = params_follow_compute && state->params_backend != compute_backend;
-        if (!compute_changes && !params_changes) {
+        return compute_changes || params_changes;
+    };
+
+    for (const TensorState* state : required_states) {
+        if (state == nullptr || state->tensor == nullptr) {
+            continue;
+        }
+        bool params_follow_compute = false;
+        if (!needs_move(state, &params_follow_compute)) {
             continue;
         }
-
         if (state->active_prepare_count > 0 || state->staged_to_compute_backend) {
             LOG_ERROR("model manager cannot move active tensor '%s' to another compute backend",
                       state->name.c_str());
             return false;
         }
+        const bool params_changes = params_follow_compute && state->params_backend != compute_backend;
         if (params_changes && state->loaded_to_params_backend) {
             LOG_ERROR("model manager cannot move loaded tensor '%s' to another params backend",
                       state->name.c_str());
             return false;
         }
+    }
 
+    for (TensorState* state : required_states) {
+        if (state == nullptr || state->tensor == nullptr) {
+            continue;
+        }
+        bool params_follow_compute = false;
+        if (!needs_move(state, &params_follow_compute)) {
+            continue;
+        }
         state->compute_backend = compute_backend;
         if (params_follow_compute) {
             state->params_backend = compute_backend;

src/stable-diffusion.cpp

@@ -7517,16 +7517,23 @@ bool sd_abot_scene_create(const sd_abot_scene_params_t* p) {
             LOG_ERROR("sd_abot_scene_create: backend init failed: %s", error.c_str()); return false;
         }
         const int threads = p->n_threads > 0 ? p->n_threads : sd_get_num_physical_cores();
+        // The runners below own the ggml tensors that ~ModelManager's
+        // storage-block teardown writes through (state->tensor->buffer =
+        // nullptr), and they hold only weak_ptr refs to the manager. Declare
+        // them BEFORE the manager so reverse local destruction runs
+        // ~ModelManager first, while every registered tensor is still alive
+        // (mirrors the member ordering in StableDiffusionGGML).
+        std::shared_ptr<AbotT5Runner> t5;
+        std::shared_ptr<AbotWanVAE> vae;
         auto model_manager = std::make_shared<ModelManager>();
         model_manager->set_n_threads(threads);
         ModelLoader& model_loader = model_manager->loader();
         if (!model_loader.init_from_file(p->t5_path, "text_encoders.t5xxl.transformer.")) return false;
-        auto t5 = std::make_shared<AbotT5Runner>(backends.runtime_backend(SDBackendModule::TE),
-                                                   model_loader.get_tensor_storage_map(),
-                                                   "text_encoders.t5xxl.transformer",
-                                                   true,
-                                                   model_manager);
-        std::shared_ptr<AbotWanVAE> vae;
+        t5 = std::make_shared<AbotT5Runner>(backends.runtime_backend(SDBackendModule::TE),
+                                              model_loader.get_tensor_storage_map(),
+                                              "text_encoders.t5xxl.transformer",
+                                              true,
+                                              model_manager);
         if (has_image) {
             if (!model_loader.init_from_file(p->vae_path, "first_stage_model.")) return false;
             vae = std::make_shared<AbotWanVAE>(backends.runtime_backend(SDBackendModule::VAE),