From d2a818231effb12b7b20b80b3b8c7756a9a33a04 Mon Sep 17 00:00:00 2001 From: Xuan-Son Nguyen Date: Sun, 26 Jul 2026 20:54:25 +0200 Subject: [PATCH] common: add `subproc.h` wrapper, disabled on android/ios (#26102) * add common/subproc.h|cpp * add compile flag LLAMA_SUBPROCESS * disabled by default on android and ios * test-jinja: use common subproc * mtmd: disable video if subproc is not set * disable subproc on wasm * make is_created atomic * migrate server-mcp --- CMakeLists.txt | 9 +++ common/CMakeLists.txt | 6 ++ common/subproc.cpp | 143 +++++++++++++++++++++++++++++++++ common/subproc.h | 59 ++++++++++++++ tests/test-jinja.cpp | 25 +++--- tools/mtmd/CMakeLists.txt | 9 ++- tools/server/server-mcp.cpp | 34 +++----- tools/server/server-models.cpp | 66 ++++----------- tools/server/server-tools.cpp | 30 +++---- 9 files changed, 272 insertions(+), 109 deletions(-) create mode 100644 common/subproc.cpp create mode 100644 common/subproc.h diff --git a/CMakeLists.txt b/CMakeLists.txt index 81f23d7e7..3df1d82db 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -84,6 +84,14 @@ else() set(LLAMA_TOOLS_INSTALL_DEFAULT ${LLAMA_STANDALONE}) endif() +# subprocess spawning isn't a supported/sandbox-friendly operation on mobile OSes or in WASM +if (CMAKE_SYSTEM_NAME STREQUAL "iOS" OR CMAKE_SYSTEM_NAME STREQUAL "Android" OR ANDROID + OR CMAKE_SYSTEM_NAME STREQUAL "Emscripten" OR EMSCRIPTEN) + set(LLAMA_SUBPROCESS_DEFAULT OFF) +else() + set(LLAMA_SUBPROCESS_DEFAULT ON) +endif() + # # option list # @@ -117,6 +125,7 @@ option(LLAMA_TESTS_INSTALL "llama: install tests" ON) # 3rd party libs option(LLAMA_OPENSSL "llama: use openssl to support HTTPS" ON) +option(LLAMA_SUBPROCESS "llama-common: use subprocess, required by server tools and server router mode" ${LLAMA_SUBPROCESS_DEFAULT}) option(LLAMA_LLGUIDANCE "llama-common: include LLGuidance library for structured output in common utils" OFF) diff --git a/common/CMakeLists.txt b/common/CMakeLists.txt index 99688f53b..799d22751 100644 --- a/common/CMakeLists.txt +++ b/common/CMakeLists.txt @@ -100,6 +100,8 @@ add_library(${TARGET} sampling.h speculative.cpp speculative.h + subproc.cpp + subproc.h trie.cpp trie.h unicode.cpp @@ -127,6 +129,10 @@ set_target_properties(${TARGET} PROPERTIES target_include_directories(${TARGET} PUBLIC . ../vendor) target_compile_features (${TARGET} PUBLIC cxx_std_17) +if (LLAMA_SUBPROCESS) + target_compile_definitions(${TARGET} PUBLIC LLAMA_SUBPROCESS) +endif() + if (BUILD_SHARED_LIBS) set_target_properties(${TARGET} PROPERTIES POSITION_INDEPENDENT_CODE ON) diff --git a/common/subproc.cpp b/common/subproc.cpp new file mode 100644 index 000000000..6d37f5900 --- /dev/null +++ b/common/subproc.cpp @@ -0,0 +1,143 @@ +#include "subproc.h" + +bool common_subproc::is_supported() { +#ifdef LLAMA_SUBPROCESS + return true; +#else + return false; +#endif +} + +#ifdef LLAMA_SUBPROCESS + +static std::vector to_cstr_vec(const std::vector & v) { + std::vector r; + r.reserve(v.size() + 1); + for (const auto & s : v) { + r.push_back(const_cast(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 & args, + int options, + const std::vector & 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::close_stdin() { + if (is_created && proc.stdin_file) { + fclose(proc.stdin_file); + proc.stdin_file = 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; +} + +#else // !LLAMA_SUBPROCESS + +common_subproc::~common_subproc() = default; + +bool common_subproc::create( + const std::vector &, + int, + const std::vector &, + const char *) { + (void)(proc); + (void)(is_created); + return false; +} + +bool common_subproc::has_handle() const { + return false; +} + +bool common_subproc::alive() { + return false; +} + +FILE * common_subproc::stdin_file() { + return nullptr; +} + +FILE * common_subproc::stdout_file() { + return nullptr; +} + +FILE * common_subproc::stderr_file() { + return nullptr; +} + +void common_subproc::close_stdin() { +} + +void common_subproc::terminate() { +} + +int common_subproc::join() { + return -1; +} + +#endif // LLAMA_SUBPROCESS diff --git a/common/subproc.h b/common/subproc.h new file mode 100644 index 000000000..89b69ee26 --- /dev/null +++ b/common/subproc.h @@ -0,0 +1,59 @@ +#pragma once + +#include +#include +#include +#include + +#ifdef LLAMA_SUBPROCESS +#include +#else +// dummy values to allow compilation when subprocess is disabled +struct subprocess_s {}; +static constexpr int subprocess_option_no_window = 0; +static constexpr int subprocess_option_combined_stdout_stderr = 0; +static constexpr int subprocess_option_inherit_environment = 0; +static constexpr int subprocess_option_search_user_path = 0; +#endif + +// 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 & args, + int options, + const std::vector & env = {}, + const char * cwd = nullptr); + + bool alive(); + + // true if LLAMA_SUBPROCESS was enabled at build time; when false, create() always fails + static bool is_supported(); + + FILE * stdin_file(); + FILE * stdout_file(); + FILE * stderr_file(); + + // close stdin and detach it from the process, so a later join()/destroy() won't double-close it; + // use this after writing all input to signal EOF to the child while it's still running + void close_stdin(); + + void terminate(); + + // wait for the process to exit, release the underlying handle and return its exit code + int join(); + +private: + subprocess_s proc {}; + std::atomic is_created{false}; + + bool has_handle() const; +}; diff --git a/tests/test-jinja.cpp b/tests/test-jinja.cpp index 90bdbc445..1ac5b57de 100644 --- a/tests/test-jinja.cpp +++ b/tests/test-jinja.cpp @@ -4,7 +4,7 @@ #include #include -#include +#include "subproc.h" #include "jinja/runtime.h" #include "jinja/parser.h" @@ -2135,21 +2135,20 @@ static void test_template_py(testing & t, const std::string & name, const std::s const char * python_executable = "python3"; #endif - const char * command_line[] = {python_executable, "-c", py_script.c_str(), NULL}; + std::vector args = {python_executable, "-c", py_script, }; - struct subprocess_s subprocess; + common_subproc subprocess; int options = subprocess_option_combined_stdout_stderr | subprocess_option_no_window | subprocess_option_inherit_environment | subprocess_option_search_user_path; - int result = subprocess_create(command_line, options, &subprocess); - if (result != 0) { - t.log("Failed to create subprocess, error code: " + std::to_string(result)); + if (!subprocess.create(args, options)) { + t.log("Failed to create subprocess"); t.assert_true("subprocess creation", false); return; } - FILE * p_stdin = subprocess_stdin(&subprocess); + FILE * p_stdin = subprocess.stdin_file(); // Write input std::string input = merged.dump(); @@ -2157,24 +2156,22 @@ static void test_template_py(testing & t, const std::string & name, const std::s if (written != input.size()) { t.log("Failed to write complete input to subprocess stdin"); t.assert_true("subprocess stdin write", false); - subprocess_destroy(&subprocess); + subprocess.close_stdin(); + subprocess.join(); return; } fflush(p_stdin); - fclose(p_stdin); // Close stdin to signal EOF to the Python process - subprocess.stdin_file = nullptr; + subprocess.close_stdin(); // Close stdin to signal EOF to the Python process // Read output std::string output; char buffer[1024]; - FILE * p_stdout = subprocess_stdout(&subprocess); + FILE * p_stdout = subprocess.stdout_file(); while (fgets(buffer, sizeof(buffer), p_stdout)) { output += buffer; } - int process_return; - subprocess_join(&subprocess, &process_return); - subprocess_destroy(&subprocess); + int process_return = subprocess.join(); if (process_return != 0) { t.log("Python script failed with exit code: " + std::to_string(process_return)); diff --git a/tools/mtmd/CMakeLists.txt b/tools/mtmd/CMakeLists.txt index ea684d9f1..d0329ca56 100644 --- a/tools/mtmd/CMakeLists.txt +++ b/tools/mtmd/CMakeLists.txt @@ -1,8 +1,15 @@ # mtmd -set(MTMD_VIDEO ON CACHE BOOL "enable video support in mtmd (requires ffmpeg binary in PATH)") +set(MTMD_VIDEO_HELP "enable video support in mtmd (requires ffmpeg binary in PATH)") + +set(MTMD_VIDEO ON CACHE BOOL "${MTMD_VIDEO_HELP}") # TODO: add MTMD_VIDEO_METHOD in the future to select between ffmpeg and other backends +if (MTMD_VIDEO AND NOT LLAMA_SUBPROCESS) + message(STATUS "Disabling MTMD_VIDEO because LLAMA_SUBPROCESS is OFF") + set(MTMD_VIDEO OFF CACHE BOOL "${MTMD_VIDEO_HELP}" FORCE) +endif() + find_package(Threads REQUIRED) add_library(mtmd diff --git a/tools/server/server-mcp.cpp b/tools/server/server-mcp.cpp index aa87afeb0..93db6164d 100644 --- a/tools/server/server-mcp.cpp +++ b/tools/server/server-mcp.cpp @@ -1,6 +1,6 @@ #include "server-mcp.h" -#include +#include "subproc.h" #include #include @@ -341,7 +341,7 @@ json server_mcp_transport::call_tool(const std::string & tool_name, // struct server_mcp_stdio::process_handle { - subprocess_s sp; + common_subproc sp; FILE * in = nullptr; // child stdin FILE * out = nullptr; // child stdout FILE * err = nullptr; // child stderr @@ -483,30 +483,15 @@ bool server_mcp_stdio::start() { envp_s = mcp_build_env(config.env); } - auto to_ptrs = [](std::vector & v) { - std::vector p; - p.reserve(v.size() + 1); - for (auto & s : v) { - p.push_back(s.c_str()); - } - p.push_back(nullptr); - return p; - }; - auto argv = to_ptrs(argv_s); - auto envp = to_ptrs(envp_s); - auto handle = std::make_unique(); - int rc = subprocess_create_ex(argv.data(), options, - config.env.empty() ? nullptr : envp.data(), - config.cwd.empty() ? nullptr : config.cwd.c_str(), - &handle->sp); - if (rc != 0) { + bool ok = handle->sp.create(argv_s, options, envp_s, config.cwd.empty() ? nullptr : config.cwd.c_str()); + if (!ok) { SRV_WRN("MCP '%s': failed to spawn '%s'\n", config.name.c_str(), config.command.c_str()); return false; } - handle->in = subprocess_stdin(&handle->sp); - handle->out = subprocess_stdout(&handle->sp); - handle->err = subprocess_stderr(&handle->sp); + handle->in = handle->sp.stdin_file(); + handle->out = handle->sp.stdout_file(); + handle->err = handle->sp.stderr_file(); proc = std::move(handle); running.store(true); @@ -654,14 +639,13 @@ void server_mcp_stdio::join_pumps() { to_server.close_write(); // wake the writer if it waits for a message from_server.close_write(); // wake any caller waiting for a reply - subprocess_terminate(&proc->sp); // child death unblocks the blocked fread/fwrite + proc->sp.terminate(); // child death unblocks the blocked fread/fwrite if (writer.joinable()) writer.join(); if (reader.joinable()) reader.join(); if (errlog.joinable()) errlog.join(); - subprocess_join(&proc->sp, nullptr); // reap the child: destroy() never waits, so the pid would stay a zombie for the process lifetime - subprocess_destroy(&proc->sp); // safe now: no thread touches the FILE* anymore + proc->sp.join(); // reap the child: never waiting would leave the pid a zombie for the process lifetime proc.reset(); } diff --git a/tools/server/server-models.cpp b/tools/server/server-models.cpp index ba6378814..923b3533e 100644 --- a/tools/server/server-models.cpp +++ b/tools/server/server-models.cpp @@ -8,10 +8,10 @@ #include "preset.h" #include "download.h" #include "http.h" +#include "subproc.h" #include // TODO: remove this once we use HTTP client from download.h #include -#include #include #include @@ -49,43 +49,24 @@ extern char **environ; #define CHILD_ADDR "127.0.0.1" struct server_subproc { - std::optional sproc; // empty while in DOWNLOADING state + common_subproc sproc; // not yet spawned while in DOWNLOADING state std::atomic 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_models::get_meta(const std::string & nam return std::nullopt; } -// helper to convert vector to char ** -// pointers are only valid as long as the original vector is valid -static std::vector to_char_ptr_array(const std::vector & vec) { - std::vector result; - result.reserve(vec.size() + 1); - for (const auto & s : vec) { - result.push_back(const_cast(s.c_str())); - } - result.push_back(nullptr); - return result; -} - std::vector server_models::get_all_meta() { std::unique_lock 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 argv = to_char_ptr_array(child_args); - std::vector 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) { @@ -1567,6 +1529,10 @@ static std::optional resolve_child_for_conv( } void server_models_routes::init_routes() { + if (!common_subproc::is_supported()) { + throw std::runtime_error("subprocess is not enabled on this build"); + } + this->get_router_props = [this](const server_http_req & req) { std::string name = req.get_param("model"); if (name.empty()) { diff --git a/tools/server/server-tools.cpp b/tools/server/server-tools.cpp index 2af44e49b..90b7e2a9f 100644 --- a/tools/server/server-tools.cpp +++ b/tools/server/server-tools.cpp @@ -1,6 +1,6 @@ #include "server-tools.h" -#include +#include "subproc.h" #include #include @@ -138,15 +138,14 @@ public: const std::function & 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; } @@ -159,14 +158,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) { @@ -177,7 +176,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 { @@ -195,8 +194,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(); @@ -207,16 +205,6 @@ public: } private: - static std::vector to_cstr_vec(const std::vector & v) { - std::vector r; - r.reserve(v.size() + 1); - for (const auto & s : v) { - r.push_back(const_cast(s.c_str())); - } - r.push_back(nullptr); - return r; - } - static const std::unordered_set & junk_dir_names() { static const std::unordered_set names = { ".git", ".svn", ".hg", "node_modules", "__pycache__", @@ -1203,6 +1191,10 @@ static std::vector> build_tools() { void server_tools::setup(const std::vector & enabled_tools, server_mcp & mcp_mgr) { if (!enabled_tools.empty()) { + if (!common_subproc::is_supported()) { + throw std::runtime_error("subprocess is not enabled on this build"); + } + std::unordered_set enabled_set(enabled_tools.begin(), enabled_tools.end()); auto all_tools = build_tools();