diff --git a/Makefile.cbm b/Makefile.cbm index ddd080822..4d2f23200 100644 --- a/Makefile.cbm +++ b/Makefile.cbm @@ -174,7 +174,7 @@ STORE_SRCS = src/store/store.c CYPHER_SRCS = src/cypher/cypher.c # MCP server module (new) -MCP_SRCS = src/mcp/mcp.c +MCP_SRCS = src/mcp/mcp.c src/mcp/index_supervisor.c # Discover module (new) DISCOVER_SRCS = \ diff --git a/internal/cbm/cbm.c b/internal/cbm/cbm.c index af4fda31e..aefcca4d9 100644 --- a/internal/cbm/cbm.c +++ b/internal/cbm/cbm.c @@ -494,9 +494,33 @@ static int count_params_from_signature(const char *sig) { // --- Main extraction function --- +/* Test-only deterministic fault injection for the crash/hang supervisor tests. + * Gated entirely behind env vars that are never set in production; a matching + * rel_path either aborts (a fault signal the supervisor classifies as a crash) + * or spins forever (an external-scanner infinite loop the quiet-timeout kills). + * This gives an honest guard — green iff the supervisor actually contains a real + * fault — instead of a fixture that may stop faulting once a root cause is fixed. */ +static void cbm_test_fault_inject(const char *rel_path) { + if (!rel_path || !rel_path[0]) { + return; + } + const char *crash_on = getenv("CBM_TEST_CRASH_ON"); + if (crash_on && crash_on[0] && strstr(rel_path, crash_on)) { + abort(); /* SIGABRT → WIFSIGNALED → classified as a crash */ + } + const char *hang_on = getenv("CBM_TEST_HANG_ON"); + if (hang_on && hang_on[0] && strstr(rel_path, hang_on)) { + for (;;) { + /* Busy-spin: the supervisor's quiet-timeout kills + reports us. */ + } + } +} + CBMFileResult *cbm_extract_file(const char *source, int source_len, CBMLanguage language, const char *project, const char *rel_path, int64_t timeout_micros, const char **extra_defines, const char **include_paths) { + cbm_test_fault_inject(rel_path); + // Allocate result on heap (arena inside for all string data) enum { SINGLE = 1 }; CBMFileResult *result = (CBMFileResult *)calloc(SINGLE, sizeof(CBMFileResult)); diff --git a/scripts/smoke-invariants.sh b/scripts/smoke-invariants.sh index fc35e0d2f..8e14c5b66 100755 --- a/scripts/smoke-invariants.sh +++ b/scripts/smoke-invariants.sh @@ -707,6 +707,47 @@ inv_garbage_files_cli() { fi } +# ── Invariant: a hard crash on one file is CONTAINED by the supervisor ────── +# A file that hard-crashes the native indexer (SIGSEGV/abort/stack-overflow) must +# not take down the whole process. Uses the test-only fault injector +# (CBM_TEST_CRASH_ON) so the guard is honest: with the supervisor OFF the crash +# must genuinely escape as a signal (rc>=128), and with it ON (default) the crash +# must be contained (rc<128) and reported (outcome=crash). If the baseline does +# not crash, the injector is inactive and the guard would be vacuous → fail. +inv_crasher_contained_cli() { + local crepo="$SCRATCH/crasher_repo" + mkdir -p "$crepo" + printf 'def good():\n return 1\n' > "$crepo/good.py" + printf 'def boom():\n return 2\n' > "$crepo/crash_me.py" + git -C "$crepo" init -q 2>/dev/null || true + local cn; cn="$(native_path "$crepo")" + + # Honesty baseline: supervisor OFF → the injected fault must escape as a signal. + export CBM_TEST_CRASH_ON=crash_me + export CBM_INDEX_SUPERVISOR=0 + cli_call 60 index_repository "{\"repo_path\":\"$cn\"}" + local base_rc="$CLI_RC" + unset CBM_INDEX_SUPERVISOR + + # Supervisor ON (default) → the crash must be contained and reported. + cli_call 60 index_repository "{\"repo_path\":\"$cn\"}" + local sup_rc="$CLI_RC" + local sup_out="$CLI_OUT" + unset CBM_TEST_CRASH_ON + + if [ "$base_rc" -le 128 ]; then + fail "crasher-contained-cli" "baseline did not crash (rc=$base_rc) — injector inactive, guard would be vacuous" + elif [ "$sup_rc" -eq 124 ]; then + fail "crasher-contained-cli" "supervised run hung (rc=124)" + elif [ "$sup_rc" -gt 128 ]; then + fail "crasher-contained-cli" "crash escaped the supervisor (signal $((sup_rc-128)))" + elif ! printf '%s' "$sup_out" | grep -q '"outcome":"crash"'; then + fail "crasher-contained-cli" "crash not reported (no outcome=crash): $sup_out" + else + pass "crasher-contained-cli (baseline rc=$base_rc escaped; supervised rc=$sup_rc contained + reported)" + fi +} + # ── Invariant 8: clean exit on stdin EOF within a bounded wait (no hang) ──── # Close the server's stdin (fd3). The server must reach EOF, break its loop, and # exit cleanly. We bound the wait WITHOUT sleep: closing stdin makes the server @@ -841,6 +882,7 @@ inv_index_status_cli inv_nonexistent_repo_cli inv_empty_repo_cli inv_garbage_files_cli +inv_crasher_contained_cli # MCP server-lifecycle invariants (one shared server instance). inv_mcp_initialize diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 14ba76a40..a0cd5dadb 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -15,6 +15,7 @@ #include #else #include +#include #include #include #include @@ -225,13 +226,20 @@ static int cbm_run_posix(const cbm_proc_opts_t *opts, cbm_proc_result_t *out) { return -1; } if (pid == 0) { - /* Child: redirect stdout+stderr to the log (or discard), then exec. */ - if (opts->log_file) { - (void)!freopen(opts->log_file, "w", stderr); - (void)!freopen(opts->log_file, "a", stdout); - } else { - (void)!freopen("/dev/null", "w", stderr); - (void)!freopen("/dev/null", "w", stdout); + /* Child: redirect stdout+stderr to the log (or discard), then exec. + * Use open()+dup2() (async-signal-safe, no malloc) rather than freopen(): + * the parent may be multithreaded (the MCP server holds worker/watcher/http + * threads plus mimalloc/sqlite/libgit2 global state), and a fork() copies + * only the calling thread — a malloc between fork and exec could deadlock on + * a lock another thread held at fork time. open/dup2/execv touch no heap. */ + const char *target = opts->log_file ? opts->log_file : "/dev/null"; + int fd = open(target, O_WRONLY | O_CREAT | O_TRUNC, 0644); + if (fd >= 0) { + (void)dup2(fd, STDOUT_FILENO); + (void)dup2(fd, STDERR_FILENO); + if (fd > STDERR_FILENO) { + (void)close(fd); + } } execv(bin, (char *const *)argv); _exit(127); /* exec failed */ diff --git a/src/main.c b/src/main.c index a58acdbbd..2870a6c92 100644 --- a/src/main.c +++ b/src/main.c @@ -15,6 +15,7 @@ */ #include "cbm.h" // cbm_alloc_init — bind 3rd-party allocators to mimalloc before any sqlite/git init #include "mcp/mcp.h" +#include "mcp/index_supervisor.h" #include "watcher/watcher.h" #include "pipeline/pipeline.h" #include "store/store.h" @@ -37,6 +38,7 @@ enum { #include "foundation/diagnostics.h" #include "foundation/platform.h" #include "foundation/compat.h" +#include "foundation/compat_fs.h" #include "foundation/compat_thread.h" #include "foundation/mem.h" #include "foundation/profile.h" @@ -228,6 +230,25 @@ static bool cli_strip_flag(int *argc, char **argv, const char *flag) { return false; } +/* Strip a flag AND its following value from argv, returning the value (a pointer + * into the original argv strings, valid for the process lifetime) or NULL if the + * flag is absent. */ +static const char *cli_strip_flag_value(int *argc, char **argv, const char *flag) { + for (int i = 0; i < *argc; i++) { + if (strcmp(argv[i], flag) != 0) { + continue; + } + const char *value = (i + SKIP_ONE < *argc) ? argv[i + SKIP_ONE] : NULL; + int remove_count = value ? 2 : 1; + for (int j = i; j < *argc - remove_count; j++) { + argv[j] = argv[j + remove_count]; + } + *argc -= remove_count; + return value; + } + return NULL; +} + /* Portable "is fd a terminal?" — _isatty on Windows, isatty on POSIX. */ #ifdef _WIN32 #define cli_isatty(fd) _isatty(fd) @@ -296,6 +317,14 @@ static int run_cli(int argc, char **argv) { bool progress = cli_strip_flag(&argc, argv, "--progress"); bool raw_json = cli_strip_flag(&argc, argv, "--json"); + /* Supervisor worker role: when this process was spawned as a supervised index + * worker, run indexing in-process (never re-supervise) and write the result to + * the given file for the parent to read back. Stripped here so the tool + * dispatch below sees only the tool name + its args. */ + bool index_worker = cli_strip_flag(&argc, argv, "--index-worker"); + const char *response_out = cli_strip_flag_value(&argc, argv, "--response-out"); + cbm_index_set_worker_role(index_worker, response_out); + if (argc < MAIN_MIN_ARGC) { (void)fprintf(stderr, CLI_USAGE); return SKIP_ONE; @@ -390,6 +419,16 @@ static int run_cli(int argc, char **argv) { int exit_code = 0; if (result) { + /* Supervised worker: hand the full result string to the parent via the + * response file before printing (parent reads it back on a clean exit). */ + const char *ro = cbm_index_worker_response_out(); + if (ro) { + FILE *rf = cbm_fopen(ro, "wb"); + if (rf) { + (void)fputs(result, rf); + (void)fclose(rf); + } + } if (raw_json) { printf("%s\n", result); } else { diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c new file mode 100644 index 000000000..48b24e1ce --- /dev/null +++ b/src/mcp/index_supervisor.c @@ -0,0 +1,179 @@ +/* + * index_supervisor.c — see index_supervisor.h. + */ +#include "index_supervisor.h" + +#include "foundation/compat_fs.h" /* cbm_mkdir_p, cbm_fopen */ +#include "foundation/log.h" +#include "foundation/platform.h" /* cbm_resolve_cache_dir */ +#include "ui/http_server.h" /* cbm_http_server_resolve_binary_path */ + +#include +#include +#include + +#ifdef _WIN32 +#include /* _getpid */ +#define cbm_getpid _getpid +#else +#include /* getpid */ +#define cbm_getpid getpid +#endif + +/* ── Worker-role state ────────────────────────────────────────────── */ + +static bool g_worker_active = false; +static char g_worker_response_out[1024] = {0}; + +void cbm_index_set_worker_role(bool is_worker, const char *response_out) { + g_worker_active = is_worker; + if (response_out && response_out[0]) { + snprintf(g_worker_response_out, sizeof(g_worker_response_out), "%s", response_out); + } else { + g_worker_response_out[0] = '\0'; + } +} + +bool cbm_index_worker_active(void) { + return g_worker_active; +} + +const char *cbm_index_worker_response_out(void) { + return g_worker_response_out[0] ? g_worker_response_out : NULL; +} + +bool cbm_index_supervisor_should_wrap(void) { + if (g_worker_active) { + return false; /* I am the worker — run in-process, never re-supervise */ + } + const char *sv = getenv("CBM_INDEX_SUPERVISOR"); + if (sv && strcmp(sv, "0") == 0) { + return false; /* kill switch → in-process */ + } + return true; +} + +/* Quiet-timeout (ms) for a supervised worker: killed + reported as a hang if it + * emits no progress within the window. 0 => disabled. Full hang coverage (a + * generous default + attribution) is wired in a later stage; for now honour the + * env override and default to disabled so crash containment ships independently. */ +static int worker_quiet_timeout_ms(void) { + const char *e = getenv("CBM_INDEX_WORKER_TIMEOUT_S"); + if (e && e[0]) { + long s = atol(e); + if (s > 0) { + return (int)(s * 1000); + } + } + return 0; +} + +/* Read an entire file into a heap string (NUL-terminated). NULL on error. */ +static char *slurp_file(const char *path) { + FILE *f = cbm_fopen(path, "rb"); + if (!f) { + return NULL; + } + if (fseek(f, 0, SEEK_END) != 0) { + (void)fclose(f); + return NULL; + } + long n = ftell(f); + if (n < 0) { + (void)fclose(f); + return NULL; + } + (void)fseek(f, 0, SEEK_SET); + char *buf = (char *)malloc((size_t)n + 1); + if (!buf) { + (void)fclose(f); + return NULL; + } + size_t rd = fread(buf, 1, (size_t)n, f); + (void)fclose(f); + buf[rd] = '\0'; + return buf; +} + +/* Resolve a per-run temp path /logs/.worker-. */ +static void worker_tmp_path(char *out, size_t out_sz, int pid, const char *suffix) { + const char *cdir = cbm_resolve_cache_dir(); + if (cdir && cdir[0]) { + char dir[900]; + snprintf(dir, sizeof(dir), "%s/logs", cdir); + cbm_mkdir_p(dir, 0755); + snprintf(out, out_sz, "%s/.worker-%d%s", dir, pid, suffix); + } else { + snprintf(out, out_sz, ".worker-%d%s", pid, suffix); + } +} + +int cbm_index_spawn_worker(const char *args_json, const char *exclude_file, + cbm_index_worker_result_t *result) { + result->outcome = CBM_PROC_SPAWN_FAILED; + result->exit_code = -1; + result->term_signal = 0; + result->response = NULL; + + char self[1024] = {0}; + if (!cbm_http_server_resolve_binary_path(NULL, self, sizeof(self)) || !self[0]) { + cbm_log_warn("index.supervisor.no_self_path", "action", "degrade_in_process"); + return -1; + } + + int pid = (int)cbm_getpid(); + char resp_path[1024]; + char log_path[1024]; + worker_tmp_path(resp_path, sizeof(resp_path), pid, ".response"); + worker_tmp_path(log_path, sizeof(log_path), pid, ".log"); + (void)remove(resp_path); /* clear any stale file */ + + const char *argv[12]; + int n = 0; + argv[n++] = self; + argv[n++] = "cli"; + argv[n++] = "--index-worker"; + argv[n++] = "index_repository"; + argv[n++] = args_json; + argv[n++] = "--response-out"; + argv[n++] = resp_path; + if (exclude_file && exclude_file[0]) { + argv[n++] = "--exclude-file"; + argv[n++] = exclude_file; + } + argv[n] = NULL; + + cbm_proc_opts_t opts = {0}; + opts.bin = self; + opts.argv = argv; + opts.log_file = log_path; + opts.quiet_timeout_ms = worker_quiet_timeout_ms(); + opts.delete_log_on_exit = true; + + cbm_proc_result_t r; + if (cbm_subprocess_run(&opts, &r) != 0) { + (void)remove(resp_path); + cbm_log_warn("index.supervisor.spawn_failed", "action", "degrade_in_process"); + return -1; + } + + result->outcome = r.outcome; + result->exit_code = r.exit_code; + result->term_signal = r.term_signal; + if (r.outcome == CBM_PROC_CLEAN) { + result->response = slurp_file(resp_path); + } + (void)remove(resp_path); + + char sig[16]; + snprintf(sig, sizeof(sig), "%d", r.term_signal); + cbm_log_info("index.supervisor.reap", "outcome", cbm_proc_outcome_str(r.outcome), "signal", sig); + return 0; +} + +void cbm_index_worker_result_free(cbm_index_worker_result_t *result) { + if (result) { + free(result->response); + result->response = NULL; + } +} diff --git a/src/mcp/index_supervisor.h b/src/mcp/index_supervisor.h new file mode 100644 index 000000000..3c05bc914 --- /dev/null +++ b/src/mcp/index_supervisor.h @@ -0,0 +1,58 @@ +/* + * index_supervisor.h — run index_repository in a supervised worker subprocess. + * + * A single pathological file can hard-crash (SIGSEGV / stack overflow / abort) or + * hang the native indexer, and today that takes down the whole MCP server or CLI. + * The supervisor runs the actual index in a CHILD process (the same binary + * re-invoked as `cli --index-worker index_repository …`), reaps it, and classifies + * how it ended. A crash/hang is contained to the child; the parent survives and + * reports it instead of dying. + * + * This module owns only the spawn/reap MECHANISM and the worker-role state. The + * MCP handler (mcp.c) owns the gate placement and the response building, so this + * module has no dependency on the response format. + * + * fork+exec only (never fork-and-run-in-child): the server holds persistent + * threads plus mimalloc/sqlite/libgit2 global state with no pthread_atfork, so a + * fork without exec would be a latent deadlock. Recursion is prevented by an argv + * flag (`--index-worker`), never an ambient env var. + */ +#ifndef CBM_INDEX_SUPERVISOR_H +#define CBM_INDEX_SUPERVISOR_H + +#include + +#include "foundation/subprocess.h" /* cbm_proc_outcome_t */ + +/* Worker-role state, set once from the CLI arg parser (main.c) when this process + * was spawned as a supervised worker. When active, indexing must run in-process + * (the gate must NOT re-supervise). response_out (may be NULL) is the file the + * worker writes its final result string to, for the parent to read back. */ +void cbm_index_set_worker_role(bool is_worker, const char *response_out); +bool cbm_index_worker_active(void); +const char *cbm_index_worker_response_out(void); + +/* True when handle_index_repository should wrap the run in a supervised child: + * this process is not itself a worker AND the kill switch (CBM_INDEX_SUPERVISOR=0) + * is not set. */ +bool cbm_index_supervisor_should_wrap(void); + +typedef struct { + cbm_proc_outcome_t outcome; /* how the worker ended */ + int exit_code; /* worker exit code (-1 if signalled) */ + int term_signal; /* POSIX terminating signal, else 0 */ + char *response; /* worker's result string on CLEAN exit (caller frees); else NULL */ +} cbm_index_worker_result_t; + +/* Spawn ` cli --index-worker index_repository --response-out + * [--exclude-file ]`, supervise it (quiet-timeout for hangs), reap, + * and classify. On a clean exit, result->response holds the worker's response + * string (read from the temp file). Returns 0 if a worker was spawned and reaped + * (result filled), or -1 if the child could not be spawned (caller degrades to + * in-process). exclude_file may be NULL. */ +int cbm_index_spawn_worker(const char *args_json, const char *exclude_file, + cbm_index_worker_result_t *result); + +void cbm_index_worker_result_free(cbm_index_worker_result_t *result); + +#endif /* CBM_INDEX_SUPERVISOR_H */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index ac9fd00e3..1b14dd0db 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -54,6 +54,7 @@ enum { #include "foundation/compat_fs.h" #include "foundation/compat_thread.h" #include "foundation/log.h" +#include "mcp/index_supervisor.h" #include "foundation/str_util.h" #include "foundation/dump_verify.h" #include "foundation/compat_regex.h" @@ -3311,7 +3312,89 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * return degraded; } +/* Build the response for a worker that crashed/hung/failed without producing a + * result. The crash is already contained (this process survived); we report it + * rather than dying. Precise skip-and-continue (quarantine the culprit, index the + * rest) is layered on in the probe stage. */ +static char *build_worker_failure_response(const char *args, cbm_proc_outcome_t outcome) { + char *repo_path = cbm_mcp_get_string_arg(args, "repo_path"); + yyjson_mut_doc *doc = yyjson_mut_doc_new(NULL); + yyjson_mut_val *root = yyjson_mut_obj(doc); + yyjson_mut_doc_set_root(doc, root); + yyjson_mut_obj_add_str(doc, root, "status", "error"); + yyjson_mut_obj_add_str(doc, root, "outcome", cbm_proc_outcome_str(outcome)); + yyjson_mut_obj_add_str( + doc, root, "hint", + outcome == CBM_PROC_HANG + ? "Indexing worker timed out (a file made no progress). The worker was " + "terminated and the server survived. Re-run to retry." + : "Indexing worker crashed on a file. The crash was contained (the server " + "survived). Re-run to retry; a future release isolates the culprit file."); + if (repo_path) { + yyjson_mut_obj_add_strcpy(doc, root, "repo_path", repo_path); + } + char *json = yy_doc_to_str(doc); + yyjson_mut_doc_free(doc); + free(repo_path); + char *result = cbm_mcp_text_result(json, true); + free(json); + return result; +} + +/* Run index_repository in a supervised worker subprocess. Returns the response + * string (caller frees) — the worker's own response on a clean exit, or a + * contained-failure response on a crash/hang. Returns NULL only when the worker + * could not be spawned, so the caller degrades to the in-process path. */ +static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { + /* Drop the cached store: the worker (a fresh process) deletes + recreates the + * .db, and the parent must reopen fresh afterwards. */ + if (srv->owns_store && srv->store) { + cbm_store_close(srv->store); + srv->store = NULL; + } + free(srv->current_project); + srv->current_project = NULL; + + cbm_index_worker_result_t wr; + int rc = cbm_index_spawn_worker(args, NULL, &wr); + + /* Invalidate the cached store again so the next query reopens whatever the + * worker wrote (or the prior DB, if it crashed before its final dump). */ + if (srv->owns_store && srv->store) { + cbm_store_close(srv->store); + srv->store = NULL; + } + free(srv->current_project); + srv->current_project = NULL; + + if (rc != 0 || wr.outcome == CBM_PROC_SPAWN_FAILED) { + cbm_index_worker_result_free(&wr); + return NULL; /* degrade to in-process */ + } + + if (wr.outcome == CBM_PROC_CLEAN && wr.response) { + char *resp = wr.response; /* transfer ownership to caller */ + wr.response = NULL; + cbm_index_worker_result_free(&wr); + return resp; + } + + char *resp = build_worker_failure_response(args, wr.outcome); + cbm_index_worker_result_free(&wr); + return resp; +} + static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { + /* Supervisor gate: run the index in a crash/hang-isolating worker subprocess + * unless this process IS the worker or the kill switch (CBM_INDEX_SUPERVISOR=0) + * is set. On spawn failure, fall through to the in-process path (degrade). */ + if (cbm_index_supervisor_should_wrap()) { + char *supervised = index_run_supervised(srv, args); + if (supervised) { + return supervised; + } + } + char *repo_path = cbm_mcp_get_string_arg(args, "repo_path"); char *mode_str = cbm_mcp_get_string_arg(args, "mode"); char *name_override = cbm_mcp_get_string_arg(args, "name"); diff --git a/src/ui/http_server.c b/src/ui/http_server.c index c12685ef9..ee198cf8c 100644 --- a/src/ui/http_server.c +++ b/src/ui/http_server.c @@ -786,7 +786,11 @@ static void *index_thread_fn(void *arg) { /* Build command line for CreateProcess */ char cmdline[2048]; - snprintf(cmdline, sizeof(cmdline), "\"%s\" cli index_repository \"%s\"", bin, json_arg); + /* --index-worker: this http_server spawn is already the crash-isolation layer, + * so the child runs indexing in-process rather than spawning its own supervisor + * (avoids redundant process nesting). */ + snprintf(cmdline, sizeof(cmdline), "\"%s\" cli --index-worker index_repository \"%s\"", bin, + json_arg); cbm_log_info("ui.index.spawn", "bin", bin, "log", log_file); @@ -854,7 +858,7 @@ static void *index_thread_fn(void *arg) { FILE *lf = freopen(log_file, "w", stderr); (void)lf; freopen("/dev/null", "w", stdout); - execl(bin, bin, "cli", "index_repository", json_arg, (char *)NULL); + execl(bin, bin, "cli", "--index-worker", "index_repository", json_arg, (char *)NULL); _exit(127); }