From 6610724f8e4543a217389f27abf8feacc48ad02a Mon Sep 17 00:00:00 2001
From: Xuan Son Nguyen <son@huggingface.co>
Date: Thu, 20 Nov 2025 16:13:30 +0100
Subject: [PATCH] fix unsafe pointer

---
 tools/server/server-models.cpp | 25 ++++++++++++-------------
 tools/server/server-models.h   |  4 ++--
 tools/server/server.cpp        |  4 +++-
 3 files changed, 17 insertions(+), 16 deletions(-)

diff --git a/tools/server/server-models.cpp b/tools/server/server-models.cpp
index 776bd4c26d..9c03d5ecb7 100644
--- a/tools/server/server-models.cpp
+++ b/tools/server/server-models.cpp
@@ -91,9 +91,8 @@ server_models::server_models(
             /* status      */ SERVER_MODEL_STATUS_UNLOADED
         };
         mapping[meta.name] = instance_t{
-            /* subproc */ subprocess_s(),
+            /* subproc */ std::make_shared<subprocess_s>(),
             /* th      */ std::thread(),
-            /* th_log  */ std::thread(),
             /* meta    */ meta
         };
     }
@@ -220,7 +219,7 @@ void server_models::load(const std::string & name) {
     inst.meta.port   = get_free_port();
     inst.meta.status = SERVER_MODEL_STATUS_LOADING;
 
-    subprocess_s child_proc;
+    inst.subproc = std::make_shared<subprocess_s>();
     {
         std::string exec_path = get_server_exec_path().string();
         SRV_INF("spawning server instance with name=%s on port %d\n", inst.meta.name.c_str(), inst.meta.port);
@@ -255,19 +254,17 @@ void server_models::load(const std::string & name) {
         std::vector<char *> envp = to_char_ptr_array(child_env);
 
         int options = subprocess_option_no_window | subprocess_option_combined_stdout_stderr;
-        int result = subprocess_create_ex(argv.data(), options, envp.data(), &child_proc);
+        int result = subprocess_create_ex(argv.data(), options, envp.data(), inst.subproc.get());
         if (result != 0) {
             throw std::runtime_error("failed to spawn server instance");
         }
     }
 
-    inst.subproc = std::move(child_proc);
-
     // start a thread to manage the child process
-    inst.th = std::thread([this, name, child_proc = &inst.subproc, port = inst.meta.port]() {
+    inst.th = std::thread([this, name, child_proc = inst.subproc, port = inst.meta.port]() {
         // read stdout/stderr and forward to main server log
         {
-            FILE * p_stdout_stderr = subprocess_stdout(child_proc);
+            FILE * p_stdout_stderr = subprocess_stdout(child_proc.get());
             if (!p_stdout_stderr) {
                 return;
             }
@@ -278,7 +275,7 @@ void server_models::load(const std::string & name) {
         }
         // we reach here when the child process exits
         int exit_code = 0;
-        subprocess_join(child_proc, &exit_code);
+        subprocess_join(child_proc.get(), &exit_code);
         // update PID and status
         {
             std::lock_guard<std::mutex> lk(mutex);
@@ -308,7 +305,7 @@ void server_models::unload(const std::string & name) {
     if (it != mapping.end()) {
         if (it->second.meta.status == SERVER_MODEL_STATUS_LOADED) {
             SRV_INF("unloading model instance name=%s\n", name.c_str());
-            subprocess_destroy(&it->second.subproc);
+            subprocess_destroy(it->second.subproc.get());
             // status change will be handled by the managing thread
         } else {
             SRV_WRN("model instance name=%s is not loaded\n", name.c_str());
@@ -323,10 +320,11 @@ void server_models::unload_all() {
         for (auto & [name, inst] : mapping) {
             if (inst.meta.status == SERVER_MODEL_STATUS_LOADED) {
                 SRV_INF("unloading model instance name=%s\n", name.c_str());
-                subprocess_destroy(&inst.subproc);
+                subprocess_destroy(inst.subproc.get());
                 // status change will be handled by the managing thread
-                to_join.push_back(std::move(inst.th));
             }
+            // moving the thread to join list to avoid deadlock
+            to_join.push_back(std::move(inst.th));
         }
     }
     for (auto & th : to_join) {
@@ -361,9 +359,10 @@ void server_models::ensure_model_loaded(const std::string & name) {
     if (!meta.has_value()) {
         throw std::runtime_error("model name=" + name + " is not found");
     }
-    if (meta->status == SERVER_MODEL_STATUS_LOADED) {
+    if (meta->status == SERVER_MODEL_STATUS_LOADED || meta->status == SERVER_MODEL_STATUS_LOADING) {
         return; // already loaded
     }
+    SRV_INF("model name=%s is not loaded, loading...\n", name.c_str());
     load(name);
     wait_until_loaded(name);
 }
diff --git a/tools/server/server-models.h b/tools/server/server-models.h
index 00c4e88da8..f5080a84ca 100644
--- a/tools/server/server-models.h
+++ b/tools/server/server-models.h
@@ -9,6 +9,7 @@
 #include <mutex>
 #include <condition_variable>
 #include <functional>
+#include <memory>
 
 enum server_model_status {
     SERVER_MODEL_STATUS_UNLOADED,
@@ -53,9 +54,8 @@ struct server_model_meta {
 struct server_models {
 private:
     struct instance_t {
-        subprocess_s subproc;
+        std::shared_ptr<subprocess_s> subproc; // shared between main thread and monitoring thread
         std::thread th;
-        std::thread th_log; // logging thread
         server_model_meta meta;
     };
 
diff --git a/tools/server/server.cpp b/tools/server/server.cpp
index 2c8bb595b1..37ef9d96ea 100644
--- a/tools/server/server.cpp
+++ b/tools/server/server.cpp
@@ -5816,7 +5816,9 @@ int main(int argc, char ** argv, char ** envp) {
     if (is_router_server) {
         LOG_INF("%s: router server is listening on %s\n", __func__, ctx_http.listening_address.c_str());
         ctx_http.is_ready.store(true);
-        ctx_http.thread.join(); // keep the main thread alive
+        if (ctx_http.thread.joinable()) {
+            ctx_http.thread.join(); // keep the main thread alive
+        }
 
         // when the HTTP server stops, clean up and exit
         clean_up();