add common/subproc.h|cpp

common/CMakeLists.txt

@@ -100,6 +100,8 @@ add_library(${TARGET}
     sampling.h
     speculative.cpp
     speculative.h
+    subproc.cpp
+    subproc.h
     unicode.cpp
     unicode.h
     jinja/lexer.cpp

common/subproc.cpp

@@ -0,0 +1,80 @@
+#include "subproc.h"
+
+static std::vector<char *> to_cstr_vec(const std::vector<std::string> & v) {
+    std::vector<char *> r;
+    r.reserve(v.size() + 1);
+    for (const auto & s : v) {
+        r.push_back(const_cast<char *>(s.c_str()));
+    }
+    r.push_back(nullptr);
+    return r;
+}
+
+common_subproc::~common_subproc() {
+    if (is_created) {
+        subprocess_destroy(&proc);
+        is_created = false;
+    }
+}
+
+bool common_subproc::create(
+        const std::vector<std::string> & args,
+        int options,
+        const std::vector<std::string> & env,
+        const char * cwd) {
+    auto argv = to_cstr_vec(args);
+
+    int result;
+    if (env.empty() && cwd == nullptr) {
+        result = subprocess_create(argv.data(), options, &proc);
+    } else {
+        auto envp = to_cstr_vec(env);
+        result = subprocess_create_ex(argv.data(), options, env.empty() ? nullptr : envp.data(), cwd, &proc);
+    }
+
+    is_created = result == 0;
+    return is_created;
+}
+
+bool common_subproc::has_handle() const {
+    if (!is_created) {
+        return false;
+    }
+#if defined(_WIN32)
+    return proc.hProcess != nullptr;
+#else
+    return proc.child > 0;
+#endif
+}
+
+bool common_subproc::alive() {
+    return is_created && subprocess_alive(&proc);
+}
+
+FILE * common_subproc::stdin_file() {
+    return is_created ? subprocess_stdin(&proc) : nullptr;
+}
+
+FILE * common_subproc::stdout_file() {
+    return is_created ? subprocess_stdout(&proc) : nullptr;
+}
+
+FILE * common_subproc::stderr_file() {
+    return is_created ? subprocess_stderr(&proc) : nullptr;
+}
+
+void common_subproc::terminate() {
+    if (has_handle()) {
+        subprocess_terminate(&proc);
+    }
+}
+
+int common_subproc::join() {
+    int exit_code = -1;
+    if (is_created) {
+        subprocess_join(&proc, &exit_code);
+        subprocess_destroy(&proc);
+        is_created = false;
+    }
+    return exit_code;
+}

common/subproc.h

@@ -0,0 +1,42 @@
+#pragma once
+
+#include <sheredom/subprocess.h>
+
+#include <cstdio>
+#include <string>
+#include <vector>
+
+// RAII-style wrapper around https://github.com/sheredom/subprocess.h,
+// exposing method calls instead of free functions operating on subprocess_s.
+struct common_subproc {
+    common_subproc() = default;
+    ~common_subproc();
+
+    common_subproc(const common_subproc &) = delete;
+    common_subproc & operator=(const common_subproc &) = delete;
+
+    // spawn a child process; if env is non-empty it replaces the child's environment
+    // (do not combine with subprocess_option_inherit_environment)
+    bool create(
+            const std::vector<std::string> & args,
+            int options,
+            const std::vector<std::string> & env = {},
+            const char * cwd = nullptr);
+
+    bool alive();
+
+    FILE * stdin_file();
+    FILE * stdout_file();
+    FILE * stderr_file();
+
+    void terminate();
+
+    // wait for the process to exit, release the underlying handle and return its exit code
+    int join();
+
+private:
+    subprocess_s proc {};
+    bool is_created = false;
+
+    bool has_handle() const;
+};

tools/server/server-models.cpp

@@ -8,10 +8,10 @@
 #include "preset.h"
 #include "download.h"
 #include "http.h"
+#include "subproc.h"
 
 #include <cpp-httplib/httplib.h> // TODO: remove this once we use HTTP client from download.h
 #include <optional>
-#include <sheredom/subprocess.h>
 
 #include <functional>
 #include <optional>
@@ -49,43 +49,24 @@ extern char **environ;
 #define CHILD_ADDR "127.0.0.1"
 
 struct server_subproc {
-    std::optional<subprocess_s> sproc; // empty while in DOWNLOADING state
+    common_subproc sproc; // not yet spawned while in DOWNLOADING state
     std::atomic<bool> stopped{false}; // set to cancel a download or signal child process exit
 
-    subprocess_s & get() {
-        GGML_ASSERT(sproc.has_value() && "subprocess not initialized");
-        return sproc.value();
-    }
-
     bool is_alive() {
-        return sproc.has_value() && subprocess_alive(&sproc.value());
+        return sproc.alive();
     }
 
     void request_exit() {
-        if (sproc.has_value()) {
-            FILE * stdin_file = subprocess_stdin(&sproc.value());
-            if (stdin_file) {
-                fprintf(stdin_file, "%s\n", CMD_ROUTER_TO_CHILD_EXIT);
-                fflush(stdin_file);
-            }
+        FILE * stdin_file = sproc.stdin_file();
+        if (stdin_file) {
+            fprintf(stdin_file, "%s\n", CMD_ROUTER_TO_CHILD_EXIT);
+            fflush(stdin_file);
         }
         stopped.store(true, std::memory_order_relaxed);
     }
 
     void terminate() {
-        if (!sproc.has_value()) {
-            return;
-        }
-#if defined(_WIN32)
-        if (sproc->hProcess == NULL) {
-            return;
-        }
-#else
-        if (sproc->child <= 0) {
-            return;
-        }
-#endif
-        subprocess_terminate(&sproc.value());
+        sproc.terminate();
     }
 };
 
@@ -711,18 +692,6 @@ std::optional<server_model_meta> server_models::get_meta(const std::string & nam
     return std::nullopt;
 }
 
-// helper to convert vector<string> to char **
-// pointers are only valid as long as the original vector is valid
-static std::vector<char *> to_char_ptr_array(const std::vector<std::string> & vec) {
-    std::vector<char *> result;
-    result.reserve(vec.size() + 1);
-    for (const auto & s : vec) {
-        result.push_back(const_cast<char*>(s.c_str()));
-    }
-    result.push_back(nullptr);
-    return result;
-}
-
 std::vector<server_model_meta> server_models::get_all_meta() {
     std::unique_lock<std::mutex> lk(mutex);
     if (need_reload) {
@@ -845,15 +814,10 @@ void server_models::load(const std::string & name, const load_options & opts) {
         }
         inst.meta.args = child_args; // save for debugging
 
-        std::vector<char *> argv = to_char_ptr_array(child_args);
-        std::vector<char *> envp = to_char_ptr_array(child_env);
-
         // TODO @ngxson : maybe separate stdout and stderr in the future
         //                so that we can use stdout for commands and stderr for logging
         int options = subprocess_option_no_window | subprocess_option_combined_stdout_stderr;
-        inst.subproc->sproc.emplace();
-        int result = subprocess_create_ex(argv.data(), options, envp.data(), nullptr, &inst.subproc->get());
-        if (result != 0) {
+        if (!inst.subproc->sproc.create(child_args, options, child_env)) {
             throw std::runtime_error("failed to spawn server instance");
         }
     }
@@ -867,8 +831,8 @@ void server_models::load(const std::string & name, const load_options & opts) {
         stop_timeout = inst.meta.stop_timeout,
         child_mode = opts.mode
     ]() {
-        FILE * stdin_file = subprocess_stdin(&child_proc->get());
-        FILE * stdout_file = subprocess_stdout(&child_proc->get()); // combined stdout/stderr
+        FILE * stdin_file = child_proc->sproc.stdin_file();
+        FILE * stdout_file = child_proc->sproc.stdout_file(); // combined stdout/stderr
 
         std::thread log_thread([&]() {
             // read stdout/stderr and forward to main server log
@@ -942,9 +906,7 @@ void server_models::load(const std::string & name, const load_options & opts) {
         }
 
         // get the exit code
-        int exit_code = 0;
-        subprocess_join(&child_proc->get(), &exit_code);
-        subprocess_destroy(&child_proc->get());
+        int exit_code = child_proc->sproc.join();
 
         // update status and exit code
         if (child_mode == SERVER_CHILD_MODE_DOWNLOAD) {

tools/server/server-tools.cpp

@@ -1,6 +1,6 @@
 #include "server-tools.h"
 
-#include <sheredom/subprocess.h>
+#include "subproc.h"
 
 #include <filesystem>
 #include <fstream>
@@ -137,15 +137,14 @@ public:
             const std::function<bool(const std::string &)> & on_chunk = nullptr) const override {
         exec_result res;
 
-        subprocess_s proc;
-        auto argv = to_cstr_vec(args);
+        common_subproc proc;
 
         int options = subprocess_option_no_window
                     | subprocess_option_combined_stdout_stderr
                     | subprocess_option_inherit_environment
                     | subprocess_option_search_user_path;
 
-        if (subprocess_create(argv.data(), options, &proc) != 0) {
+        if (!proc.create(args, options)) {
             res.output = "failed to spawn process";
             return res;
         }
@@ -158,14 +157,14 @@ public:
             while (!done.load()) {
                 if (std::chrono::steady_clock::now() >= deadline) {
                     timed_out.store(true);
-                    subprocess_terminate(&proc);
+                    proc.terminate();
                     return;
                 }
                 std::this_thread::sleep_for(std::chrono::milliseconds(100));
             }
         });
 
-        FILE * f = subprocess_stdout(&proc);
+        FILE * f = proc.stdout_file();
         std::string output;
         bool truncated = false;
         if (f) {
@@ -176,7 +175,7 @@ public:
                     if (output.size() + len <= max_output) {
                         output.append(buf, len);
                         if (on_chunk && !on_chunk(std::string(buf, len))) {
-                            subprocess_terminate(&proc);
+                            proc.terminate();
                             break;
                         }
                     } else {
@@ -194,8 +193,7 @@ public:
             timeout_thread.join();
         }
 
-        subprocess_join(&proc, &res.exit_code);
-        subprocess_destroy(&proc);
+        res.exit_code = proc.join();
 
         res.output    = output;
         res.timed_out = timed_out.load();
@@ -206,16 +204,6 @@ public:
     }
 
 private:
-    static std::vector<char *> to_cstr_vec(const std::vector<std::string> & v) {
-        std::vector<char *> r;
-        r.reserve(v.size() + 1);
-        for (const auto & s : v) {
-            r.push_back(const_cast<char *>(s.c_str()));
-        }
-        r.push_back(nullptr);
-        return r;
-    }
-
     static const std::unordered_set<std::string> & junk_dir_names() {
         static const std::unordered_set<std::string> names = {
             ".git", ".svn", ".hg", "node_modules", "__pycache__",