From de1615264aa7ec2ef997d8e73fede1f0abc71965 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=86=B2?= Date: Wed, 19 Aug 2026 14:39:38 +0800 Subject: [PATCH 1/8] feat(index): add worker resource watchdogs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Discovery limits bound what indexing accepts, not what it then costs. A repository well inside those bounds can still exhaust the host through parser memory, or simply never finish, and a supervised worker that hangs leaves the parent waiting with nothing to report. Add index_max_rss_mb and index_max_duration_seconds, enforced by the parent against the worker process tree rather than the worker process alone, so a runaway child cannot hide behind a small parent. Resident memory is sampled through the platform interface on macOS, Linux and Windows. Crossing a limit terminates the tree and yields one trusted, structured terminal result that attributes the failure to the resource that caused it. Both limits default to off. A measurement that cannot be taken fails the attempt instead of passing it: a watchdog that quietly stops watching is worse than no watchdog at all. The shell fixture that stands in for the supervisor names the two new keys. The worker accepts only a policy that spells out every key it knows, which is what keeps a stale supervisor from starting a worker it cannot bound. Signed-off-by: 刘冲 --- README.md | 4 +- docs/CONFIGURATION.md | 10 +- docs/INDEX_RESOURCE_LIMITS.md | 48 +++++- scripts/test-runtime.sh | 3 +- src/cli/cli.c | 10 +- src/daemon/application.c | 20 ++- src/foundation/index_policy.c | 126 +++++++++----- src/foundation/index_policy.h | 11 ++ src/foundation/subprocess.c | 285 ++++++++++++++++++++++++++++++++ src/foundation/subprocess.h | 18 ++ src/mcp/index_supervisor.c | 267 ++++++++++++++++++++++++++++-- src/mcp/index_supervisor.h | 38 ++++- src/mcp/mcp.c | 114 ++++++++++--- src/mcp/mcp.h | 1 + src/mcp/mcp_internal.h | 4 + src/pipeline/pipeline.c | 5 +- tests/test_daemon_application.c | 133 +++++++++++++-- tests/test_index_policy.c | 72 +++++++- tests/test_index_supervisor.c | 269 ++++++++++++++++++++++++++++++ tests/test_mcp.c | 98 +++++++++++ tests/test_subprocess.c | 47 ++++++ 21 files changed, 1472 insertions(+), 111 deletions(-) diff --git a/README.md b/README.md index 590c9eef9..4466dae50 100644 --- a/README.md +++ b/README.md @@ -928,10 +928,12 @@ codebase-memory-mcp config set watcher_enabled false # stop the watcher thre codebase-memory-mcp config set watch_non_git true # also poll non-git project roots (default: false) codebase-memory-mcp config set index_max_files 250000 # optional per-index source-file limit codebase-memory-mcp config set index_max_source_mb 16384 # optional per-index source-size limit +codebase-memory-mcp config set index_max_rss_mb 8192 # optional worker-tree current RSS limit +codebase-memory-mcp config set index_max_duration_seconds 3600 # optional total worker duration codebase-memory-mcp config reset auto_index # reset to default ``` -The two `index_max_*` settings default to `off`. Exceeding one fails the complete +The four `index_max_*` settings default to `off`. Exceeding one fails the complete index attempt rather than publishing a partial graph; an existing serving index is preserved. See [Index resource limits](docs/INDEX_RESOURCE_LIMITS.md). diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 3e9956d60..3d3e9a971 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -93,6 +93,8 @@ Current keys: | `watch_non_git` | `false` | Also poll project roots that are **not git repositories**. By default the watcher only follows git projects, so a project indexed from a plain directory is never refreshed after its first index — reindex it manually with `index_repository`. Set `true` to poll such roots on the same adaptive cadence with a file-tree scan: the indexer's own discovery walk (same skip lists, `.gitignore` and `.cbmignore` rules) hashed over each file's path, size and mtime. Any change reindexes once; paths the indexer skips (including cbm's own `.codebase-memory/` output) never trigger. The first poll after the daemon starts reindexes each such project once, since nothing records which tree state the index holds. The scan walks the whole tree every poll, so it costs more than git polling on very large trees. Read once when the daemon starts, like `watcher_enabled`. | | `index_max_files` | `off` | Optional maximum number of accepted source files in one discovery run. | | `index_max_source_mb` | `off` | Optional maximum accepted source size in MiB in one discovery run. | +| `index_max_rss_mb` | `off` | Optional maximum current RSS in MiB for the complete contained index-worker process tree (`64..1048576`). | +| `index_max_duration_seconds` | `off` | Optional maximum total worker duration in seconds (`1..86400`). | For a watched project, `index_status` names the polling method in `watch.strategy`: `git` (HEAD and dirty-state polling), `tree` (a non-git root polled by the @@ -126,11 +128,13 @@ has not run yet). > `auto_index` still runs, and `index_repository` stays available for manual > reindexing. -The two `index_max_*` settings are independent and disabled by default. They +The four `index_max_*` settings are independent and disabled by default. They apply to explicit indexing, automatic indexing, and watcher re-indexing, but not to `cross-repo-intelligence`, which does not scan repository source files. -Equality is allowed; exceeding either setting fails the complete index request -and preserves any previously serving database. See +Equality is allowed; exceeding any setting fails the complete index request and +preserves any previously serving database. Worker RSS covers descendants and is +not the same as the internal `CBM_MEM_BUDGET_MB` allocation budget. Total +duration is independent of the existing 15-minute no-log-progress timeout. See [Index resource limits](INDEX_RESOURCE_LIMITS.md) for counting, validation, and error-response details. diff --git a/docs/INDEX_RESOURCE_LIMITS.md b/docs/INDEX_RESOURCE_LIMITS.md index a3d57e015..66be4ecf4 100644 --- a/docs/INDEX_RESOURCE_LIMITS.md +++ b/docs/INDEX_RESOURCE_LIMITS.md @@ -1,8 +1,8 @@ # Index resource limits Index resource limits are optional operator controls for repositories whose -discovery breadth is not known in advance. They are disabled by default so -existing large-repository workloads retain their current behavior. +discovery breadth or worker runtime is not known in advance. They are disabled +by default so existing large-repository workloads retain their current behavior. ## Discovery settings @@ -23,6 +23,39 @@ Values use base-10 integers. MiB means 1,048,576 bytes. Empty values, zero, negative values, suffixes, trailing characters, and values outside the stated ranges are rejected without changing the stored value. +## Worker settings + +| Key | Default | Accepted value | Protects | +|---|---:|---:|---| +| `index_max_rss_mb` | `off` | `off` or `64..1048576` | Current RSS of the complete worker process tree | +| `index_max_duration_seconds` | `off` | `off` or `1..86400` | Total worker wall-clock duration | + +```bash +codebase-memory-mcp config set index_max_rss_mb 8192 +codebase-memory-mcp config set index_max_duration_seconds 3600 +``` + +RSS is the current resident memory of the contained worker and every descendant, +not the worker's allocation budget and not peak memory. This hard watchdog is +separate from the internal `CBM_MEM_BUDGET_MB` soft budget. The supervisor +samples RSS at most once every 250 milliseconds so the watchdog does not turn +full process-table enumeration into a busy loop. + +Duration uses a monotonic clock from successful spawn. It is independent of the +existing 15-minute quiet timeout: continuous log progress does not reset total +duration, while the quiet timeout continues to identify a worker that stops +making progress. + +Equality is allowed. The first RSS or elapsed-duration observation above its +limit starts the existing graceful-to-force process-tree shutdown. CBM reports +terminal only after the tree is quiescent or a bounded containment failure is +explicitly surfaced. Resource termination is not retried and does not +quarantine a source file. + +If RSS is enabled and three consecutive probes cannot obtain any trustworthy +tree measurement while the root worker is still running, CBM fails closed with +`code=resource_probe_failed`. + ## Counting and failure semantics `index_max_files` counts a file only after it passes directory pruning, ignore @@ -55,6 +88,12 @@ The previous database remains available because publication occurs only after a complete discovery and successful staged build. If no previous database exists, `serving_index_preserved` is false. +Worker limit failures use the same shape with `stage=worker`, +`resource=rss_bytes` and `unit=bytes`, or `resource=duration_ms` and +`unit=milliseconds`. RSS measurement failures use `code=resource_probe_failed` +and omit `observed`, `limit`, and `unit` because no trustworthy observation was +available. + ## Trust and compatibility Limits are read from the CLI-managed `_config.db`; they are not MCP request @@ -64,5 +103,6 @@ parent policy. These settings do not replace or increase `auto_index_limit`, change the 512 MiB single-file cap, alter workspace-root authorization, or affect -`cross-repo-intelligence`. With both settings `off`, discovery follows the -existing unbounded path. +`cross-repo-intelligence`. With all settings `off`, discovery follows the +existing path and the supervisor performs no periodic RSS probe or total-duration +termination. diff --git a/scripts/test-runtime.sh b/scripts/test-runtime.sh index 5d2c0c3ae..e6e60b15a 100644 --- a/scripts/test-runtime.sh +++ b/scripts/test-runtime.sh @@ -112,7 +112,8 @@ _cbm_test_runtime_daemon() { # cbm_mcp_index_policy_add_to_args: every key in cbm_index_policy_key_at, and # nothing else. cbm_test_index_worker_policy_json() { - printf '%s' '"_cbm_index_policy":{"index_max_files":"off","index_max_source_mb":"off"}' + printf '%s' '"_cbm_index_policy":{"index_max_files":"off","index_max_source_mb":"off"' + printf '%s' ',"index_max_rss_mb":"off","index_max_duration_seconds":"off"}' } cbm_test_runtime_cleanup() { diff --git a/src/cli/cli.c b/src/cli/cli.c index 1ebe1687e..00b2db341 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -7603,6 +7603,8 @@ static const config_key_def_t CONFIG_KEYS[] = { {CBM_CONFIG_UI_PORT, "9749", "Port for the graph UI listener when enabled"}, {CBM_INDEX_CONFIG_MAX_FILES, "off", "Max accepted source files per index, or off"}, {CBM_INDEX_CONFIG_MAX_SOURCE_MB, "off", "Max accepted source MiB per index, or off"}, + {CBM_INDEX_CONFIG_MAX_RSS_MB, "off", "Max worker process-tree RSS MiB, or off"}, + {CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "off", "Max worker duration in seconds, or off"}, }; /* #1558: ui_enabled and ui_port were reachable ONLY by hand-editing @@ -7629,8 +7631,12 @@ static bool config_key_is_ui(const char *key) { } static bool config_key_is_index_policy(const char *key) { - return key && (strcmp(key, CBM_INDEX_CONFIG_MAX_FILES) == 0 || - strcmp(key, CBM_INDEX_CONFIG_MAX_SOURCE_MB) == 0); + for (size_t index = 0; key && index < cbm_index_policy_key_count(); index++) { + if (strcmp(key, cbm_index_policy_key_at(index)) == 0) { + return true; + } + } + return false; } static int config_index_policy_write(cbm_config_t *config, const char *key, const char *value) { diff --git a/src/daemon/application.c b/src/daemon/application.c index 22b6f0d1a..a7b2f4f5b 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -363,6 +363,14 @@ static int application_worker_start_default(void *context, const char *args_json const char *quarantine_file, cbm_daemon_application_worker_t *worker_out) { (void)context; + cbm_index_resource_policy_t resource_policy; + char error[CBM_SZ_256] = {0}; + if (!cbm_mcp_index_policy_from_internal_args(args_json, &resource_policy, error, + sizeof(error))) { + cbm_log_error("daemon.index.policy", "error", error); + *worker_out = NULL; + return -1; + } application_default_worker_t *worker = cbm_calloc(CBM_MEM_CLASS_OTHER, sizeof(*worker)); if (!worker) { return -1; @@ -372,9 +380,9 @@ static int application_worker_start_default(void *context, const char *args_json const char *root = yyjson_get_str(yyjson_obj_get(yyjson_doc_get_root(args_doc), "repo_path")); worker->project = application_index_project_key(root, args_json); yyjson_doc_free(args_doc); - int result = cbm_index_worker_start_with_log(args_json, memory_budget_bytes, false, marker_file, - quarantine_file, application_worker_diagnostic, - worker, &worker->handle); + int result = cbm_index_worker_start_with_policy_log( + args_json, memory_budget_bytes, &resource_policy, false, marker_file, quarantine_file, + application_worker_diagnostic, worker, &worker->handle); if (result != 0) { application_worker_destroy_default(NULL, worker); *worker_out = NULL; @@ -1567,6 +1575,12 @@ static application_attempt_decision_t application_consume_attempt( application_attempt_free(attempt); return APPLICATION_ATTEMPT_DECISION_SUCCESS; } + if (disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { + execution->response = + cbm_mcp_index_worker_resource_response(job->args_json, &attempt->result); + application_attempt_free(attempt); + return APPLICATION_ATTEMPT_DECISION_STOP; + } if (disposition == CBM_MCP_SUPERVISED_RESULT_UNSAFE_TERMINAL) { execution->unsafe_terminal = true; execution->supervision_failed = diff --git a/src/foundation/index_policy.c b/src/foundation/index_policy.c index 0d2b986bf..80aa2f8a6 100644 --- a/src/foundation/index_policy.c +++ b/src/foundation/index_policy.c @@ -1,21 +1,38 @@ #include "foundation/index_policy.h" +#include #include #include -static const char *const INDEX_POLICY_KEYS[] = { - CBM_INDEX_CONFIG_MAX_FILES, - CBM_INDEX_CONFIG_MAX_SOURCE_MB, +typedef struct { + const char *key; + size_t field_offset; + uint64_t minimum; + uint64_t maximum; + uint64_t multiplier; +} index_policy_metadata_t; + +static const index_policy_metadata_t INDEX_POLICY_METADATA[] = { + {CBM_INDEX_CONFIG_MAX_FILES, offsetof(cbm_index_resource_policy_t, max_files), 1, + CBM_INDEX_MAX_FILES_VALUE, 1}, + {CBM_INDEX_CONFIG_MAX_SOURCE_MB, offsetof(cbm_index_resource_policy_t, max_source_bytes), 1, + CBM_INDEX_MAX_SOURCE_MB_VALUE, CBM_INDEX_MIB_BYTES}, + {CBM_INDEX_CONFIG_MAX_RSS_MB, offsetof(cbm_index_resource_policy_t, max_rss_bytes), + CBM_INDEX_MIN_RSS_MB_VALUE, CBM_INDEX_MAX_RSS_MB_VALUE, CBM_INDEX_MIB_BYTES}, + {CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, offsetof(cbm_index_resource_policy_t, max_duration_ms), + 1, CBM_INDEX_MAX_DURATION_SECONDS_VALUE, UINT64_C(1000)}, }; -static void set_error(char *error, size_t error_size, const char *key, uint64_t maximum) { +static void set_error(char *error, size_t error_size, const char *key, uint64_t minimum, + uint64_t maximum) { if (error && error_size > 0) { - (void)snprintf(error, error_size, "%s must be off or an integer from 1 to %llu", key, - (unsigned long long)maximum); + (void)snprintf(error, error_size, "%s must be off or an integer from %llu to %llu", key, + (unsigned long long)minimum, (unsigned long long)maximum); } } -static bool parse_bounded_uint64(const char *value, uint64_t maximum, uint64_t *parsed) { +static bool parse_bounded_uint64(const char *value, uint64_t minimum, uint64_t maximum, + uint64_t *parsed) { if (!value || !value[0] || !parsed) { return false; } @@ -30,7 +47,7 @@ static bool parse_bounded_uint64(const char *value, uint64_t maximum, uint64_t * } result = result * 10U + digit; } - if (result == 0) { + if (result < minimum) { return false; } *parsed = result; @@ -44,20 +61,39 @@ void cbm_index_policy_init(cbm_index_resource_policy_t *policy) { } bool cbm_index_policy_enabled(const cbm_index_resource_policy_t *policy) { + if (!policy) { + return false; + } + for (size_t index = 0; index < cbm_index_policy_key_count(); index++) { + const cbm_index_limit_u64_t *limit = + (const cbm_index_limit_u64_t *)((const unsigned char *)policy + + INDEX_POLICY_METADATA[index].field_offset); + if (limit->enabled) { + return true; + } + } + return false; +} + +bool cbm_index_policy_discovery_enabled(const cbm_index_resource_policy_t *policy) { return policy && (policy->max_files.enabled || policy->max_source_bytes.enabled); } +bool cbm_index_policy_worker_enabled(const cbm_index_resource_policy_t *policy) { + return policy && (policy->max_rss_bytes.enabled || policy->max_duration_ms.enabled); +} + size_t cbm_index_policy_key_count(void) { - return sizeof(INDEX_POLICY_KEYS) / sizeof(INDEX_POLICY_KEYS[0]); + return sizeof(INDEX_POLICY_METADATA) / sizeof(INDEX_POLICY_METADATA[0]); } const char *cbm_index_policy_key_at(size_t index) { - return index < cbm_index_policy_key_count() ? INDEX_POLICY_KEYS[index] : NULL; + return index < cbm_index_policy_key_count() ? INDEX_POLICY_METADATA[index].key : NULL; } const char *cbm_index_policy_default_value(const char *key) { for (size_t index = 0; index < cbm_index_policy_key_count(); index++) { - if (key && strcmp(key, INDEX_POLICY_KEYS[index]) == 0) { + if (key && strcmp(key, INDEX_POLICY_METADATA[index].key) == 0) { return "off"; } } @@ -70,34 +106,33 @@ bool cbm_index_policy_set(cbm_index_resource_policy_t *policy, const char *key, error[0] = '\0'; } if (!policy || !key || !value) { - set_error(error, error_size, key ? key : "index resource limit", 0); + set_error(error, error_size, key ? key : "index resource limit", 0, 0); return false; } - cbm_index_limit_u64_t *target = NULL; - uint64_t maximum = 0; - uint64_t multiplier = 1; - if (strcmp(key, CBM_INDEX_CONFIG_MAX_FILES) == 0) { - target = &policy->max_files; - maximum = CBM_INDEX_MAX_FILES_VALUE; - } else if (strcmp(key, CBM_INDEX_CONFIG_MAX_SOURCE_MB) == 0) { - target = &policy->max_source_bytes; - maximum = CBM_INDEX_MAX_SOURCE_MB_VALUE; - multiplier = CBM_INDEX_MIB_BYTES; - } else { - set_error(error, error_size, key, 0); + const index_policy_metadata_t *metadata = NULL; + for (size_t index = 0; index < cbm_index_policy_key_count(); index++) { + if (strcmp(key, INDEX_POLICY_METADATA[index].key) == 0) { + metadata = &INDEX_POLICY_METADATA[index]; + break; + } + } + if (!metadata) { + set_error(error, error_size, key, 0, 0); return false; } + cbm_index_limit_u64_t *target = + (cbm_index_limit_u64_t *)((unsigned char *)policy + metadata->field_offset); cbm_index_limit_u64_t candidate = {0}; if (strcmp(value, "off") != 0) { uint64_t parsed = 0; - if (!parse_bounded_uint64(value, maximum, &parsed)) { - set_error(error, error_size, key, maximum); + if (!parse_bounded_uint64(value, metadata->minimum, metadata->maximum, &parsed)) { + set_error(error, error_size, key, metadata->minimum, metadata->maximum); return false; } candidate.enabled = true; - candidate.value = parsed * multiplier; + candidate.value = parsed * metadata->multiplier; } *target = candidate; return true; @@ -108,18 +143,21 @@ bool cbm_index_policy_format(const cbm_index_resource_policy_t *policy, const ch if (!policy || !key || !out || out_size == 0) { return false; } - const cbm_index_limit_u64_t *limit = NULL; - uint64_t divisor = 1; - if (strcmp(key, CBM_INDEX_CONFIG_MAX_FILES) == 0) { - limit = &policy->max_files; - } else if (strcmp(key, CBM_INDEX_CONFIG_MAX_SOURCE_MB) == 0) { - limit = &policy->max_source_bytes; - divisor = CBM_INDEX_MIB_BYTES; - } else { + const index_policy_metadata_t *metadata = NULL; + for (size_t index = 0; index < cbm_index_policy_key_count(); index++) { + if (strcmp(key, INDEX_POLICY_METADATA[index].key) == 0) { + metadata = &INDEX_POLICY_METADATA[index]; + break; + } + } + if (!metadata) { return false; } + const cbm_index_limit_u64_t *limit = + (const cbm_index_limit_u64_t *)((const unsigned char *)policy + metadata->field_offset); int length = limit->enabled - ? snprintf(out, out_size, "%llu", (unsigned long long)(limit->value / divisor)) + ? snprintf(out, out_size, "%llu", + (unsigned long long)(limit->value / metadata->multiplier)) : snprintf(out, out_size, "off"); return length >= 0 && (size_t)length < out_size; } @@ -130,6 +168,10 @@ const char *cbm_index_resource_name(cbm_index_resource_t resource) { return "files"; case CBM_INDEX_RESOURCE_SOURCE_BYTES: return "source_bytes"; + case CBM_INDEX_RESOURCE_RSS_BYTES: + return "rss_bytes"; + case CBM_INDEX_RESOURCE_DURATION_MS: + return "duration_ms"; case CBM_INDEX_RESOURCE_NONE: default: return "unknown"; @@ -137,7 +179,13 @@ const char *cbm_index_resource_name(cbm_index_resource_t resource) { } const char *cbm_index_resource_unit(cbm_index_resource_t resource) { - return resource == CBM_INDEX_RESOURCE_FILES ? "files" : "bytes"; + if (resource == CBM_INDEX_RESOURCE_FILES) { + return "files"; + } + if (resource == CBM_INDEX_RESOURCE_DURATION_MS) { + return "milliseconds"; + } + return "bytes"; } const char *cbm_index_resource_config_key(cbm_index_resource_t resource) { @@ -146,6 +194,10 @@ const char *cbm_index_resource_config_key(cbm_index_resource_t resource) { return CBM_INDEX_CONFIG_MAX_FILES; case CBM_INDEX_RESOURCE_SOURCE_BYTES: return CBM_INDEX_CONFIG_MAX_SOURCE_MB; + case CBM_INDEX_RESOURCE_RSS_BYTES: + return CBM_INDEX_CONFIG_MAX_RSS_MB; + case CBM_INDEX_RESOURCE_DURATION_MS: + return CBM_INDEX_CONFIG_MAX_DURATION_SECONDS; case CBM_INDEX_RESOURCE_NONE: default: return "index_resource_limit"; diff --git a/src/foundation/index_policy.h b/src/foundation/index_policy.h index 6a6c499fc..617a6ab7d 100644 --- a/src/foundation/index_policy.h +++ b/src/foundation/index_policy.h @@ -7,9 +7,14 @@ #define CBM_INDEX_CONFIG_MAX_FILES "index_max_files" #define CBM_INDEX_CONFIG_MAX_SOURCE_MB "index_max_source_mb" +#define CBM_INDEX_CONFIG_MAX_RSS_MB "index_max_rss_mb" +#define CBM_INDEX_CONFIG_MAX_DURATION_SECONDS "index_max_duration_seconds" #define CBM_INDEX_MAX_FILES_VALUE UINT64_C(10000000) #define CBM_INDEX_MAX_SOURCE_MB_VALUE UINT64_C(1048576) +#define CBM_INDEX_MIN_RSS_MB_VALUE UINT64_C(64) +#define CBM_INDEX_MAX_RSS_MB_VALUE UINT64_C(1048576) +#define CBM_INDEX_MAX_DURATION_SECONDS_VALUE UINT64_C(86400) #define CBM_INDEX_MIB_BYTES UINT64_C(1048576) typedef struct { @@ -20,12 +25,16 @@ typedef struct { typedef struct { cbm_index_limit_u64_t max_files; cbm_index_limit_u64_t max_source_bytes; + cbm_index_limit_u64_t max_rss_bytes; + cbm_index_limit_u64_t max_duration_ms; } cbm_index_resource_policy_t; typedef enum { CBM_INDEX_RESOURCE_NONE = 0, CBM_INDEX_RESOURCE_FILES, CBM_INDEX_RESOURCE_SOURCE_BYTES, + CBM_INDEX_RESOURCE_RSS_BYTES, + CBM_INDEX_RESOURCE_DURATION_MS, } cbm_index_resource_t; typedef struct { @@ -36,6 +45,8 @@ typedef struct { void cbm_index_policy_init(cbm_index_resource_policy_t *policy); bool cbm_index_policy_enabled(const cbm_index_resource_policy_t *policy); +bool cbm_index_policy_discovery_enabled(const cbm_index_resource_policy_t *policy); +bool cbm_index_policy_worker_enabled(const cbm_index_resource_policy_t *policy); size_t cbm_index_policy_key_count(void); const char *cbm_index_policy_key_at(size_t index); diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 5e3a549ed..474ada1d2 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -14,6 +14,7 @@ #include "platform.h" /* cbm_now_ms */ #include "sanitized.h" /* CBM_SANITIZED — spawn-retry budget */ +#include #include #include #include @@ -21,15 +22,18 @@ #ifdef _WIN32 #include +#include #include "win_utf8.h" /* cbm_utf8_to_wide — spawn the worker with a wide command line so a * non-ASCII repo path survives CreateProcess (#423/#20) */ #include /* free */ #else +#include #include #include #include #ifdef __APPLE__ #include +#include #include #endif extern char **environ; @@ -456,6 +460,287 @@ struct cbm_subprocess { #endif }; +static void cbm_rss_add_saturated(uint64_t *total, uint64_t value) { + if (value > UINT64_MAX - *total) { + *total = UINT64_MAX; + } else { + *total += value; + } +} + +#ifdef CBM_ENABLE_TEST_SEAMS +uint64_t cbm_subprocess_rss_sum_for_testing(const uint64_t *values, size_t count) { + uint64_t total = 0; + for (size_t index = 0; values && index < count; index++) { + cbm_rss_add_saturated(&total, values[index]); + } + return total; +} +#endif + +#ifdef _WIN32 +static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, + uint64_t *rss_bytes) { + DWORD capacity = 32; + JOBOBJECT_BASIC_PROCESS_ID_LIST *processes = NULL; + for (;;) { + size_t bytes = sizeof(*processes) + (size_t)(capacity - 1) * sizeof(ULONG_PTR); + if (bytes > UINT32_MAX) { + cbm_free(CBM_MEM_CLASS_OTHER, processes); + return CBM_PROC_TREE_RSS_ERROR; + } + JOBOBJECT_BASIC_PROCESS_ID_LIST *grown = cbm_realloc(CBM_MEM_CLASS_OTHER, processes, bytes); + if (!grown) { + cbm_free(CBM_MEM_CLASS_OTHER, processes); + return CBM_PROC_TREE_RSS_ERROR; + } + processes = grown; + ZeroMemory(processes, bytes); + processes->NumberOfAssignedProcesses = capacity; + if (QueryInformationJobObject(process->job, JobObjectBasicProcessIdList, processes, + (DWORD)bytes, NULL)) { + break; + } + if (GetLastError() != ERROR_MORE_DATA || capacity > (UINT32_MAX / 2U)) { + cbm_free(CBM_MEM_CLASS_OTHER, processes); + return CBM_PROC_TREE_RSS_ERROR; + } + capacity *= 2U; + } + + uint64_t total = 0; + DWORD measured = 0; + bool root_failed = false; + for (DWORD index = 0; index < processes->NumberOfProcessIdsInList; index++) { + DWORD pid = (DWORD)processes->ProcessIdList[index]; + HANDLE member = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid); + if (!member) { + root_failed = root_failed || pid == process->process_id; + continue; + } + PROCESS_MEMORY_COUNTERS memory; + ZeroMemory(&memory, sizeof(memory)); + if (GetProcessMemoryInfo(member, &memory, sizeof(memory))) { + cbm_rss_add_saturated(&total, (uint64_t)memory.WorkingSetSize); + measured++; + } else if (pid == process->process_id) { + root_failed = true; + } + CloseHandle(member); + } + DWORD listed = processes->NumberOfProcessIdsInList; + cbm_free(CBM_MEM_CLASS_OTHER, processes); + if (root_failed || (measured == 0 && listed > 0)) { + return CBM_PROC_TREE_RSS_ERROR; + } + if (measured == 0) { + return CBM_PROC_TREE_RSS_EMPTY; + } + *rss_bytes = total; + return CBM_PROC_TREE_RSS_OK; +} +#elif defined(__APPLE__) +static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, + uint64_t *rss_bytes) { + int capacity = proc_listallpids(NULL, 0); + if (capacity <= 0 || capacity > INT_MAX / (int)sizeof(pid_t) - 64) { + return CBM_PROC_TREE_RSS_ERROR; + } + capacity += 64; + pid_t *pids = NULL; + int count = 0; + for (;;) { + pid_t *grown = cbm_realloc(CBM_MEM_CLASS_OTHER, pids, (size_t)capacity * sizeof(*pids)); + if (!grown) { + cbm_free(CBM_MEM_CLASS_OTHER, pids); + return CBM_PROC_TREE_RSS_ERROR; + } + pids = grown; + count = proc_listallpids(pids, capacity * (int)sizeof(*pids)); + if (count <= 0) { + cbm_free(CBM_MEM_CLASS_OTHER, pids); + return CBM_PROC_TREE_RSS_ERROR; + } + if (count < capacity) { + break; + } + if (capacity > INT_MAX / (int)sizeof(*pids) / 2) { + cbm_free(CBM_MEM_CLASS_OTHER, pids); + return CBM_PROC_TREE_RSS_ERROR; + } + capacity *= 2; + } + + uint64_t total = 0; + int measured = 0; + bool root_failed = false; + for (int index = 0; index < count; index++) { + struct proc_bsdinfo info; + if (pids[index] <= 0) { + continue; + } + if (proc_pidinfo(pids[index], PROC_PIDTBSDINFO, 0, &info, sizeof(info)) != + (int)sizeof(info)) { + root_failed = root_failed || pids[index] == process->pid; + continue; + } + if ((pid_t)info.pbi_pgid != process->pgid) { + continue; + } + struct rusage_info_v2 usage; + if (proc_pid_rusage(pids[index], RUSAGE_INFO_V2, (rusage_info_t *)&usage) != 0) { + root_failed = root_failed || pids[index] == process->pid; + continue; + } + cbm_rss_add_saturated(&total, usage.ri_resident_size); + measured++; + } + cbm_free(CBM_MEM_CLASS_OTHER, pids); + if (root_failed) { + return CBM_PROC_TREE_RSS_ERROR; + } + if (measured == 0) { + return CBM_PROC_TREE_RSS_EMPTY; + } + *rss_bytes = total; + return CBM_PROC_TREE_RSS_OK; +} +#else +static bool cbm_proc_pid_name(const char *name) { + if (!name || !name[0]) { + return false; + } + for (const unsigned char *cursor = (const unsigned char *)name; *cursor; cursor++) { + if (*cursor < '0' || *cursor > '9') { + return false; + } + } + return true; +} + +static bool cbm_linux_proc_stat(const char *pid_name, pid_t *group, int64_t *rss_pages) { + char path[64]; + int length = snprintf(path, sizeof(path), "/proc/%s/stat", pid_name); + if (length <= 0 || length >= (int)sizeof(path)) { + return false; + } + FILE *stat_file = cbm_fopen(path, "rb"); + if (!stat_file) { + return false; + } + char stat_line[4096]; + bool read = fgets(stat_line, sizeof(stat_line), stat_file) != NULL; + (void)fclose(stat_file); + char *command_end = read ? strrchr(stat_line, ')') : NULL; + if (!command_end || command_end[1] != ' ') { + return false; + } + + char *save = NULL; + char *token = strtok_r(command_end + 2, " ", &save); + int field = 3; + bool have_group = false; + bool have_rss = false; + while (token) { + if (field == 5 || field == 24) { + char *end = NULL; + long long value = strtoll(token, &end, 10); + if (!end || *end != '\0') { + return false; + } + if (field == 5) { + *group = (pid_t)value; + have_group = true; + } else { + *rss_pages = (int64_t)value; + have_rss = true; + break; + } + } + token = strtok_r(NULL, " ", &save); + field++; + } + return have_group && have_rss; +} + +static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, + uint64_t *rss_bytes) { + DIR *proc = opendir("/proc"); + if (!proc) { + return CBM_PROC_TREE_RSS_ERROR; + } + long page_size = sysconf(_SC_PAGESIZE); + if (page_size <= 0) { + (void)closedir(proc); + return CBM_PROC_TREE_RSS_ERROR; + } + + uint64_t total = 0; + int measured = 0; + bool root_failed = false; + struct dirent *entry; + while ((entry = readdir(proc)) != NULL) { + if (!cbm_proc_pid_name(entry->d_name)) { + continue; + } + pid_t group = 0; + int64_t pages = 0; + bool root_entry = strtol(entry->d_name, NULL, 10) == (long)process->pid; + if (!cbm_linux_proc_stat(entry->d_name, &group, &pages)) { + root_failed = root_failed || root_entry; + continue; + } + if (group != process->pgid || pages < 0) { + continue; + } + uint64_t page_count = (uint64_t)pages; + uint64_t bytes = page_count > UINT64_MAX / (uint64_t)page_size + ? UINT64_MAX + : page_count * (uint64_t)page_size; + cbm_rss_add_saturated(&total, bytes); + measured++; + } + (void)closedir(proc); + if (root_failed) { + return CBM_PROC_TREE_RSS_ERROR; + } + if (measured == 0) { + return CBM_PROC_TREE_RSS_EMPTY; + } + *rss_bytes = total; + return CBM_PROC_TREE_RSS_OK; +} +#endif + +cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_bytes(cbm_subprocess_t *process, + uint64_t *rss_bytes) { + if (!process || !rss_bytes) { + return CBM_PROC_TREE_RSS_ERROR; + } + *rss_bytes = 0; + return cbm_subprocess_tree_rss_platform(process, rss_bytes); +} + +bool cbm_subprocess_root_running(const cbm_subprocess_t *process) { + if (!process || process->root_reaped) { + return false; + } + int lifecycle = atomic_load_explicit(&process->lifecycle, memory_order_acquire); + return lifecycle == CBM_SUBPROCESS_ACTIVE || lifecycle == CBM_SUBPROCESS_CANCEL_REQUESTED; +} + +bool cbm_subprocess_termination_pending(const cbm_subprocess_t *process) { + return process && process->termination_started; +} + +bool cbm_subprocess_supervision_active(const cbm_subprocess_t *process) { + if (!process) { + return false; + } + int lifecycle = atomic_load_explicit(&process->lifecycle, memory_order_acquire); + return lifecycle == CBM_SUBPROCESS_ACTIVE || lifecycle == CBM_SUBPROCESS_CANCEL_REQUESTED; +} + static void cbm_subprocess_result_init(cbm_proc_result_t *result) { result->outcome = CBM_PROC_SPAWN_FAILED; result->exit_code = -1; diff --git a/src/foundation/subprocess.h b/src/foundation/subprocess.h index 4dc8f1870..bfcb2311b 100644 --- a/src/foundation/subprocess.h +++ b/src/foundation/subprocess.h @@ -22,6 +22,7 @@ #include #include /* size_t (cbm_build_win_cmdline) */ +#include /* How a supervised child ended. */ typedef enum { @@ -118,6 +119,12 @@ typedef enum { CBM_PROC_POLL_TERMINAL = 1 } cbm_proc_poll_t; +typedef enum { + CBM_PROC_TREE_RSS_ERROR = -1, + CBM_PROC_TREE_RSS_EMPTY = 0, + CBM_PROC_TREE_RSS_OK = 1, +} cbm_proc_tree_rss_status_t; + /* Spawn opts->bin and return immediately with a supervisor handle. On success, * *out owns the process until a terminal poll followed by destroy. Spawn copies * the option strings/argv it needs after return; log_ud remains caller-owned until @@ -148,6 +155,16 @@ int cbm_subprocess_spawn(const cbm_proc_opts_t *opts, cbm_subprocess_t **out); * tree when cancel_grace_ms elapses. Callers must keep polling to make progress. */ cbm_proc_poll_t cbm_subprocess_poll(cbm_subprocess_t *process, cbm_proc_result_t *out); +/* Read the current resident-set size of the complete contained process tree. + * OK returns an overflow-safe byte total, EMPTY means the owned tree currently + * has no observable members, and ERROR means no trustworthy measurement could + * be obtained. Individual processes that exit during enumeration are ignored. */ +cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_bytes(cbm_subprocess_t *process, + uint64_t *rss_bytes); +bool cbm_subprocess_root_running(const cbm_subprocess_t *process); +bool cbm_subprocess_termination_pending(const cbm_subprocess_t *process); +bool cbm_subprocess_supervision_active(const cbm_subprocess_t *process); + /* Record an explicit cancellation request without waiting. Safe to repeat and * safe to call from a cancellation thread while one owner thread polls. The * owner must stop cancellation producers before destroying the handle. true @@ -223,6 +240,7 @@ void cbm_subprocess_hold_parent_until_child_exits_for_testing(int spawns); * shows for a child that is EXITING: setpgid and getpgid answer ESRCH while * waitid does not report it yet. Test builds only. */ void cbm_subprocess_observe_exiting_child_for_testing(int spawns); +uint64_t cbm_subprocess_rss_sum_for_testing(const uint64_t *values, size_t count); #endif /* How the POSIX fork+exec child closed its inherited descriptors (#1484). */ diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index 223945d3c..4ffeecbb8 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -481,8 +481,56 @@ enum { INDEX_WORKER_SYNC_POLL_NS = 10000000, INDEX_WORKER_RELAY_LINES_PER_POLL = 64, INDEX_WORKER_RELAY_BYTES_PER_POLL = 64 * 1024, + INDEX_WORKER_RSS_PROBE_FAILURE_LIMIT = 3, + INDEX_WORKER_RSS_PROBE_INTERVAL_MS = 250, }; +typedef enum { + INDEX_WORKER_TERMINATION_NONE = 0, + INDEX_WORKER_TERMINATION_CANCEL_PENDING, + INDEX_WORKER_TERMINATION_CANCEL, + INDEX_WORKER_TERMINATION_RESOURCE, +} index_worker_termination_reason_t; + +#ifdef CBM_ENABLE_TEST_SEAMS +static cbm_index_supervisor_clock_fn g_resource_clock; +static cbm_index_supervisor_rss_fn g_resource_rss; +static void *g_resource_hook_context; + +void cbm_index_supervisor_set_resource_hooks_for_testing(cbm_index_supervisor_clock_fn clock_fn, + cbm_index_supervisor_rss_fn rss_fn, + void *context) { + g_resource_clock = clock_fn; + g_resource_rss = rss_fn; + g_resource_hook_context = context; +} + +void cbm_index_supervisor_reset_resource_hooks_for_testing(void) { + g_resource_clock = NULL; + g_resource_rss = NULL; + g_resource_hook_context = NULL; +} +#endif + +static uint64_t worker_resource_now_ms(void) { +#ifdef CBM_ENABLE_TEST_SEAMS + if (g_resource_clock) { + return g_resource_clock(g_resource_hook_context); + } +#endif + return cbm_now_ms(); +} + +static cbm_proc_tree_rss_status_t worker_resource_rss(cbm_subprocess_t *process, + uint64_t *rss_bytes) { +#ifdef CBM_ENABLE_TEST_SEAMS + if (g_resource_rss) { + return g_resource_rss(process, rss_bytes, g_resource_hook_context); + } +#endif + return cbm_subprocess_tree_rss_bytes(process, rss_bytes); +} + struct cbm_index_worker_handle { cbm_subprocess_t *process; char response_path[INDEX_WORKER_PATH_CAP]; @@ -492,6 +540,12 @@ struct cbm_index_worker_handle { long relay_tail_pos; bool process_terminal; cbm_proc_result_t process_result; + cbm_index_resource_policy_t resource_policy; + uint64_t started_ms; + uint64_t last_rss_probe_ms; + unsigned int rss_probe_failures; + bool rss_probe_started; + atomic_int termination_reason; atomic_bool terminal; cbm_index_worker_result_t result; }; @@ -670,7 +724,8 @@ static bool worker_unique_file(char *out, size_t out_size, const char *kind) { static bool worker_result_succeeded(const cbm_index_worker_result_t *result) { return result && result->outcome == CBM_PROC_CLEAN && !result->cancellation_requested && - result->tree_quiesced && !result->supervision_failed; + result->resource_violation.resource == CBM_INDEX_RESOURCE_NONE && + !result->resource_probe_failed && result->tree_quiesced && !result->supervision_failed; } static void worker_terminal_log(cbm_index_worker_handle_t *handle) { @@ -701,6 +756,13 @@ static void worker_terminal_log(cbm_index_worker_handle_t *handle) { } else if (handle->result.supervision_failed || !handle->result.tree_quiesced) { cbm_log_error("index.supervisor.containment_failed", "outcome", cbm_proc_outcome_str(handle->result.outcome), "log", handle->log_path); + } else if (handle->result.resource_probe_failed) { + cbm_log_error("index.supervisor.resource_probe_failed", "resource", "rss_bytes", "log", + handle->log_path); + } else if (handle->result.resource_violation.resource != CBM_INDEX_RESOURCE_NONE) { + cbm_log_warn("index.supervisor.resource_limit", "resource", + cbm_index_resource_name(handle->result.resource_violation.resource), "log", + handle->log_path); } else if (handle->result.cancellation_requested) { cbm_log_warn("index.supervisor.worker_cancelled", "outcome", cbm_proc_outcome_str(handle->result.outcome), "log", handle->log_path); @@ -739,10 +801,87 @@ size_t cbm_index_worker_job_memory_limit(size_t memory_budget_bytes) { return memory_budget_bytes > SIZE_MAX - headroom ? SIZE_MAX : memory_budget_bytes + headroom; } -int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_bytes, - bool single_thread, const char *marker_file, - const char *quarantine_file, cbm_proc_log_cb log_callback, - void *log_context, cbm_index_worker_handle_t **handle_out) { +static void worker_request_resource_termination(cbm_index_worker_handle_t *handle, + cbm_index_resource_t resource, uint64_t observed, + uint64_t limit, bool probe_failed) { + int expected = INDEX_WORKER_TERMINATION_NONE; + if (!atomic_compare_exchange_strong_explicit(&handle->termination_reason, &expected, + INDEX_WORKER_TERMINATION_RESOURCE, + memory_order_acq_rel, memory_order_acquire)) { + return; + } + handle->result.resource_violation = (cbm_index_resource_violation_t){ + .resource = resource, .observed = observed, .limit = limit}; + handle->result.resource_probe_failed = probe_failed; + if (!cbm_subprocess_request_cancel(handle->process)) { + handle->result.resource_violation = (cbm_index_resource_violation_t){0}; + handle->result.resource_probe_failed = false; + expected = INDEX_WORKER_TERMINATION_RESOURCE; + (void)atomic_compare_exchange_strong_explicit(&handle->termination_reason, &expected, + INDEX_WORKER_TERMINATION_NONE, + memory_order_acq_rel, memory_order_acquire); + } +} + +static bool worker_check_resource_limits(cbm_index_worker_handle_t *handle) { + if (atomic_load_explicit(&handle->termination_reason, memory_order_acquire) != + INDEX_WORKER_TERMINATION_NONE || + cbm_subprocess_termination_pending(handle->process) || + !cbm_subprocess_supervision_active(handle->process)) { + return false; + } + uint64_t now = 0; + bool have_now = false; + bool rss_probe_failed = false; + if (handle->resource_policy.max_rss_bytes.enabled) { + now = worker_resource_now_ms(); + have_now = true; + bool probe_due = !handle->rss_probe_started || now < handle->last_rss_probe_ms || + now - handle->last_rss_probe_ms >= INDEX_WORKER_RSS_PROBE_INTERVAL_MS; + if (!probe_due) { + goto duration_check; + } + handle->rss_probe_started = true; + handle->last_rss_probe_ms = now; + uint64_t rss_bytes = 0; + cbm_proc_tree_rss_status_t status = worker_resource_rss(handle->process, &rss_bytes); + if (status == CBM_PROC_TREE_RSS_OK) { + handle->rss_probe_failures = 0; + if (rss_bytes > handle->resource_policy.max_rss_bytes.value) { + worker_request_resource_termination(handle, CBM_INDEX_RESOURCE_RSS_BYTES, rss_bytes, + handle->resource_policy.max_rss_bytes.value, + false); + return false; + } + } else if (status == CBM_PROC_TREE_RSS_ERROR) { + handle->rss_probe_failures++; + if (handle->rss_probe_failures >= INDEX_WORKER_RSS_PROBE_FAILURE_LIMIT) { + rss_probe_failed = true; + } + } else { + handle->rss_probe_failures = 0; + } + } +duration_check: + if (handle->resource_policy.max_duration_ms.enabled) { + if (!have_now) { + now = worker_resource_now_ms(); + } + uint64_t elapsed = now >= handle->started_ms ? now - handle->started_ms : 0; + if (elapsed > handle->resource_policy.max_duration_ms.value) { + worker_request_resource_termination(handle, CBM_INDEX_RESOURCE_DURATION_MS, elapsed, + handle->resource_policy.max_duration_ms.value, + false); + } + } + return rss_probe_failed; +} + +static int worker_start_internal(const char *args_json, size_t memory_budget_bytes, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, cbm_proc_log_cb log_callback, + void *log_context, cbm_index_worker_handle_t **handle_out) { if (handle_out) { *handle_out = NULL; } @@ -774,6 +913,11 @@ int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_ if (!handle) { return -1; } + cbm_index_policy_init(&handle->resource_policy); + if (resource_policy) { + handle->resource_policy = *resource_policy; + } + atomic_init(&handle->termination_reason, INDEX_WORKER_TERMINATION_NONE); atomic_init(&handle->terminal, false); handle->log_callback = log_callback; handle->log_context = log_context; @@ -840,15 +984,46 @@ int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_ cbm_log_error("index.supervisor.spawn_failed", "action", "fail_closed"); return -1; } + if (handle->resource_policy.max_duration_ms.enabled) { + handle->started_ms = worker_resource_now_ms(); + } *handle_out = handle; return 0; } +int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_bytes, + bool single_thread, const char *marker_file, + const char *quarantine_file, cbm_proc_log_cb log_callback, + void *log_context, cbm_index_worker_handle_t **handle_out) { + return worker_start_internal(args_json, memory_budget_bytes, NULL, single_thread, marker_file, + quarantine_file, log_callback, log_context, handle_out); +} + int cbm_index_worker_start(const char *args_json, size_t memory_budget_bytes, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_index_worker_handle_t **handle_out) { - return cbm_index_worker_start_with_log(args_json, memory_budget_bytes, single_thread, - marker_file, quarantine_file, NULL, NULL, handle_out); + return worker_start_internal(args_json, memory_budget_bytes, NULL, single_thread, marker_file, + quarantine_file, NULL, NULL, handle_out); +} + +int cbm_index_worker_start_with_policy(const char *args_json, size_t memory_budget_bytes, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, + cbm_index_worker_handle_t **handle_out) { + return worker_start_internal(args_json, memory_budget_bytes, resource_policy, single_thread, + marker_file, quarantine_file, NULL, NULL, handle_out); +} + +int cbm_index_worker_start_with_policy_log(const char *args_json, size_t memory_budget_bytes, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, + cbm_proc_log_cb log_callback, void *log_context, + cbm_index_worker_handle_t **handle_out) { + return worker_start_internal(args_json, memory_budget_bytes, resource_policy, single_thread, + marker_file, quarantine_file, log_callback, log_context, + handle_out); } cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, @@ -865,10 +1040,16 @@ cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, } bool relay_caught_up = true; if (!handle->process_terminal) { + bool rss_probe_failed = worker_check_resource_limits(handle); cbm_proc_result_t process_result; cbm_proc_poll_t state = cbm_subprocess_poll(handle->process, &process_result); relay_caught_up = worker_relay_log(handle); if (state == CBM_PROC_POLL_RUNNING) { + if (rss_probe_failed && cbm_subprocess_root_running(handle->process)) { + worker_request_resource_termination(handle, CBM_INDEX_RESOURCE_RSS_BYTES, 0, + handle->resource_policy.max_rss_bytes.value, + true); + } return CBM_INDEX_WORKER_POLL_RUNNING; } if (state != CBM_PROC_POLL_TERMINAL) { @@ -889,7 +1070,12 @@ cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, handle->result.outcome = process_result->outcome; handle->result.exit_code = process_result->exit_code; handle->result.term_signal = process_result->term_signal; - handle->result.cancellation_requested = process_result->cancellation_requested; + int termination_reason = + atomic_load_explicit(&handle->termination_reason, memory_order_acquire); + handle->result.cancellation_requested = + termination_reason == INDEX_WORKER_TERMINATION_CANCEL || + (termination_reason == INDEX_WORKER_TERMINATION_CANCEL_PENDING && + process_result->cancellation_requested); handle->result.forced = process_result->forced; handle->result.tree_quiesced = process_result->tree_quiesced; handle->result.supervision_failed = process_result->supervision_failed; @@ -922,8 +1108,36 @@ cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, } bool cbm_index_worker_request_cancel(cbm_index_worker_handle_t *handle) { - return handle && !atomic_load_explicit(&handle->terminal, memory_order_acquire) && - cbm_subprocess_request_cancel(handle->process); + if (!handle || atomic_load_explicit(&handle->terminal, memory_order_acquire)) { + return false; + } + int expected = INDEX_WORKER_TERMINATION_NONE; + bool claimed = atomic_compare_exchange_strong_explicit( + &handle->termination_reason, &expected, INDEX_WORKER_TERMINATION_CANCEL_PENDING, + memory_order_acq_rel, memory_order_acquire); + if (!claimed) { + if (expected == INDEX_WORKER_TERMINATION_CANCEL) { + return true; + } + if (expected != INDEX_WORKER_TERMINATION_CANCEL_PENDING) { + return false; + } + bool accepted = cbm_subprocess_request_cancel(handle->process); + if (accepted) { + expected = INDEX_WORKER_TERMINATION_CANCEL_PENDING; + (void)atomic_compare_exchange_strong_explicit( + &handle->termination_reason, &expected, INDEX_WORKER_TERMINATION_CANCEL, + memory_order_acq_rel, memory_order_acquire); + } + return accepted; + } + bool accepted = cbm_subprocess_request_cancel(handle->process); + expected = INDEX_WORKER_TERMINATION_CANCEL_PENDING; + (void)atomic_compare_exchange_strong_explicit(&handle->termination_reason, &expected, + accepted ? INDEX_WORKER_TERMINATION_CANCEL + : INDEX_WORKER_TERMINATION_NONE, + memory_order_acq_rel, memory_order_acquire); + return accepted; } const char *cbm_index_worker_response_path(const cbm_index_worker_handle_t *handle) { @@ -948,18 +1162,19 @@ void cbm_index_worker_destroy(cbm_index_worker_handle_t *handle) { free(handle); } -int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_thread, - const char *marker_file, const char *quarantine_file, - cbm_proc_log_cb log_callback, void *log_context, - const atomic_int *cancel_requested, - cbm_index_worker_result_t *result) { +static int worker_spawn_internal(const char *args_json, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, cbm_proc_log_cb log_callback, + void *log_context, const atomic_int *cancel_requested, + cbm_index_worker_result_t *result) { if (!result) { return -1; } worker_result_init(result); cbm_index_worker_handle_t *handle = NULL; - if (cbm_index_worker_start_with_log(args_json, 0, single_thread, marker_file, quarantine_file, - log_callback, log_context, &handle) != 0) { + if (worker_start_internal(args_json, 0, resource_policy, single_thread, marker_file, + quarantine_file, log_callback, log_context, &handle) != 0) { return -1; } const cbm_index_worker_result_t *cached = NULL; @@ -985,6 +1200,24 @@ int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_th return 0; } +int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_thread, + const char *marker_file, const char *quarantine_file, + cbm_proc_log_cb log_callback, void *log_context, + const atomic_int *cancel_requested, + cbm_index_worker_result_t *result) { + return worker_spawn_internal(args_json, NULL, single_thread, marker_file, quarantine_file, + log_callback, log_context, cancel_requested, result); +} + +int cbm_index_spawn_worker_with_policy_log_cancel( + const char *args_json, const cbm_index_resource_policy_t *resource_policy, bool single_thread, + const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, + void *log_context, const atomic_int *cancel_requested, cbm_index_worker_result_t *result) { + return worker_spawn_internal(args_json, resource_policy, single_thread, marker_file, + quarantine_file, log_callback, log_context, cancel_requested, + result); +} + int cbm_index_spawn_worker_with_log(const char *args_json, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, void *log_context, diff --git a/src/mcp/index_supervisor.h b/src/mcp/index_supervisor.h index c231a563e..a86761d77 100644 --- a/src/mcp/index_supervisor.h +++ b/src/mcp/index_supervisor.h @@ -22,8 +22,10 @@ #include #include +#include #include +#include "foundation/index_policy.h" #include "foundation/subprocess.h" /* cbm_proc_outcome_t */ /* Worker-role state, set once from the CLI arg parser (main.c) when this process @@ -156,8 +158,10 @@ typedef struct { char last_phase[128]; char worker_log[4096]; bool response_rejected; /* clean worker exceeded the bounded response protocol */ - char *response; /* worker result only after a contained, uncancelled CLEAN exit; - * borrowed for async polls, caller-owned from the sync wrapper */ + cbm_index_resource_violation_t resource_violation; + bool resource_probe_failed; + char *response; /* worker result only after a contained, uncancelled CLEAN exit; + * borrowed for async polls, caller-owned from the sync wrapper */ } cbm_index_worker_result_t; /* Daemon-owned, nonblocking supervisor for one contained worker process tree. */ @@ -179,6 +183,21 @@ size_t cbm_index_worker_job_memory_limit(size_t memory_budget_bytes); int cbm_index_worker_start(const char *args_json, size_t memory_budget_bytes, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_index_worker_handle_t **handle_out); +int cbm_index_worker_start_with_policy(const char *args_json, size_t memory_budget_bytes, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, + cbm_index_worker_handle_t **handle_out); + +/* Policy plus the request-scoped log callback. The daemon default start path + * needs both: a missing policy fails closed, and worker diagnostics still + * reach the daemon journal. */ +int cbm_index_worker_start_with_policy_log(const char *args_json, size_t memory_budget_bytes, + const cbm_index_resource_policy_t *resource_policy, + bool single_thread, const char *marker_file, + const char *quarantine_file, + cbm_proc_log_cb log_callback, void *log_context, + cbm_index_worker_handle_t **handle_out); /* Request-scoped variant used by interactive local CLI calls. The callback is * invoked by the owner thread while it polls the contained worker; log_context @@ -242,7 +261,22 @@ int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_th cbm_proc_log_cb log_callback, void *log_context, const atomic_int *cancel_requested, cbm_index_worker_result_t *result); +int cbm_index_spawn_worker_with_policy_log_cancel( + const char *args_json, const cbm_index_resource_policy_t *resource_policy, bool single_thread, + const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, + void *log_context, const atomic_int *cancel_requested, cbm_index_worker_result_t *result); void cbm_index_worker_result_free(cbm_index_worker_result_t *result); +#ifdef CBM_ENABLE_TEST_SEAMS +typedef uint64_t (*cbm_index_supervisor_clock_fn)(void *context); +typedef cbm_proc_tree_rss_status_t (*cbm_index_supervisor_rss_fn)(cbm_subprocess_t *process, + uint64_t *rss_bytes, + void *context); +void cbm_index_supervisor_set_resource_hooks_for_testing(cbm_index_supervisor_clock_fn clock_fn, + cbm_index_supervisor_rss_fn rss_fn, + void *context); +void cbm_index_supervisor_reset_resource_hooks_for_testing(void); +#endif + #endif /* CBM_INDEX_SUPERVISOR_H */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 1d1489e7f..1016f781a 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11205,6 +11205,22 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * return degraded; } +static bool project_db_is_servable(const char *project, const char *db_path); + +/* Serialize a worker JSON document and release the heap strings it borrowed. + * Extra arguments may be NULL; free(NULL) is a no-op. */ +static char *index_worker_json_result(yyjson_mut_doc *doc, char *repo_path, char *name_override, + char *project_name) { + char *json = yy_doc_to_str(doc); + yyjson_mut_doc_free(doc); + free(repo_path); + free(name_override); + free(project_name); + char *result = cbm_mcp_text_result(json, true); + free(json); + return result; +} + /* 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 @@ -11237,12 +11253,7 @@ static char *build_worker_failure_response(const char *args, cbm_proc_outcome_t 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; + return index_worker_json_result(doc, repo_path, NULL, NULL); } static char *build_worker_unsafe_terminal_response(const char *args, cbm_proc_outcome_t outcome, @@ -11263,12 +11274,50 @@ static char *build_worker_unsafe_terminal_response(const char *args, cbm_proc_ou 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 *response = cbm_mcp_text_result(json, true); - free(json); - return response; + return index_worker_json_result(doc, repo_path, NULL, NULL); +} + +char *cbm_mcp_index_worker_resource_response(const char *args, + const cbm_index_worker_result_t *worker_result) { + char *repo_path = cbm_mcp_get_string_arg(args, "repo_path"); + char *name_override = cbm_mcp_get_string_arg(args, "name"); + char *project_name = + cbm_project_name_from_path(name_override && name_override[0] ? name_override : repo_path); + char db_path[CBM_SZ_1K] = {0}; + if (project_name) { + project_db_path(project_name, db_path, sizeof(db_path)); + } + + const cbm_index_resource_violation_t *violation = &worker_result->resource_violation; + const char *config_key = cbm_index_resource_config_key(violation->resource); + char message[CBM_SZ_256]; + (void)snprintf(message, sizeof(message), + worker_result->resource_probe_failed + ? "Worker resource measurement failed for %s" + : "Index worker exceeded %s", + config_key); + 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, "code", + worker_result->resource_probe_failed ? "resource_probe_failed" + : "resource_limit_exceeded"); + yyjson_mut_obj_add_str(doc, root, "stage", "worker"); + yyjson_mut_obj_add_str(doc, root, "resource", cbm_index_resource_name(violation->resource)); + if (!worker_result->resource_probe_failed) { + yyjson_mut_obj_add_uint(doc, root, "observed", violation->observed); + yyjson_mut_obj_add_uint(doc, root, "limit", violation->limit); + yyjson_mut_obj_add_str(doc, root, "unit", cbm_index_resource_unit(violation->resource)); + } + yyjson_mut_obj_add_bool(doc, root, "retryable", true); + yyjson_mut_obj_add_bool(doc, root, "serving_index_preserved", + project_name && project_db_is_servable(project_name, db_path)); + yyjson_mut_obj_add_strcpy(doc, root, "message", message); + if (repo_path) { + yyjson_mut_obj_add_strcpy(doc, root, "repo_path", repo_path); + } + return index_worker_json_result(doc, repo_path, name_override, project_name); } /* Drop the cached store so the next query reopens whatever the worker wrote (each @@ -11414,6 +11463,9 @@ cbm_mcp_supervised_result_disposition_t cbm_mcp_supervised_result_disposition( worker_result->supervision_failed) { return CBM_MCP_SUPERVISED_RESULT_UNSAFE_TERMINAL; } + if (worker_result->resource_violation.resource != CBM_INDEX_RESOURCE_NONE) { + return CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE; + } if (worker_result->outcome == CBM_PROC_CLEAN) { return worker_result->response ? CBM_MCP_SUPERVISED_RESULT_SUCCESS : CBM_MCP_SUPERVISED_RESULT_FALLBACK; @@ -11421,8 +11473,8 @@ cbm_mcp_supervised_result_disposition_t cbm_mcp_supervised_result_disposition( return CBM_MCP_SUPERVISED_RESULT_CONTAINED_FAILURE; } -static bool index_policy_from_worker_args(const char *args, cbm_index_resource_policy_t *policy, - char *error, size_t error_size) { +bool cbm_mcp_index_policy_from_internal_args(const char *args, cbm_index_resource_policy_t *policy, + char *error, size_t error_size) { yyjson_doc *doc = args ? yyjson_read(args, strlen(args), 0) : NULL; yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; yyjson_val *encoded = @@ -11452,7 +11504,7 @@ static bool index_policy_from_worker_args(const char *args, cbm_index_resource_p static bool load_index_policy(cbm_mcp_server_t *srv, const char *args, cbm_index_resource_policy_t *policy, char *error, size_t error_size) { if (cbm_index_worker_active()) { - return index_policy_from_worker_args(args, policy, error, error_size); + return cbm_mcp_index_policy_from_internal_args(args, policy, error, error_size); } cbm_config_t *owned_config = NULL; cbm_config_t *config = srv ? srv->config : NULL; @@ -11562,13 +11614,14 @@ char *cbm_mcp_index_worker_no_response_failure(const char *args, return cbm_mcp_text_result(written ? json : hint, true); } -static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { +static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, + const cbm_index_resource_policy_t *resource_policy) { invalidate_cached_store(srv); /* First attempt: normal parallel run. */ cbm_index_worker_result_t wr; - int rc = cbm_index_spawn_worker_with_log_cancel( - args, false, NULL, NULL, srv ? srv->index_log_callback : NULL, + int rc = cbm_index_spawn_worker_with_policy_log_cancel( + args, resource_policy, false, NULL, NULL, srv ? srv->index_log_callback : NULL, srv ? srv->index_log_context : NULL, srv ? &srv->pipeline_cancel_requested : NULL, &wr); cbm_mcp_supervised_result_disposition_t disposition = cbm_mcp_supervised_result_disposition(rc, &wr); @@ -11593,6 +11646,12 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { invalidate_cached_store(srv); return failure; } + if (disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { + char *failure = cbm_mcp_index_worker_resource_response(args, &wr); + cbm_index_worker_result_free(&wr); + invalidate_cached_store(srv); + return failure; + } if (disposition == CBM_MCP_SUPERVISED_RESULT_SUCCESS) { /* Clean exit → transfer the worker's response (the common path). */ char *resp = wr.response; /* transfer ownership to caller (may be NULL) */ @@ -11642,8 +11701,8 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { bool terminal_cancelled = false; for (int i = 0; i < cap; i++) { cbm_index_worker_result_t wr2; - int rc2 = cbm_index_spawn_worker_with_log_cancel( - args, /*single_thread=*/false, marker_path, quarantine_path, + int rc2 = cbm_index_spawn_worker_with_policy_log_cancel( + args, resource_policy, /*single_thread=*/false, marker_path, quarantine_path, srv ? srv->index_log_callback : NULL, srv ? srv->index_log_context : NULL, srv ? &srv->pipeline_cancel_requested : NULL, &wr2); cbm_mcp_supervised_result_disposition_t recovery_disposition = @@ -11670,6 +11729,11 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { cbm_index_worker_result_free(&wr2); break; } + if (recovery_disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { + resp = cbm_mcp_index_worker_resource_response(args, &wr2); + cbm_index_worker_result_free(&wr2); + break; + } if (recovery_disposition == CBM_MCP_SUPERVISED_RESULT_SUCCESS) { resp = wr2.response; /* transfer ownership to caller */ wr2.response = NULL; @@ -11764,8 +11828,8 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { * so it cannot itself hang. Rare given monotonic progress. */ if (!resp && !unsafe_terminal && quarantined > 0) { cbm_index_worker_result_t wrp; - int rcp = cbm_index_spawn_worker_with_log_cancel( - args, /*single_thread=*/false, NULL, quarantine_path, + int rcp = cbm_index_spawn_worker_with_policy_log_cancel( + args, resource_policy, /*single_thread=*/false, NULL, quarantine_path, srv ? srv->index_log_callback : NULL, srv ? srv->index_log_context : NULL, srv ? &srv->pipeline_cancel_requested : NULL, &wrp); cbm_mcp_supervised_result_disposition_t partial_disposition = @@ -11781,6 +11845,8 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) { last_outcome = wrp.outcome; unsafe_terminal = true; terminal_cancelled = wrp.cancellation_requested; + } else if (partial_disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { + resp = cbm_mcp_index_worker_resource_response(args, &wrp); } cbm_index_worker_result_free(&wrp); } @@ -11827,7 +11893,7 @@ static char *index_run_supervised_path(cbm_mcp_server_t *srv, const char *root_p if (!args) { return NULL; } - char *resp = index_run_supervised(srv, args); + char *resp = index_run_supervised(srv, args, &policy); free(args); return resp; } @@ -12366,7 +12432,7 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { index_args_free(repo_path, mode_str, name_override); return cbm_mcp_text_result("failed to prepare supervised index request", true); } - char *supervised = index_run_supervised(srv, worker_args); + char *supervised = index_run_supervised(srv, worker_args, &resource_policy); free(worker_args); if (supervised) { free(mutation_project); diff --git a/src/mcp/mcp.h b/src/mcp/mcp.h index 3a5fce5ff..a7284672b 100644 --- a/src/mcp/mcp.h +++ b/src/mcp/mcp.h @@ -275,6 +275,7 @@ char *cbm_mcp_index_worker_no_response_failure(const char *args, typedef enum { CBM_MCP_SUPERVISED_RESULT_FALLBACK = 0, CBM_MCP_SUPERVISED_RESULT_SUCCESS, + CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE, CBM_MCP_SUPERVISED_RESULT_CONTAINED_FAILURE, CBM_MCP_SUPERVISED_RESULT_UNSAFE_TERMINAL, } cbm_mcp_supervised_result_disposition_t; diff --git a/src/mcp/mcp_internal.h b/src/mcp/mcp_internal.h index 7035d80ba..3ca7c6ee9 100644 --- a/src/mcp/mcp_internal.h +++ b/src/mcp/mcp_internal.h @@ -60,6 +60,10 @@ bool cbm_mcp_jsonrpc_response_prepend_notice(char **response_io, const char *not * must remove any untrusted field with the same name before invoking this. */ bool cbm_mcp_index_policy_add_to_args(yyjson_mut_doc *doc, yyjson_mut_val *root, const cbm_index_resource_policy_t *policy); +bool cbm_mcp_index_policy_from_internal_args(const char *args, cbm_index_resource_policy_t *policy, + char *error, size_t error_size); +char *cbm_mcp_index_worker_resource_response(const char *args, + const cbm_index_worker_result_t *worker_result); /* Encode a doc_candidates setting the user set to a boolean on an internal * worker request (the worker reads no config). Unset or unparseable adds no diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index 92e23de64..e14bfb7f7 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -1195,7 +1195,8 @@ const char *cbm_pipeline_repo_path(const cbm_pipeline_t *p) { } const cbm_index_resource_policy_t *cbm_pipeline_resource_policy(const cbm_pipeline_t *p) { - return p && cbm_index_policy_enabled(&p->resource_policy) ? &p->resource_policy : NULL; + return p && cbm_index_policy_discovery_enabled(&p->resource_policy) ? &p->resource_policy + : NULL; } cbm_index_resource_violation_t *cbm_pipeline_resource_violation(cbm_pipeline_t *p) { @@ -3941,7 +3942,7 @@ static int cbm_pipeline_run_staged(cbm_pipeline_t *p) { .ignore_file = NULL, .max_file_size = 0, .resource_policy = - cbm_index_policy_enabled(&p->resource_policy) ? &p->resource_policy : NULL, + cbm_index_policy_discovery_enabled(&p->resource_policy) ? &p->resource_policy : NULL, .resource_violation = &p->resource_violation, }; cbm_file_info_t *files = NULL; diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index a923043ca..5fe4e618f 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -1650,6 +1650,8 @@ typedef struct { cbm_project_lock_manager_t *project_locks; bool project_lock_busy_is_error; cbm_proc_outcome_t outcomes[APP_FAKE_MAX_ATTEMPTS]; + cbm_index_resource_violation_t resource_violations[APP_FAKE_MAX_ATTEMPTS]; + bool resource_probe_failures[APP_FAKE_MAX_ATTEMPTS]; const char *responses[APP_FAKE_MAX_ATTEMPTS]; const char *marker_payloads[APP_FAKE_MAX_ATTEMPTS]; char marker_paths[APP_FAKE_MAX_ATTEMPTS][APP_TEST_PATH_CAP]; @@ -1754,7 +1756,9 @@ static int app_fake_worker_start(void *opaque, const char *args_json, size_t mem static bool app_wait_for_atomic_bool(atomic_bool *value, bool expected); static bool app_fake_worker_policy_equals(app_fake_worker_context_t *context, int attempt, - const char *max_files, const char *max_source_mb) { + const char *max_files, const char *max_source_mb, + const char *max_rss_mb, + const char *max_duration_seconds) { if (!context || attempt < 0 || attempt >= APP_FAKE_MAX_ATTEMPTS) { return false; } @@ -1771,9 +1775,18 @@ static bool app_fake_worker_policy_equals(app_fake_worker_context_t *context, in yyjson_val *bytes = policy && yyjson_is_obj(policy) ? yyjson_obj_get(policy, CBM_INDEX_CONFIG_MAX_SOURCE_MB) : NULL; - bool equal = files && bytes && yyjson_is_str(files) && yyjson_is_str(bytes) && + yyjson_val *rss = policy && yyjson_is_obj(policy) + ? yyjson_obj_get(policy, CBM_INDEX_CONFIG_MAX_RSS_MB) + : NULL; + yyjson_val *duration = policy && yyjson_is_obj(policy) + ? yyjson_obj_get(policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS) + : NULL; + bool equal = files && bytes && rss && duration && yyjson_is_str(files) && + yyjson_is_str(bytes) && yyjson_is_str(rss) && yyjson_is_str(duration) && strcmp(yyjson_get_str(files), max_files) == 0 && - strcmp(yyjson_get_str(bytes), max_source_mb) == 0; + strcmp(yyjson_get_str(bytes), max_source_mb) == 0 && + strcmp(yyjson_get_str(rss), max_rss_mb) == 0 && + strcmp(yyjson_get_str(duration), max_duration_seconds) == 0; yyjson_doc_free(document); return equal; } @@ -1815,6 +1828,12 @@ static cbm_index_worker_poll_t app_fake_worker_poll(void *opaque, worker->result.outcome = outcome; worker->result.exit_code = outcome == CBM_PROC_CLEAN ? 0 : -1; worker->result.tree_quiesced = true; + if (worker->attempt < APP_FAKE_MAX_ATTEMPTS) { + worker->result.resource_violation = + worker->context->resource_violations[worker->attempt]; + worker->result.resource_probe_failed = + worker->context->resource_probe_failures[worker->attempt]; + } const char *response = worker->attempt < APP_FAKE_MAX_ATTEMPTS && worker->context->responses[worker->attempt] ? worker->context->responses[worker->attempt] @@ -2385,11 +2404,13 @@ TEST(daemon_application_initialize_coalesces_auto_index_for_full_sessions) { bool dirs_ok = cbm_mkdtemp(root) != NULL && cbm_mkdtemp(cache) != NULL; bool cache_set = dirs_ok && cache_saved && cbm_setenv("CBM_CACHE_DIR", cache, 1) == 0; cbm_config_t *stored_config = cache_set ? cbm_config_open(cache) : NULL; - bool config_ready = stored_config && - cbm_config_set(stored_config, CBM_CONFIG_AUTO_INDEX, "true") == 0 && - cbm_config_set(stored_config, CBM_CONFIG_AUTO_WATCH, "false") == 0 && - cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_FILES, "3") == 0 && - cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_SOURCE_MB, "4") == 0; + bool config_ready = + stored_config && cbm_config_set(stored_config, CBM_CONFIG_AUTO_INDEX, "true") == 0 && + cbm_config_set(stored_config, CBM_CONFIG_AUTO_WATCH, "false") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_FILES, "3") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_SOURCE_MB, "4") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_RSS_MB, "64") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "7") == 0; char canonical_root[APP_TEST_PATH_CAP] = {0}; bool canonical = dirs_ok && cbm_canonical_path(root, canonical_root, sizeof(canonical_root)); char *project = canonical ? cbm_project_name_from_path(canonical_root) : NULL; @@ -2434,7 +2455,8 @@ TEST(daemon_application_initialize_coalesces_auto_index_for_full_sessions) { bool first_owned = first_initialized && project && app_wait_for_subscribers(application, project, 1) && app_wait_for_atomic_int(&fake.starts, 1); - bool auto_policy_propagated = first_owned && app_fake_worker_policy_equals(&fake, 0, "3", "4"); + bool auto_policy_propagated = + first_owned && app_fake_worker_policy_equals(&fake, 0, "3", "4", "64", "7"); bool second_initialized = app_test_initialize_profile(&callbacks, sessions[1], root, CBM_MCP_TOOL_PROFILE_ALL, NULL, NULL); bool coalesced = first_owned && second_initialized && project && @@ -2777,9 +2799,11 @@ TEST(daemon_application_programmatic_index_injects_resource_policy) { ASSERT_NOT_NULL(root); ASSERT_NOT_NULL(cache); cbm_config_t *stored_config = cbm_config_open(cache); - bool configured = stored_config && - cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_FILES, "5") == 0 && - cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_SOURCE_MB, "6") == 0; + bool configured = + stored_config && cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_FILES, "5") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_SOURCE_MB, "6") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_RSS_MB, "64") == 0 && + cbm_config_set(stored_config, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "7") == 0; app_fake_worker_context_t fake; app_fake_worker_context_init(&fake); @@ -2799,7 +2823,8 @@ TEST(daemon_application_programmatic_index_injects_resource_policy) { cbm_daemon_application_t *application = configured ? cbm_daemon_application_new(&config) : NULL; int index_rc = application ? cbm_daemon_application_index(application, "policy-programmatic", root) : -1; - bool policy_propagated = index_rc == 0 && app_fake_worker_policy_equals(&fake, 0, "5", "6"); + bool policy_propagated = + index_rc == 0 && app_fake_worker_policy_equals(&fake, 0, "5", "6", "64", "7"); bool stopped = application && cbm_daemon_application_shutdown(application, APP_TEST_TIMEOUT_MS); cbm_daemon_application_free(application); @@ -2884,6 +2909,87 @@ TEST(daemon_application_programmatic_index_forwards_set_doc_candidates) { ASSERT_EQ(garbage, -1); PASS(); } + +TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried) { + char *cache = th_mktempdir("cbm_app_worker_resource_cache"); + cbm_config_t *stored_config = cache ? cbm_config_open(cache) : NULL; + app_fake_worker_context_t fake; + app_fake_worker_context_init(&fake); + atomic_store(&fake.scripted, true); + fake.outcomes[0] = CBM_PROC_KILLED; + fake.resource_violations[0] = (cbm_index_resource_violation_t){ + .resource = CBM_INDEX_RESOURCE_DURATION_MS, .observed = 7001, .limit = 7000}; + cbm_daemon_application_worker_ops_t worker_ops = { + .context = &fake, + .start = app_fake_worker_start, + .poll = app_fake_worker_poll, + .cancel = app_fake_worker_cancel, + .log_path = app_fake_worker_log_path, + .destroy = app_fake_worker_destroy, + }; + cbm_daemon_application_config_t config = { + .config = stored_config, + .worker_ops = &worker_ops, + }; + cbm_daemon_application_t *application = + stored_config ? cbm_daemon_application_new(&config) : NULL; + cbm_daemon_runtime_application_callbacks_t callbacks = + cbm_daemon_application_runtime_callbacks(application); + cbm_daemon_runtime_application_session_t *session = app_test_open(&callbacks, 4188); + char *root = th_mktempdir("cbm_app_worker_resource"); + uint8_t *context = NULL; + uint32_t context_length = 0; + uint8_t *tool = NULL; + uint32_t tool_length = 0; + char args[APP_TEST_PATH_CAP + 96]; + (void)snprintf(args, sizeof(args), + "{\"repo_path\":\"%s\",\"name\":\"DaemonWorkerResourceFixture\"}", + root ? root : ""); + bool setup = cache && stored_config && application && session && root && + app_test_context_request(root, root, &context, &context_length) && + app_test_tool_request("index_repository", args, &tool, &tool_length); + uint8_t *response = NULL; + uint32_t response_length = 0; + if (setup) { + uint8_t *context_response = NULL; + uint32_t context_response_length = 0; + setup = app_test_request(&callbacks, session, context, context_length, &context_response, + &context_response_length) == CBM_DAEMON_RUNTIME_APPLICATION_OK; + free(context_response); + } + cbm_daemon_runtime_application_status_t status = + setup + ? app_test_request(&callbacks, session, tool, tool_length, &response, &response_length) + : CBM_DAEMON_RUNTIME_APPLICATION_TRANSPORT_ERROR; + bool structured = response && strstr((char *)response, "resource_limit_exceeded") && + strstr((char *)response, "\\\"stage\\\":\\\"worker\\\"") && + strstr((char *)response, "\\\"resource\\\":\\\"duration_ms\\\"") && + strstr((char *)response, "\\\"observed\\\":7001") && + strstr((char *)response, "\\\"limit\\\":7000") && + strstr((char *)response, "\\\"unit\\\":\\\"milliseconds\\\""); + if (session) { + callbacks.session_close(callbacks.context, session); + } + bool stopped = application && cbm_daemon_application_shutdown(application, APP_TEST_TIMEOUT_MS); + int starts = atomic_load(&fake.starts); + int destroys = atomic_load(&fake.destroys); + cbm_daemon_application_free(application); + cbm_config_close(stored_config); + free(response); + free(context); + free(tool); + th_cleanup(root); + th_cleanup(cache); + + ASSERT_TRUE(setup); + ASSERT_EQ(status, CBM_DAEMON_RUNTIME_APPLICATION_OK); + ASSERT_TRUE(structured); + ASSERT_EQ(starts, 1); + ASSERT_EQ(destroys, 1); + ASSERT_TRUE(stopped); + PASS(); +} + TEST(daemon_application_auto_index_honors_tracked_file_limit) { app_env_backup_t cache_environment; bool cache_saved = app_env_backup_capture(&cache_environment, "CBM_CACHE_DIR"); @@ -6953,6 +7059,7 @@ SUITE(daemon_application) { RUN_TEST(daemon_application_unnamed_async_and_status_refuse_ambiguous_root_issue2134); RUN_TEST(daemon_application_programmatic_index_injects_resource_policy); RUN_TEST(daemon_application_programmatic_index_forwards_set_doc_candidates); + RUN_TEST(daemon_application_worker_resource_failure_is_structured_and_not_retried); RUN_TEST(daemon_application_auto_index_honors_tracked_file_limit); RUN_TEST(daemon_application_auto_index_file_count_handles_literal_metacharacter_path); RUN_TEST(daemon_application_auto_index_file_count_supports_non_git_roots); diff --git a/tests/test_index_policy.c b/tests/test_index_policy.c index d866a0631..b33d64cdb 100644 --- a/tests/test_index_policy.c +++ b/tests/test_index_policy.c @@ -21,9 +21,15 @@ TEST(index_policy_defaults_are_disabled) { ASSERT_FALSE(policy.max_files.enabled); ASSERT_FALSE(policy.max_source_bytes.enabled); + ASSERT_FALSE(policy.max_rss_bytes.enabled); + ASSERT_FALSE(policy.max_duration_ms.enabled); ASSERT_FALSE(cbm_index_policy_enabled(&policy)); + ASSERT_FALSE(cbm_index_policy_discovery_enabled(&policy)); + ASSERT_FALSE(cbm_index_policy_worker_enabled(&policy)); ASSERT_STR_EQ(cbm_index_policy_default_value(CBM_INDEX_CONFIG_MAX_FILES), "off"); ASSERT_STR_EQ(cbm_index_policy_default_value(CBM_INDEX_CONFIG_MAX_SOURCE_MB), "off"); + ASSERT_STR_EQ(cbm_index_policy_default_value(CBM_INDEX_CONFIG_MAX_RSS_MB), "off"); + ASSERT_STR_EQ(cbm_index_policy_default_value(CBM_INDEX_CONFIG_MAX_DURATION_SECONDS), "off"); PASS(); } @@ -60,6 +66,38 @@ TEST(index_policy_source_limit_converts_mib_without_overflow) { PASS(); } +TEST(index_policy_worker_limits_validate_and_convert_units) { + cbm_index_resource_policy_t policy; + cbm_index_policy_init(&policy); + char error[256]; + + ASSERT_TRUE( + cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_RSS_MB, "64", error, sizeof(error))); + ASSERT_TRUE(cbm_index_policy_enabled(&policy)); + ASSERT_TRUE(cbm_index_policy_worker_enabled(&policy)); + ASSERT_FALSE(cbm_index_policy_discovery_enabled(&policy)); + ASSERT_EQ(policy.max_rss_bytes.value, UINT64_C(64) * CBM_INDEX_MIB_BYTES); + ASSERT_TRUE(cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_RSS_MB, "1048576", error, + sizeof(error))); + ASSERT_EQ(policy.max_rss_bytes.value, UINT64_C(1048576) * CBM_INDEX_MIB_BYTES); + cbm_index_resource_policy_t before = policy; + ASSERT_FALSE( + cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_RSS_MB, "63", error, sizeof(error))); + ASSERT_EQ(memcmp(&policy, &before, sizeof(policy)), 0); + + ASSERT_TRUE(cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "1", error, + sizeof(error))); + ASSERT_EQ(policy.max_duration_ms.value, 1000); + ASSERT_TRUE(cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "86400", error, + sizeof(error))); + ASSERT_EQ(policy.max_duration_ms.value, UINT64_C(86400000)); + before = policy; + ASSERT_FALSE(cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "86401", + error, sizeof(error))); + ASSERT_EQ(memcmp(&policy, &before, sizeof(policy)), 0); + PASS(); +} + TEST(index_policy_invalid_value_is_rejected_atomically) { static const char *const invalid[] = {"", "0", "-1", "1MB", "1 ", "+1", "10000001", "18446744073709551616"}; @@ -95,6 +133,16 @@ TEST(index_policy_format_round_trips_public_values) { ASSERT_TRUE( cbm_index_policy_format(&policy, CBM_INDEX_CONFIG_MAX_SOURCE_MB, value, sizeof(value))); ASSERT_STR_EQ(value, "7"); + ASSERT_TRUE( + cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_RSS_MB, "64", error, sizeof(error))); + ASSERT_TRUE( + cbm_index_policy_format(&policy, CBM_INDEX_CONFIG_MAX_RSS_MB, value, sizeof(value))); + ASSERT_STR_EQ(value, "64"); + ASSERT_TRUE(cbm_index_policy_set(&policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "9", error, + sizeof(error))); + ASSERT_TRUE(cbm_index_policy_format(&policy, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, value, + sizeof(value))); + ASSERT_STR_EQ(value, "9"); PASS(); } @@ -103,10 +151,18 @@ TEST(index_policy_violation_metadata_is_stable) { ASSERT_STR_EQ(cbm_index_resource_name(CBM_INDEX_RESOURCE_SOURCE_BYTES), "source_bytes"); ASSERT_STR_EQ(cbm_index_resource_unit(CBM_INDEX_RESOURCE_FILES), "files"); ASSERT_STR_EQ(cbm_index_resource_unit(CBM_INDEX_RESOURCE_SOURCE_BYTES), "bytes"); + ASSERT_STR_EQ(cbm_index_resource_name(CBM_INDEX_RESOURCE_RSS_BYTES), "rss_bytes"); + ASSERT_STR_EQ(cbm_index_resource_name(CBM_INDEX_RESOURCE_DURATION_MS), "duration_ms"); + ASSERT_STR_EQ(cbm_index_resource_unit(CBM_INDEX_RESOURCE_RSS_BYTES), "bytes"); + ASSERT_STR_EQ(cbm_index_resource_unit(CBM_INDEX_RESOURCE_DURATION_MS), "milliseconds"); ASSERT_STR_EQ(cbm_index_resource_config_key(CBM_INDEX_RESOURCE_FILES), CBM_INDEX_CONFIG_MAX_FILES); ASSERT_STR_EQ(cbm_index_resource_config_key(CBM_INDEX_RESOURCE_SOURCE_BYTES), CBM_INDEX_CONFIG_MAX_SOURCE_MB); + ASSERT_STR_EQ(cbm_index_resource_config_key(CBM_INDEX_RESOURCE_RSS_BYTES), + CBM_INDEX_CONFIG_MAX_RSS_MB); + ASSERT_STR_EQ(cbm_index_resource_config_key(CBM_INDEX_RESOURCE_DURATION_MS), + CBM_INDEX_CONFIG_MAX_DURATION_SECONDS); PASS(); } @@ -122,9 +178,13 @@ TEST(index_policy_config_loads_defaults_values_and_rejects_corruption) { ASSERT_FALSE(cbm_index_policy_enabled(&policy)); ASSERT_EQ(cbm_config_set(config, CBM_INDEX_CONFIG_MAX_FILES, "9"), 0); ASSERT_EQ(cbm_config_set(config, CBM_INDEX_CONFIG_MAX_SOURCE_MB, "3"), 0); + ASSERT_EQ(cbm_config_set(config, CBM_INDEX_CONFIG_MAX_RSS_MB, "64"), 0); + ASSERT_EQ(cbm_config_set(config, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "7"), 0); ASSERT_TRUE(cbm_config_load_index_policy(config, &policy, error, sizeof(error))); ASSERT_EQ(policy.max_files.value, 9); ASSERT_EQ(policy.max_source_bytes.value, UINT64_C(3) * CBM_INDEX_MIB_BYTES); + ASSERT_EQ(policy.max_rss_bytes.value, UINT64_C(64) * CBM_INDEX_MIB_BYTES); + ASSERT_EQ(policy.max_duration_ms.value, UINT64_C(7000)); ASSERT_EQ(cbm_config_set(config, CBM_INDEX_CONFIG_MAX_FILES, "corrupt"), 0); ASSERT_FALSE(cbm_config_load_index_policy(config, &policy, error, sizeof(error))); @@ -135,16 +195,23 @@ TEST(index_policy_config_loads_defaults_values_and_rejects_corruption) { PASS(); } -TEST(index_policy_cli_lists_both_operator_keys) { +TEST(index_policy_cli_lists_all_operator_keys) { bool files_found = false; bool bytes_found = false; + bool rss_found = false; + bool duration_found = false; for (size_t index = 0; index < cbm_cli_config_key_count_for_testing(); index++) { const char *key = cbm_cli_config_key_at_for_testing(index); files_found = files_found || (key && strcmp(key, CBM_INDEX_CONFIG_MAX_FILES) == 0); bytes_found = bytes_found || (key && strcmp(key, CBM_INDEX_CONFIG_MAX_SOURCE_MB) == 0); + rss_found = rss_found || (key && strcmp(key, CBM_INDEX_CONFIG_MAX_RSS_MB) == 0); + duration_found = + duration_found || (key && strcmp(key, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS) == 0); } ASSERT_TRUE(files_found); ASSERT_TRUE(bytes_found); + ASSERT_TRUE(rss_found); + ASSERT_TRUE(duration_found); PASS(); } @@ -391,11 +458,12 @@ SUITE(index_policy) { RUN_TEST(index_policy_defaults_are_disabled); RUN_TEST(index_policy_file_limit_accepts_off_and_exact_range); RUN_TEST(index_policy_source_limit_converts_mib_without_overflow); + RUN_TEST(index_policy_worker_limits_validate_and_convert_units); RUN_TEST(index_policy_invalid_value_is_rejected_atomically); RUN_TEST(index_policy_format_round_trips_public_values); RUN_TEST(index_policy_violation_metadata_is_stable); RUN_TEST(index_policy_config_loads_defaults_values_and_rejects_corruption); - RUN_TEST(index_policy_cli_lists_both_operator_keys); + RUN_TEST(index_policy_cli_lists_all_operator_keys); RUN_TEST(index_policy_cli_set_rejects_invalid_value_without_overwrite); RUN_TEST(index_policy_cli_set_reports_a_failed_write); RUN_TEST(index_policy_worker_rejects_missing_parent_policy); diff --git a/tests/test_index_supervisor.c b/tests/test_index_supervisor.c index 1861c8076..88d58c0d7 100644 --- a/tests/test_index_supervisor.c +++ b/tests/test_index_supervisor.c @@ -1019,6 +1019,269 @@ TEST(index_supervisor_job_memory_limit_has_floor_headroom_and_no_overflow) { PASS(); } +typedef struct { + uint64_t now_ms; + uint64_t rss_values[4]; + int rss_value_count; + int rss_calls; + int clock_calls; + cbm_proc_tree_rss_status_t rss_status; +} index_supervisor_resource_fake_t; + +static uint64_t index_supervisor_fake_clock(void *context) { + index_supervisor_resource_fake_t *fake = context; + fake->clock_calls++; + return fake->now_ms; +} + +static cbm_proc_tree_rss_status_t index_supervisor_fake_rss(cbm_subprocess_t *process, + uint64_t *rss_bytes, void *context) { + (void)process; + index_supervisor_resource_fake_t *fake = context; + int value_index = + fake->rss_calls < fake->rss_value_count ? fake->rss_calls : fake->rss_value_count - 1; + fake->rss_calls++; + if (fake->rss_status == CBM_PROC_TREE_RSS_OK && rss_bytes && value_index >= 0) { + *rss_bytes = fake->rss_values[value_index]; + } + return fake->rss_status; +} + +static cbm_index_resource_policy_t index_supervisor_test_worker_policy(uint64_t rss_bytes, + uint64_t duration_ms) { + cbm_index_resource_policy_t policy; + cbm_index_policy_init(&policy); + policy.max_rss_bytes = (cbm_index_limit_u64_t){.enabled = rss_bytes > 0, .value = rss_bytes}; + policy.max_duration_ms = + (cbm_index_limit_u64_t){.enabled = duration_ms > 0, .value = duration_ms}; + return policy; +} + +TEST(index_supervisor_disabled_limits_do_not_probe) { + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_values = {UINT64_MAX}, + .rss_value_count = 1, + .rss_status = CBM_PROC_TREE_RSS_OK, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 0); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"clean\"}", 0, + &policy, false, NULL, NULL, &handle); + const cbm_index_worker_result_t *result = NULL; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + cbm_index_resource_t resource = + result ? result->resource_violation.resource : CBM_INDEX_RESOURCE_RSS_BYTES; + bool probe_failed = result && result->resource_probe_failed; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(start_rc, 0); + ASSERT_TRUE(terminal); + ASSERT_EQ(fake.rss_calls, 0); + ASSERT_EQ(fake.clock_calls, 0); + ASSERT_EQ(resource, CBM_INDEX_RESOURCE_NONE); + ASSERT_FALSE(probe_failed); + PASS(); +} + +TEST(index_supervisor_rss_equality_runs_then_excess_terminates_tree) { + const uint64_t limit = UINT64_C(64) * CBM_INDEX_MIB_BYTES; + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_values = {limit, limit + 1}, + .rss_value_count = 2, + .rss_status = CBM_PROC_TREE_RSS_OK, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 0); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, &handle); + const cbm_index_worker_result_t *result = NULL; + cbm_index_worker_poll_t equal_state = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + fake.now_ms += 250; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + const cbm_index_worker_result_t *cached = NULL; + bool cached_terminal = + terminal && cbm_index_worker_poll(handle, &cached) == CBM_INDEX_WORKER_POLL_TERMINAL && + cached == result; + bool limited = terminal && result && + result->resource_violation.resource == CBM_INDEX_RESOURCE_RSS_BYTES && + result->resource_violation.observed == limit + 1 && + result->resource_violation.limit == limit && !result->cancellation_requested && + result->tree_quiesced && !result->supervision_failed; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(start_rc, 0); + ASSERT_EQ(equal_state, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(limited); + ASSERT_TRUE(cached_terminal); + PASS(); +} + +TEST(index_supervisor_duration_is_total_time_not_quiet_timeout) { + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_EMPTY, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, &handle); + fake.now_ms = 1100; + const cbm_index_worker_result_t *result = NULL; + cbm_index_worker_poll_t equal_state = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + fake.now_ms = 1101; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool limited = terminal && result && + result->resource_violation.resource == CBM_INDEX_RESOURCE_DURATION_MS && + result->resource_violation.observed == 1001 && + result->resource_violation.limit == 1000 && result->outcome != CBM_PROC_HANG && + result->tree_quiesced; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(start_rc, 0); + ASSERT_EQ(equal_state, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(limited); + PASS(); +} + +TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled) { + const char *saved_timeout = getenv("CBM_INDEX_WORKER_TIMEOUT_S"); + char *saved_timeout_copy = saved_timeout ? cbm_strdup(saved_timeout) : NULL; + (void)cbm_setenv("CBM_INDEX_WORKER_TIMEOUT_S", "1", 1); + cbm_index_supervisor_reset_resource_hooks_for_testing(); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 60000); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, &handle); + const cbm_index_worker_result_t *result = NULL; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool remained_hang = terminal && result && result->outcome == CBM_PROC_HANG && + result->resource_violation.resource == CBM_INDEX_RESOURCE_NONE && + !result->resource_probe_failed; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + index_supervisor_test_restore_env("CBM_INDEX_WORKER_TIMEOUT_S", saved_timeout_copy); + + ASSERT_EQ(start_rc, 0); + ASSERT_TRUE(terminal); + ASSERT_TRUE(remained_hang); + PASS(); +} + +TEST(index_supervisor_cancel_precedes_resource_probe) { + const uint64_t limit = UINT64_C(64) * CBM_INDEX_MIB_BYTES; + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_values = {limit + 1}, + .rss_value_count = 1, + .rss_status = CBM_PROC_TREE_RSS_OK, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 1000); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, &handle); + fake.now_ms = 2000; + bool cancel_accepted = handle && cbm_index_worker_request_cancel(handle); + const cbm_index_worker_result_t *result = NULL; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool cancelled = terminal && result && result->cancellation_requested && + result->resource_violation.resource == CBM_INDEX_RESOURCE_NONE && + !result->resource_probe_failed && result->tree_quiesced; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(start_rc, 0); + ASSERT_TRUE(cancel_accepted); + ASSERT_TRUE(cancelled); + ASSERT_EQ(fake.rss_calls, 0); + PASS(); +} + +TEST(index_supervisor_three_failed_rss_probes_fail_closed) { + const uint64_t limit = UINT64_C(64) * CBM_INDEX_MIB_BYTES; + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_ERROR, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 0); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, &handle); + const cbm_index_worker_result_t *result = NULL; + cbm_index_worker_poll_t first = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + cbm_index_worker_poll_t throttled = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + int calls_before_interval = fake.rss_calls; + fake.now_ms += 250; + cbm_index_worker_poll_t second = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + fake.now_ms += 250; + cbm_index_worker_poll_t third = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool failed_closed = terminal && result && result->resource_probe_failed && + result->resource_violation.resource == CBM_INDEX_RESOURCE_RSS_BYTES && + !result->cancellation_requested && result->tree_quiesced; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(start_rc, 0); + ASSERT_EQ(first, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_EQ(throttled, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_EQ(calls_before_interval, 1); + ASSERT_EQ(second, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_EQ(third, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(failed_closed); + ASSERT_EQ(fake.rss_calls, 3); + PASS(); +} + SUITE(index_supervisor) { RUN_TEST(index_supervisor_job_memory_limit_has_floor_headroom_and_no_overflow); RUN_TEST(index_supervisor_worker_argv_requires_exact_build_bound_grammar); @@ -1030,4 +1293,10 @@ SUITE(index_supervisor) { RUN_TEST(index_supervisor_oversized_response_is_contained_and_log_is_retained); RUN_TEST(index_supervisor_clean_exit_without_response_is_named_failure_issue1300); RUN_TEST(index_supervisor_killed_worker_log_is_never_empty_and_names_the_run); + RUN_TEST(index_supervisor_disabled_limits_do_not_probe); + RUN_TEST(index_supervisor_rss_equality_runs_then_excess_terminates_tree); + RUN_TEST(index_supervisor_duration_is_total_time_not_quiet_timeout); + RUN_TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled); + RUN_TEST(index_supervisor_cancel_precedes_resource_probe); + RUN_TEST(index_supervisor_three_failed_rss_probes_fail_closed); } diff --git a/tests/test_mcp.c b/tests/test_mcp.c index ebf24a827..3bfcb018f 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -21581,6 +21581,103 @@ TEST(index_supervisor_clean_exit_without_response_response_issue1300) { PASS(); } +#ifndef _WIN32 +typedef struct { + int calls; +} index_worker_resource_clock_t; + +static uint64_t index_worker_resource_clock(void *context) { + index_worker_resource_clock_t *clock = context; + clock->calls++; + return clock->calls == 1 ? 100 : 1101; +} + +enum { + IDXRESOURCE_OK = 0, + IDXRESOURCE_SETUP = 71, + IDXRESOURCE_NO_RESPONSE = 72, + IDXRESOURCE_BAD_CONTRACT = 73, + IDXRESOURCE_RETRIED = 74, +}; + +static int index_worker_resource_response_check(const char *repo, const char *cache) { + (void)cbm_setenv("CBM_CACHE_DIR", cache, 1); + cbm_unsetenv("CBM_INDEX_SUPERVISOR"); + cbm_index_supervisor_mark_host(); + cbm_config_t *config = cbm_config_open(cache); + cbm_mcp_server_t *server = cbm_mcp_server_new(NULL); + if (!config || !server || + cbm_config_set(config, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "1") != 0) { + cbm_mcp_server_free(server); + cbm_config_close(config); + return IDXRESOURCE_SETUP; + } + cbm_mcp_server_set_config(server, config); + index_worker_resource_clock_t clock = {0}; + cbm_index_supervisor_set_resource_hooks_for_testing(index_worker_resource_clock, NULL, &clock); + int before = cbm_index_supervisor_spawn_count(); + char args[CBM_SZ_2K]; + (void)snprintf(args, sizeof(args), + "{\"repo_path\":\"%s\",\"mode\":\"fast\"," + "\"_cbm_index_policy\":{\"index_max_files\":\"off\"," + "\"index_max_source_mb\":\"off\",\"index_max_rss_mb\":\"off\"," + "\"index_max_duration_seconds\":\"off\"}}", + repo); + char *response = cbm_mcp_handle_tool(server, "index_repository", args); + int after = cbm_index_supervisor_spawn_count(); + cbm_index_supervisor_reset_resource_hooks_for_testing(); + int result = IDXRESOURCE_OK; + if (!response) { + result = IDXRESOURCE_NO_RESPONSE; + } else if (!strstr(response, "resource_limit_exceeded") || + !strstr(response, "\\\"stage\\\":\\\"worker\\\"") || + !strstr(response, "\\\"resource\\\":\\\"duration_ms\\\"") || + !strstr(response, "\\\"observed\\\":1001") || + !strstr(response, "\\\"limit\\\":1000") || + !strstr(response, "\\\"unit\\\":\\\"milliseconds\\\"")) { + result = IDXRESOURCE_BAD_CONTRACT; + } else if (after - before != 1) { + result = IDXRESOURCE_RETRIED; + } + free(response); + cbm_mcp_server_free(server); + cbm_config_close(config); + return result; +} +#endif + +TEST(index_worker_resource_limit_is_trusted_structured_and_not_retried) { +#ifdef _WIN32 + SKIP_PLATFORM("fork-isolated MCP host harness; supervisor state machine is cross-platform"); +#else + char *repo = th_mktempdir("cbm_worker_resource_repo"); + char *cache = th_mktempdir("cbm_worker_resource_cache"); + ASSERT_NOT_NULL(repo); + ASSERT_NOT_NULL(cache); + ASSERT_EQ(th_write_file(TH_PATH(repo, "main.c"), "int main(void) { return 0; }\n"), 0); + fflush(NULL); + pid_t pid = fork(); + if (pid == 0) { + alarm(60); + _exit(index_worker_resource_response_check(repo, cache)); + } + int status = 0; + bool waited = waitpid(pid, &status, 0) == pid; + int exit_code = waited && WIFEXITED(status) ? WEXITSTATUS(status) : -1; + int signal_code = waited && WIFSIGNALED(status) ? WTERMSIG(status) : 0; + th_cleanup(repo); + th_cleanup(cache); + if (exit_code != IDXRESOURCE_OK) { + printf(" worker resource child exit=%d signal=%d\n", exit_code, signal_code); + } + ASSERT_TRUE(waited); + ASSERT_EQ(signal_code, 0); + ASSERT_EQ(exit_code, IDXRESOURCE_OK); + PASS(); +#endif +} + + /* Child-side check: index a tiny fixture and verify it ran IN-PROCESS. * Distinct exit codes so the parent can report the exact failure mode. */ enum { @@ -25140,6 +25237,7 @@ SUITE(mcp) { RUN_TEST(index_supervisor_gate_requires_marked_host_issue845); RUN_TEST(index_supervisor_start_failure_is_fail_closed_in_real_host); RUN_TEST(index_supervisor_clean_exit_without_response_response_issue1300); + RUN_TEST(index_worker_resource_limit_is_trusted_structured_and_not_retried); RUN_TEST(index_bg_paths_route_through_supervisor_issue832); RUN_TEST(sequential_service_edge_props_are_valid_json_issue898); RUN_TEST(index_second_inprocess_run_survives_issue773); diff --git a/tests/test_subprocess.c b/tests/test_subprocess.c index c7f78b3d8..385293e08 100644 --- a/tests/test_subprocess.c +++ b/tests/test_subprocess.c @@ -101,6 +101,12 @@ TEST(subprocess_outcome_str) { PASS(); } +TEST(subprocess_tree_rss_sum_saturates_on_overflow) { + const uint64_t values[] = {UINT64_MAX - 4, 4, 1}; + ASSERT_EQ(cbm_subprocess_rss_sum_for_testing(values, 3), UINT64_MAX); + PASS(); +} + /* ── Layer 2: real spawn/reap (POSIX) ─────────────────────────────────────── */ extern const char *cbm_test_program_path; @@ -801,6 +807,45 @@ TEST(subprocess_spawn_returns_while_child_is_running) { #endif } +TEST(subprocess_tree_rss_measures_contained_descendants) { +#ifdef _WIN32 + SKIP_PLATFORM("native Job Object process-tree RSS runs on Windows CI"); +#else + char pid_path[64]; + ASSERT_TRUE(make_tree_pid_path(pid_path)); + cbm_subprocess_t *process = NULL; + ASSERT_EQ(spawn_ignoring_tree(pid_path, 0, 100, &process), 0); + pid_t parent_pid = 0; + pid_t grandchild_pid = 0; + bool ready = wait_for_tree_pids(pid_path, process, &parent_pid, &grandchild_pid, 3000); + uint64_t rss_bytes = 0; + cbm_proc_tree_rss_status_t rss_status = + ready ? cbm_subprocess_tree_rss_bytes(process, &rss_bytes) : CBM_PROC_TREE_RSS_ERROR; + bool cancel_accepted = cbm_subprocess_request_cancel(process); + cbm_proc_result_t result; + bool terminal = poll_until_terminal(process, 5000, &result); + uint64_t final_rss = UINT64_MAX; + cbm_proc_tree_rss_status_t final_rss_status = + terminal ? cbm_subprocess_tree_rss_bytes(process, &final_rss) : CBM_PROC_TREE_RSS_ERROR; + if (terminal) { + cbm_subprocess_destroy(process); + } else { + force_probe_cleanup(parent_pid, grandchild_pid); + } + (void)unlink(pid_path); + + ASSERT_TRUE(ready); + ASSERT_EQ(rss_status, CBM_PROC_TREE_RSS_OK); + ASSERT_TRUE(rss_bytes > 0); + ASSERT_TRUE(cancel_accepted); + ASSERT_TRUE(terminal); + ASSERT_TRUE(result.tree_quiesced); + ASSERT_EQ(final_rss_status, CBM_PROC_TREE_RSS_EMPTY); + ASSERT_EQ(final_rss, 0); + PASS(); +#endif +} + TEST(subprocess_natural_completion_is_cached_across_polls) { #ifdef _WIN32 SKIP_PLATFORM("POSIX /bin/sh completion-cache probe; native Job Object coverage pending"); @@ -2635,6 +2680,7 @@ SUITE(subprocess) { #ifndef _WIN32 RUN_TEST(subprocess_environment_path_preserves_caller_lookup); #endif + RUN_TEST(subprocess_tree_rss_sum_saturates_on_overflow); RUN_TEST(subprocess_run_clean); RUN_TEST(subprocess_run_exit_nonzero); RUN_TEST(subprocess_run_child_exited_before_group_check); @@ -2648,6 +2694,7 @@ SUITE(subprocess) { RUN_TEST(subprocess_run_spawn_failure); RUN_TEST(subprocess_run_null_bin_rejected); RUN_TEST(subprocess_spawn_returns_while_child_is_running); + RUN_TEST(subprocess_tree_rss_measures_contained_descendants); RUN_TEST(subprocess_natural_completion_is_cached_across_polls); RUN_TEST(subprocess_cancel_is_idempotent_and_kills_ignoring_tree); RUN_TEST(subprocess_quiet_timeout_kills_ignoring_tree); From a45858ffb5a53a3c4d7707d7a237f7f37d749aa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=86=B2?= Date: Tue, 22 Sep 2026 01:28:50 +0800 Subject: [PATCH 2/8] fix(test): send the complete worker policy from the session-scope fixture MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PR 2257 still named only the two discovery keys in the worker-scope script. The worker now fail-closes unless every known policy key is present, so the fixture must stand in for the supervisor with cbm_test_index_worker_policy_json. Signed-off-by: 刘冲 --- tests/test_worker_session_scope.sh | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/test_worker_session_scope.sh b/tests/test_worker_session_scope.sh index ab018ba76..1f32c4682 100644 --- a/tests/test_worker_session_scope.sh +++ b/tests/test_worker_session_scope.sh @@ -73,9 +73,9 @@ run_worker() { # A worker never reads the resource policy from config or environment: the # supervisor resolves it and sends it inside the request, and a request # without one is refused ("missing or incomplete trusted worker policy"). -# This script plays the supervisor, so it sends what the supervisor sends -- -# both limits off, the default. -policy='"_cbm_index_policy":{"index_max_files":"off","index_max_source_mb":"off"}' +# This script plays the supervisor, so it sends the same complete object +# cbm_test_index_worker_policy_json produces -- every known key, all off. +policy="$(cbm_test_index_worker_policy_json)" response="${tmpdir}/scoped.response" if ! run_worker "{\"repo_path\":\"${repo}\",\"mode\":\"fast\",${policy}}" "${response}" \ From 1e4751d3ca4dab07fef38416729ff7b445d8cedf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=88=98=E5=86=B2?= Date: Tue, 22 Sep 2026 03:05:13 +0800 Subject: [PATCH 3/8] fix(index): charge macOS phys_footprint and pin duration across recovery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Watchdogs now sample the OS-charged footprint instead of resident size on Darwin, and index_max_duration_seconds covers the whole request so a crash/hang respawn cannot reset the clock. Signed-off-by: 刘冲 --- README.md | 4 +- docs/CONFIGURATION.md | 4 +- docs/INDEX_RESOURCE_LIMITS.md | 26 +++++---- src/cli/cli.c | 5 +- src/daemon/application.c | 13 +++-- src/daemon/application.h | 2 +- src/foundation/subprocess.c | 12 ++++- src/foundation/subprocess.h | 10 ++-- src/mcp/index_supervisor.c | 36 ++++++++----- src/mcp/index_supervisor.h | 12 +++-- src/mcp/mcp.c | 8 +-- tests/test_daemon_application.c | 2 + tests/test_index_supervisor.c | 94 ++++++++++++++++++++++++++------- 13 files changed, 165 insertions(+), 63 deletions(-) diff --git a/README.md b/README.md index 4466dae50..c34d20b98 100644 --- a/README.md +++ b/README.md @@ -928,8 +928,8 @@ codebase-memory-mcp config set watcher_enabled false # stop the watcher thre codebase-memory-mcp config set watch_non_git true # also poll non-git project roots (default: false) codebase-memory-mcp config set index_max_files 250000 # optional per-index source-file limit codebase-memory-mcp config set index_max_source_mb 16384 # optional per-index source-size limit -codebase-memory-mcp config set index_max_rss_mb 8192 # optional worker-tree current RSS limit -codebase-memory-mcp config set index_max_duration_seconds 3600 # optional total worker duration +codebase-memory-mcp config set index_max_rss_mb 8192 # optional worker-tree charged-memory limit +codebase-memory-mcp config set index_max_duration_seconds 3600 # optional per-request duration codebase-memory-mcp config reset auto_index # reset to default ``` diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index 3d3e9a971..a02459b4a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -93,8 +93,8 @@ Current keys: | `watch_non_git` | `false` | Also poll project roots that are **not git repositories**. By default the watcher only follows git projects, so a project indexed from a plain directory is never refreshed after its first index — reindex it manually with `index_repository`. Set `true` to poll such roots on the same adaptive cadence with a file-tree scan: the indexer's own discovery walk (same skip lists, `.gitignore` and `.cbmignore` rules) hashed over each file's path, size and mtime. Any change reindexes once; paths the indexer skips (including cbm's own `.codebase-memory/` output) never trigger. The first poll after the daemon starts reindexes each such project once, since nothing records which tree state the index holds. The scan walks the whole tree every poll, so it costs more than git polling on very large trees. Read once when the daemon starts, like `watcher_enabled`. | | `index_max_files` | `off` | Optional maximum number of accepted source files in one discovery run. | | `index_max_source_mb` | `off` | Optional maximum accepted source size in MiB in one discovery run. | -| `index_max_rss_mb` | `off` | Optional maximum current RSS in MiB for the complete contained index-worker process tree (`64..1048576`). | -| `index_max_duration_seconds` | `off` | Optional maximum total worker duration in seconds (`1..86400`). | +| `index_max_rss_mb` | `off` | Optional maximum charged memory in MiB for the complete contained index-worker process tree (`64..1048576`). On macOS this is phys_footprint, not resident size. | +| `index_max_duration_seconds` | `off` | Optional maximum wall-clock duration in seconds for the whole index request, including crash/hang recovery (`1..86400`). | For a watched project, `index_status` names the polling method in `watch.strategy`: `git` (HEAD and dirty-state polling), `tree` (a non-git root polled by the diff --git a/docs/INDEX_RESOURCE_LIMITS.md b/docs/INDEX_RESOURCE_LIMITS.md index 66be4ecf4..152599360 100644 --- a/docs/INDEX_RESOURCE_LIMITS.md +++ b/docs/INDEX_RESOURCE_LIMITS.md @@ -27,24 +27,30 @@ ranges are rejected without changing the stored value. | Key | Default | Accepted value | Protects | |---|---:|---:|---| -| `index_max_rss_mb` | `off` | `off` or `64..1048576` | Current RSS of the complete worker process tree | -| `index_max_duration_seconds` | `off` | `off` or `1..86400` | Total worker wall-clock duration | +| `index_max_rss_mb` | `off` | `off` or `64..1048576` | Charged memory of the complete worker process tree | +| `index_max_duration_seconds` | `off` | `off` or `1..86400` | Wall-clock duration of the whole index request | ```bash codebase-memory-mcp config set index_max_rss_mb 8192 codebase-memory-mcp config set index_max_duration_seconds 3600 ``` -RSS is the current resident memory of the contained worker and every descendant, -not the worker's allocation budget and not peak memory. This hard watchdog is +RSS is the charged memory of the contained worker and every descendant, not +the worker's allocation budget and not peak memory. On macOS that quantity is +`phys_footprint` (the same number `cbm_mem_charged()` enforces), not +`resident_size`, which still counts pages the allocator has already handed +back. Linux and Windows use RSS / working set. This hard watchdog is separate from the internal `CBM_MEM_BUDGET_MB` soft budget. The supervisor -samples RSS at most once every 250 milliseconds so the watchdog does not turn -full process-table enumeration into a busy loop. +samples the tree at most once every 250 milliseconds so the watchdog does not +turn full process-table enumeration into a busy loop. -Duration uses a monotonic clock from successful spawn. It is independent of the -existing 15-minute quiet timeout: continuous log progress does not reset total -duration, while the quiet timeout continues to identify a worker that stops -making progress. +Duration is per request: the clock starts at the first worker spawn and is +not reset when crash/hang recovery starts a later attempt. Continuous log +progress does not reset it. It is independent of the existing 15-minute quiet +timeout, which still identifies a worker that stops making progress. A +duration limit shorter than that quiet timeout kills a hung worker before hang +quarantine can name the in-flight file, so the next attempt may hang on the +same file. Equality is allowed. The first RSS or elapsed-duration observation above its limit starts the existing graceful-to-force process-tree shutdown. CBM reports diff --git a/src/cli/cli.c b/src/cli/cli.c index 00b2db341..fd51c48e9 100644 --- a/src/cli/cli.c +++ b/src/cli/cli.c @@ -7603,8 +7603,9 @@ static const config_key_def_t CONFIG_KEYS[] = { {CBM_CONFIG_UI_PORT, "9749", "Port for the graph UI listener when enabled"}, {CBM_INDEX_CONFIG_MAX_FILES, "off", "Max accepted source files per index, or off"}, {CBM_INDEX_CONFIG_MAX_SOURCE_MB, "off", "Max accepted source MiB per index, or off"}, - {CBM_INDEX_CONFIG_MAX_RSS_MB, "off", "Max worker process-tree RSS MiB, or off"}, - {CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "off", "Max worker duration in seconds, or off"}, + {CBM_INDEX_CONFIG_MAX_RSS_MB, "off", "Max worker process-tree charged memory MiB, or off"}, + {CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "off", + "Max wall-clock seconds for the whole index request, or off"}, }; /* #1558: ui_enabled and ui_port were reachable ONLY by hand-editing diff --git a/src/daemon/application.c b/src/daemon/application.c index a7b2f4f5b..0f696d59c 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -159,6 +159,7 @@ struct cbm_daemon_application_job { * job outlives the request and session that started it. */ bool async_owned; bool worker_started; + uint64_t request_started_ms; cbm_daemon_application_job_t *next; }; @@ -361,6 +362,7 @@ static void application_worker_diagnostic(const char *line, void *context) { static int application_worker_start_default(void *context, const char *args_json, size_t memory_budget_bytes, const char *marker_file, const char *quarantine_file, + uint64_t duration_origin_ms, cbm_daemon_application_worker_t *worker_out) { (void)context; cbm_index_resource_policy_t resource_policy; @@ -382,7 +384,7 @@ static int application_worker_start_default(void *context, const char *args_json yyjson_doc_free(args_doc); int result = cbm_index_worker_start_with_policy_log( args_json, memory_budget_bytes, &resource_policy, false, marker_file, quarantine_file, - application_worker_diagnostic, worker, &worker->handle); + application_worker_diagnostic, worker, duration_origin_ms, &worker->handle); if (result != 0) { application_worker_destroy_default(NULL, worker); *worker_out = NULL; @@ -1377,10 +1379,13 @@ static application_attempt_status_t application_job_run_attempt(cbm_daemon_appli } cbm_daemon_application_worker_t worker = NULL; + if (job->request_started_ms == 0) { + job->request_started_ms = cbm_index_worker_now_ms(); + } application_tmp_lock(); - int start_result = - application->worker_ops.start(application->worker_ops.context, job->args_json, - memory_budget_bytes, marker_path, quarantine_path, &worker); + int start_result = application->worker_ops.start( + application->worker_ops.context, job->args_json, memory_budget_bytes, marker_path, + quarantine_path, job->request_started_ms, &worker); application_tmp_unlock(); if (start_result != 0 || !worker) { return application_job_cancel_requested(job) ? APPLICATION_ATTEMPT_CANCELLED diff --git a/src/daemon/application.h b/src/daemon/application.h index 933186aa7..426bef3db 100644 --- a/src/daemon/application.h +++ b/src/daemon/application.h @@ -31,7 +31,7 @@ typedef void *cbm_daemon_application_update_worker_t; typedef struct { void *context; int (*start)(void *context, const char *args_json, size_t memory_budget_bytes, - const char *marker_file, const char *quarantine_file, + const char *marker_file, const char *quarantine_file, uint64_t duration_origin_ms, cbm_daemon_application_worker_t *worker_out); cbm_index_worker_poll_t (*poll)(void *context, cbm_daemon_application_worker_t worker, const cbm_index_worker_result_t **result_out); diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 474ada1d2..519a554e2 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -513,6 +513,12 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces bool root_failed = false; for (DWORD index = 0; index < processes->NumberOfProcessIdsInList; index++) { DWORD pid = (DWORD)processes->ProcessIdList[index]; + /* GetProcessMemoryInfo documents PROCESS_QUERY_INFORMATION | + * PROCESS_VM_READ. PROCESS_QUERY_LIMITED_INFORMATION is a lesser + * alternative on newer Windows; the Job Object membership list + * already closes most of the pid-reuse window between listing and + * opening. A member we cannot open is a probe failure when it is + * the root. */ HANDLE member = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid); if (!member) { root_failed = root_failed || pid == process->process_id; @@ -592,7 +598,11 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces root_failed = root_failed || pids[index] == process->pid; continue; } - cbm_rss_add_saturated(&total, usage.ri_resident_size); + /* phys_footprint is what the OS charges, matching cbm_mem_charged(). + * ri_resident_size still counts pages mimalloc already marked + * MADV_FREE_REUSABLE, so a Mac worker can look like 17 GB RSS while + * the kernel holds it to 5.5 GB. */ + cbm_rss_add_saturated(&total, usage.ri_phys_footprint); measured++; } cbm_free(CBM_MEM_CLASS_OTHER, pids); diff --git a/src/foundation/subprocess.h b/src/foundation/subprocess.h index bfcb2311b..94a8a2ed0 100644 --- a/src/foundation/subprocess.h +++ b/src/foundation/subprocess.h @@ -155,10 +155,12 @@ int cbm_subprocess_spawn(const cbm_proc_opts_t *opts, cbm_subprocess_t **out); * tree when cancel_grace_ms elapses. Callers must keep polling to make progress. */ cbm_proc_poll_t cbm_subprocess_poll(cbm_subprocess_t *process, cbm_proc_result_t *out); -/* Read the current resident-set size of the complete contained process tree. - * OK returns an overflow-safe byte total, EMPTY means the owned tree currently - * has no observable members, and ERROR means no trustworthy measurement could - * be obtained. Individual processes that exit during enumeration are ignored. */ +/* Read the current charged memory of the complete contained process tree. + * On macOS this is phys_footprint (what the OS holds against the process); + * elsewhere it is RSS. OK returns an overflow-safe byte total, EMPTY means + * the owned tree currently has no observable members, and ERROR means no + * trustworthy measurement could be obtained. Individual processes that exit + * during enumeration are ignored. */ cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_bytes(cbm_subprocess_t *process, uint64_t *rss_bytes); bool cbm_subprocess_root_running(const cbm_subprocess_t *process); diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index 4ffeecbb8..e84dff755 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -881,7 +881,8 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, - void *log_context, cbm_index_worker_handle_t **handle_out) { + void *log_context, uint64_t duration_origin_ms, + cbm_index_worker_handle_t **handle_out) { if (handle_out) { *handle_out = NULL; } @@ -985,34 +986,42 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt return -1; } if (handle->resource_policy.max_duration_ms.enabled) { - handle->started_ms = worker_resource_now_ms(); + /* Duration is per request, not per spawn: a crash/hang recovery must + * not reset the clock. Callers pass the request origin; 0 means now. */ + uint64_t origin = duration_origin_ms != 0 ? duration_origin_ms : worker_resource_now_ms(); + handle->started_ms = origin; } *handle_out = handle; return 0; } +uint64_t cbm_index_worker_now_ms(void) { + return worker_resource_now_ms(); +} + int cbm_index_worker_start_with_log(const char *args_json, size_t memory_budget_bytes, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, void *log_context, cbm_index_worker_handle_t **handle_out) { return worker_start_internal(args_json, memory_budget_bytes, NULL, single_thread, marker_file, - quarantine_file, log_callback, log_context, handle_out); + quarantine_file, log_callback, log_context, 0, handle_out); } int cbm_index_worker_start(const char *args_json, size_t memory_budget_bytes, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_index_worker_handle_t **handle_out) { return worker_start_internal(args_json, memory_budget_bytes, NULL, single_thread, marker_file, - quarantine_file, NULL, NULL, handle_out); + quarantine_file, NULL, NULL, 0, handle_out); } int cbm_index_worker_start_with_policy(const char *args_json, size_t memory_budget_bytes, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, - const char *quarantine_file, + const char *quarantine_file, uint64_t duration_origin_ms, cbm_index_worker_handle_t **handle_out) { return worker_start_internal(args_json, memory_budget_bytes, resource_policy, single_thread, - marker_file, quarantine_file, NULL, NULL, handle_out); + marker_file, quarantine_file, NULL, NULL, duration_origin_ms, + handle_out); } int cbm_index_worker_start_with_policy_log(const char *args_json, size_t memory_budget_bytes, @@ -1020,10 +1029,11 @@ int cbm_index_worker_start_with_policy_log(const char *args_json, size_t memory_ bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, void *log_context, + uint64_t duration_origin_ms, cbm_index_worker_handle_t **handle_out) { return worker_start_internal(args_json, memory_budget_bytes, resource_policy, single_thread, marker_file, quarantine_file, log_callback, log_context, - handle_out); + duration_origin_ms, handle_out); } cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, @@ -1167,14 +1177,15 @@ static int worker_spawn_internal(const char *args_json, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, void *log_context, const atomic_int *cancel_requested, - cbm_index_worker_result_t *result) { + uint64_t duration_origin_ms, cbm_index_worker_result_t *result) { if (!result) { return -1; } worker_result_init(result); cbm_index_worker_handle_t *handle = NULL; if (worker_start_internal(args_json, 0, resource_policy, single_thread, marker_file, - quarantine_file, log_callback, log_context, &handle) != 0) { + quarantine_file, log_callback, log_context, duration_origin_ms, + &handle) != 0) { return -1; } const cbm_index_worker_result_t *cached = NULL; @@ -1206,16 +1217,17 @@ int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_th const atomic_int *cancel_requested, cbm_index_worker_result_t *result) { return worker_spawn_internal(args_json, NULL, single_thread, marker_file, quarantine_file, - log_callback, log_context, cancel_requested, result); + log_callback, log_context, cancel_requested, 0, result); } int cbm_index_spawn_worker_with_policy_log_cancel( const char *args_json, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, - void *log_context, const atomic_int *cancel_requested, cbm_index_worker_result_t *result) { + void *log_context, const atomic_int *cancel_requested, uint64_t duration_origin_ms, + cbm_index_worker_result_t *result) { return worker_spawn_internal(args_json, resource_policy, single_thread, marker_file, quarantine_file, log_callback, log_context, cancel_requested, - result); + duration_origin_ms, result); } int cbm_index_spawn_worker_with_log(const char *args_json, bool single_thread, diff --git a/src/mcp/index_supervisor.h b/src/mcp/index_supervisor.h index a86761d77..2cb244a7e 100644 --- a/src/mcp/index_supervisor.h +++ b/src/mcp/index_supervisor.h @@ -186,19 +186,24 @@ int cbm_index_worker_start(const char *args_json, size_t memory_budget_bytes, bo int cbm_index_worker_start_with_policy(const char *args_json, size_t memory_budget_bytes, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, - const char *quarantine_file, + const char *quarantine_file, uint64_t duration_origin_ms, cbm_index_worker_handle_t **handle_out); /* Policy plus the request-scoped log callback. The daemon default start path * needs both: a missing policy fails closed, and worker diagnostics still - * reach the daemon journal. */ + * reach the daemon journal. duration_origin_ms is the request clock; 0 means now. */ int cbm_index_worker_start_with_policy_log(const char *args_json, size_t memory_budget_bytes, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, void *log_context, + uint64_t duration_origin_ms, cbm_index_worker_handle_t **handle_out); +/* Supervisor clock used to stamp a request-scoped duration origin. Tests may + * replace it; production is cbm_now_ms(). */ +uint64_t cbm_index_worker_now_ms(void); + /* Request-scoped variant used by interactive local CLI calls. The callback is * invoked by the owner thread while it polls the contained worker; log_context * remains caller-owned until terminal. No process-global sink is installed. @@ -264,7 +269,8 @@ int cbm_index_spawn_worker_with_log_cancel(const char *args_json, bool single_th int cbm_index_spawn_worker_with_policy_log_cancel( const char *args_json, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, const char *quarantine_file, cbm_proc_log_cb log_callback, - void *log_context, const atomic_int *cancel_requested, cbm_index_worker_result_t *result); + void *log_context, const atomic_int *cancel_requested, uint64_t duration_origin_ms, + cbm_index_worker_result_t *result); void cbm_index_worker_result_free(cbm_index_worker_result_t *result); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 1016f781a..f23a00e5c 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11617,12 +11617,14 @@ char *cbm_mcp_index_worker_no_response_failure(const char *args, static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, const cbm_index_resource_policy_t *resource_policy) { invalidate_cached_store(srv); + uint64_t request_started_ms = cbm_index_worker_now_ms(); /* First attempt: normal parallel run. */ cbm_index_worker_result_t wr; int rc = cbm_index_spawn_worker_with_policy_log_cancel( args, resource_policy, false, NULL, NULL, srv ? srv->index_log_callback : NULL, - srv ? srv->index_log_context : NULL, srv ? &srv->pipeline_cancel_requested : NULL, &wr); + srv ? srv->index_log_context : NULL, srv ? &srv->pipeline_cancel_requested : NULL, + request_started_ms, &wr); cbm_mcp_supervised_result_disposition_t disposition = cbm_mcp_supervised_result_disposition(rc, &wr); @@ -11704,7 +11706,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, int rc2 = cbm_index_spawn_worker_with_policy_log_cancel( args, resource_policy, /*single_thread=*/false, marker_path, quarantine_path, srv ? srv->index_log_callback : NULL, srv ? srv->index_log_context : NULL, - srv ? &srv->pipeline_cancel_requested : NULL, &wr2); + srv ? &srv->pipeline_cancel_requested : NULL, request_started_ms, &wr2); cbm_mcp_supervised_result_disposition_t recovery_disposition = cbm_mcp_supervised_result_disposition(rc2, &wr2); if (wr2.response_missing) { @@ -11831,7 +11833,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, int rcp = cbm_index_spawn_worker_with_policy_log_cancel( args, resource_policy, /*single_thread=*/false, NULL, quarantine_path, srv ? srv->index_log_callback : NULL, srv ? srv->index_log_context : NULL, - srv ? &srv->pipeline_cancel_requested : NULL, &wrp); + srv ? &srv->pipeline_cancel_requested : NULL, request_started_ms, &wrp); cbm_mcp_supervised_result_disposition_t partial_disposition = cbm_mcp_supervised_result_disposition(rcp, &wrp); if (partial_disposition == CBM_MCP_SUPERVISED_RESULT_SUCCESS) { diff --git a/tests/test_daemon_application.c b/tests/test_daemon_application.c index 5fe4e618f..5641ecbac 100644 --- a/tests/test_daemon_application.c +++ b/tests/test_daemon_application.c @@ -1703,7 +1703,9 @@ static void app_fake_worker_read_file(const char *path, char *out, size_t out_si static int app_fake_worker_start(void *opaque, const char *args_json, size_t memory_budget_bytes, const char *marker_file, const char *quarantine_file, + uint64_t duration_origin_ms, cbm_daemon_application_worker_t *worker_out) { + (void)duration_origin_ms; app_fake_worker_context_t *context = opaque; app_fake_worker_t *worker = calloc(1, sizeof(*worker)); if (!worker) { diff --git a/tests/test_index_supervisor.c b/tests/test_index_supervisor.c index 88d58c0d7..df55407da 100644 --- a/tests/test_index_supervisor.c +++ b/tests/test_index_supervisor.c @@ -689,8 +689,8 @@ TEST(index_supervisor_worker_keeps_default_info_liveness_heartbeat) { (void)cbm_unsetenv("CBM_LOG_LEVEL"); cbm_index_worker_handle_t *handle = NULL; - int start_rc = cbm_index_worker_start("{\"__cbm_test_worker\":\"heartbeat\"}", 0, false, - NULL, NULL, &handle); + int start_rc = cbm_index_worker_start("{\"__cbm_test_worker\":\"heartbeat\"}", 0, false, NULL, + NULL, &handle); char log_path[INDEX_SUPERVISOR_TEST_PATH_CAP] = {0}; if (handle) { (void)snprintf(log_path, sizeof(log_path), "%s", cbm_index_worker_log_path(handle)); @@ -698,8 +698,8 @@ TEST(index_supervisor_worker_keeps_default_info_liveness_heartbeat) { bool ready = log_path[0] && index_supervisor_test_wait_file_text( log_path, "async worker heartbeat probe ready", INDEX_SUPERVISOR_TEST_READY_MS); - bool heartbeat = ready && index_supervisor_test_wait_file_text( - log_path, "msg=pipeline.discover", 1000); + bool heartbeat = + ready && index_supervisor_test_wait_file_text(log_path, "msg=pipeline.discover", 1000); const cbm_index_worker_result_t *result = NULL; bool terminal = handle && index_supervisor_test_poll_terminal( handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); @@ -1034,8 +1034,9 @@ static uint64_t index_supervisor_fake_clock(void *context) { return fake->now_ms; } -static cbm_proc_tree_rss_status_t index_supervisor_fake_rss(cbm_subprocess_t *process, - uint64_t *rss_bytes, void *context) { +static cbm_proc_tree_rss_status_t index_supervisor_fake_charged_rss(cbm_subprocess_t *process, + uint64_t *rss_bytes, + void *context) { (void)process; index_supervisor_resource_fake_t *fake = context; int value_index = @@ -1065,11 +1066,11 @@ TEST(index_supervisor_disabled_limits_do_not_probe) { .rss_status = CBM_PROC_TREE_RSS_OK, }; cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, - index_supervisor_fake_rss, &fake); + index_supervisor_fake_charged_rss, &fake); cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 0); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"clean\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); const cbm_index_worker_result_t *result = NULL; bool terminal = handle && index_supervisor_test_poll_terminal( handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); @@ -1092,7 +1093,7 @@ TEST(index_supervisor_disabled_limits_do_not_probe) { PASS(); } -TEST(index_supervisor_rss_equality_runs_then_excess_terminates_tree) { +TEST(index_supervisor_charged_rss_equality_runs_then_excess_terminates_tree) { const uint64_t limit = UINT64_C(64) * CBM_INDEX_MIB_BYTES; index_supervisor_resource_fake_t fake = { .now_ms = 100, @@ -1101,11 +1102,11 @@ TEST(index_supervisor_rss_equality_runs_then_excess_terminates_tree) { .rss_status = CBM_PROC_TREE_RSS_OK, }; cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, - index_supervisor_fake_rss, &fake); + index_supervisor_fake_charged_rss, &fake); cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 0); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); const cbm_index_worker_result_t *result = NULL; cbm_index_worker_poll_t equal_state = handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; @@ -1141,11 +1142,11 @@ TEST(index_supervisor_duration_is_total_time_not_quiet_timeout) { .rss_status = CBM_PROC_TREE_RSS_EMPTY, }; cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, - index_supervisor_fake_rss, &fake); + index_supervisor_fake_charged_rss, &fake); cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); fake.now_ms = 1100; const cbm_index_worker_result_t *result = NULL; cbm_index_worker_poll_t equal_state = @@ -1171,6 +1172,60 @@ TEST(index_supervisor_duration_is_total_time_not_quiet_timeout) { PASS(); } +TEST(index_supervisor_duration_spans_recovery_attempts) { + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_EMPTY, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_charged_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); + cbm_index_worker_handle_t *first = NULL; + int first_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, 100, &first); + fake.now_ms = 500; + const cbm_index_worker_result_t *first_result = NULL; + cbm_index_worker_poll_t mid = + first ? cbm_index_worker_poll(first, &first_result) : CBM_INDEX_WORKER_POLL_ERROR; + bool cancel_accepted = first && cbm_index_worker_request_cancel(first); + bool first_terminal = first && index_supervisor_test_poll_terminal( + first, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &first_result); + if (first_terminal) { + cbm_index_worker_destroy(first); + first = NULL; + } else { + index_supervisor_test_cleanup_handle(first); + first = NULL; + } + + fake.now_ms = 1101; + cbm_index_worker_handle_t *second = NULL; + int second_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, NULL, NULL, 100, &second); + const cbm_index_worker_result_t *second_result = NULL; + bool second_terminal = second && index_supervisor_test_poll_terminal( + second, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &second_result); + bool limited = second_terminal && second_result && + second_result->resource_violation.resource == CBM_INDEX_RESOURCE_DURATION_MS && + second_result->resource_violation.observed == 1001 && + second_result->resource_violation.limit == 1000 && + second_result->outcome != CBM_PROC_HANG && second_result->tree_quiesced; + if (second_terminal) { + cbm_index_worker_destroy(second); + } else { + index_supervisor_test_cleanup_handle(second); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + + ASSERT_EQ(first_rc, 0); + ASSERT_EQ(mid, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(cancel_accepted); + ASSERT_TRUE(first_terminal); + ASSERT_EQ(second_rc, 0); + ASSERT_TRUE(limited); + PASS(); +} + TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled) { const char *saved_timeout = getenv("CBM_INDEX_WORKER_TIMEOUT_S"); char *saved_timeout_copy = saved_timeout ? cbm_strdup(saved_timeout) : NULL; @@ -1179,7 +1234,7 @@ TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled) { cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 60000); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); const cbm_index_worker_result_t *result = NULL; bool terminal = handle && index_supervisor_test_poll_terminal( handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); @@ -1208,11 +1263,11 @@ TEST(index_supervisor_cancel_precedes_resource_probe) { .rss_status = CBM_PROC_TREE_RSS_OK, }; cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, - index_supervisor_fake_rss, &fake); + index_supervisor_fake_charged_rss, &fake); cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 1000); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); fake.now_ms = 2000; bool cancel_accepted = handle && cbm_index_worker_request_cancel(handle); const cbm_index_worker_result_t *result = NULL; @@ -1242,11 +1297,11 @@ TEST(index_supervisor_three_failed_rss_probes_fail_closed) { .rss_status = CBM_PROC_TREE_RSS_ERROR, }; cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, - index_supervisor_fake_rss, &fake); + index_supervisor_fake_charged_rss, &fake); cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(limit, 0); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, &handle); + &policy, false, NULL, NULL, 0, &handle); const cbm_index_worker_result_t *result = NULL; cbm_index_worker_poll_t first = handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; @@ -1294,8 +1349,9 @@ SUITE(index_supervisor) { RUN_TEST(index_supervisor_clean_exit_without_response_is_named_failure_issue1300); RUN_TEST(index_supervisor_killed_worker_log_is_never_empty_and_names_the_run); RUN_TEST(index_supervisor_disabled_limits_do_not_probe); - RUN_TEST(index_supervisor_rss_equality_runs_then_excess_terminates_tree); + RUN_TEST(index_supervisor_charged_rss_equality_runs_then_excess_terminates_tree); RUN_TEST(index_supervisor_duration_is_total_time_not_quiet_timeout); + RUN_TEST(index_supervisor_duration_spans_recovery_attempts); RUN_TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled); RUN_TEST(index_supervisor_cancel_precedes_resource_probe); RUN_TEST(index_supervisor_three_failed_rss_probes_fail_closed); From 767ac3781c1f672d138ff63b4a4bb5dcc504c5bf Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:16:13 +0100 Subject: [PATCH 4/8] test(index): pin the duration clock in the quiet-timeout watchdog test index_supervisor_quiet_timeout_remains_hang_with_duration_enabled ran the duration watchdog on the real clock with a 60 s limit next to a 1 s quiet timeout. On a starved host the duration limit can be crossed before the quiet timeout fires, and the verdict then turns from HANG into a resource kill, so the test depended on how fast the machine was. The resource clock is now pinned at the request origin through the existing clock seam. The quiet timeout runs on the subprocess clock, so it is the only way the hung worker can end, however slowly the host runs. The limit is set equal to the quiet timeout on purpose: more real time than the limit always passes and it still cannot fire. A new assertion checks that the duration check read the pinned clock, so HANG wins against an armed watchdog. RED (real clock, limit crossed first): FAIL ASSERT(remained_hang) with index.supervisor.resource_limit resource=duration_ms. GREEN with the pin. Signed-off-by: Martin Vogel --- tests/test_index_supervisor.c | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/tests/test_index_supervisor.c b/tests/test_index_supervisor.c index df55407da..bda503238 100644 --- a/tests/test_index_supervisor.c +++ b/tests/test_index_supervisor.c @@ -1230,11 +1230,22 @@ TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled) { const char *saved_timeout = getenv("CBM_INDEX_WORKER_TIMEOUT_S"); char *saved_timeout_copy = saved_timeout ? cbm_strdup(saved_timeout) : NULL; (void)cbm_setenv("CBM_INDEX_WORKER_TIMEOUT_S", "1", 1); - cbm_index_supervisor_reset_resource_hooks_for_testing(); - cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 60000); + /* The duration watchdog stays armed, but its clock is pinned at the request + * origin, so the quiet timeout (the subprocess's own clock) is the only way + * this hung worker can end, however slowly the host runs. With the real + * clock a starved run could cross the duration limit first and turn the + * HANG into a resource kill. The limit equals the quiet timeout on purpose: + * more real time than the limit always passes, and it still cannot fire. */ + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_EMPTY, + }; + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_charged_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, - &policy, false, NULL, NULL, 0, &handle); + &policy, false, NULL, NULL, 100, &handle); const cbm_index_worker_result_t *result = NULL; bool terminal = handle && index_supervisor_test_poll_terminal( handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); @@ -1246,11 +1257,15 @@ TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled) { } else { index_supervisor_test_cleanup_handle(handle); } + cbm_index_supervisor_reset_resource_hooks_for_testing(); index_supervisor_test_restore_env("CBM_INDEX_WORKER_TIMEOUT_S", saved_timeout_copy); ASSERT_EQ(start_rc, 0); ASSERT_TRUE(terminal); ASSERT_TRUE(remained_hang); + /* The duration check really ran while polling (it read the pinned clock), + * so HANG won against a live watchdog, not against a disabled one. */ + ASSERT_TRUE(fake.clock_calls > 0); PASS(); } From 33ae8f382e20fa39f6854f85bdfdc5fdf9b1ec88 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:16:48 +0100 Subject: [PATCH 5/8] test(subprocess): prove descendant memory is measured on every platform subprocess_tree_rss_measures_contained_descendants asserted only that the tree total was above zero, which the root alone satisfies, and it was skipped on Windows although its skip text said it ran there. The test runner gains two roles: __cbm_rss_tree_root starts one __cbm_rss_tree_hold descendant (fork on POSIX, CreateProcessW into the same Job Object on Windows) that touches a controlled allocation and publishes its pid by an atomic rename. The test measures two otherwise identical trees, one whose descendant holds nothing and one whose descendant holds 128 MiB, and requires the second total to be at least 96 MiB higher. Only a probe that measures past the root can pass. The test now runs natively on Windows. The waits have no deadline: they end when the descendant is ready or the tree stops running, and the root exits when its descendant dies. Binding check: with the macOS probe reduced to the root only, the test fails (the loaded tree is not 96 MiB above the idle one). GREEN on the real probe (idle 43 MiB, loaded 187 MiB under ASan). Signed-off-by: Martin Vogel --- tests/test_main.c | 97 ++++++++++++++++++++++++++ tests/test_subprocess.c | 150 ++++++++++++++++++++++++++++++---------- 2 files changed, 212 insertions(+), 35 deletions(-) diff --git a/tests/test_main.c b/tests/test_main.c index e0526cf85..23b2621ee 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -44,6 +44,7 @@ int tf_deselected_count = 0; #include #include "foundation/win_utf8.h" /* cbm_module_path_utf8 — the runner's own image */ #else +#include /* waitpid: the process-tree probe root */ #include #ifdef __APPLE__ #include @@ -1968,6 +1969,100 @@ static int tf_role_env_isolation(int argc, char **argv) { return 0; } +/* Process-tree memory probe coverage (test_subprocess.c). The root starts ONE + * descendant that charges a controlled amount of memory, so a tree total can + * be checked against an allocation only a descendant holds. The descendant + * publishes its pid by an atomic rename once every page is touched, then + * blocks until it is killed. The root exits when its descendant dies, so a + * failed setup ends the tree instead of leaving the test waiting. */ +static int tf_rss_tree_hold(const char *ready_path, const char *mib_text) { + char *end = NULL; + long mib = strtol(mib_text, &end, 10); + if (!end || *end != '\0' || mib < 0 || mib > 1024) { + return 81; + } + size_t bytes = (size_t)mib * 1024U * 1024U; + unsigned char *block = bytes > 0 ? malloc(bytes) : NULL; + if (bytes > 0 && !block) { + return 82; + } + if (block) { + memset(block, 0xA5, bytes); + } + char staged[1024]; + int length = snprintf(staged, sizeof(staged), "%s.tmp", ready_path); + FILE *ready = length > 0 && (size_t)length < sizeof(staged) ? cbm_fopen(staged, "wb") : NULL; + if (!ready) { + return 83; + } +#ifdef _WIN32 + (void)fprintf(ready, "%lu\n", (unsigned long)GetCurrentProcessId()); +#else + (void)fprintf(ready, "%ld\n", (long)getpid()); +#endif + if (fclose(ready) != 0 || rename(staged, ready_path) != 0) { + return 84; + } + volatile unsigned char keep = block ? block[bytes - 1] : 0; + (void)keep; + for (;;) { +#ifdef _WIN32 + Sleep(INFINITE); +#else + (void)pause(); +#endif + } +} + +static int tf_role_rss_tree_hold(int argc, char **argv) { + return argc == 4 ? tf_rss_tree_hold(argv[2], argv[3]) : -1; +} + +static int tf_role_rss_tree_root(int argc, char **argv) { + if (argc != 4) { + return -1; + } +#ifdef _WIN32 + char *self = cbm_module_path_utf8(); + const char *child_argv[] = {self, "__cbm_rss_tree_hold", argv[2], argv[3], NULL}; + char command_line[8192]; + bool built = self && cbm_build_win_cmdline(command_line, sizeof(command_line), child_argv); + wchar_t *wide_self = built ? cbm_utf8_to_wide(self) : NULL; + wchar_t *wide_command = built ? cbm_utf8_to_wide(command_line) : NULL; + STARTUPINFOW startup; + ZeroMemory(&startup, sizeof(startup)); + startup.cb = sizeof(startup); + PROCESS_INFORMATION child; + ZeroMemory(&child, sizeof(child)); + /* The child joins this process's Job Object by default, which is the tree + * the probe under test enumerates. */ + BOOL started = wide_self && wide_command && + CreateProcessW(wide_self, wide_command, NULL, NULL, FALSE, 0, NULL, NULL, + &startup, &child); + free(wide_self); + free(wide_command); + free(self); + if (!started) { + return 85; + } + CloseHandle(child.hThread); + (void)WaitForSingleObject(child.hProcess, INFINITE); + CloseHandle(child.hProcess); +#else + pid_t child = fork(); + if (child < 0) { + return 85; + } + if (child == 0) { + _exit(tf_rss_tree_hold(argv[2], argv[3])); + } + int status = 0; + while (waitpid(child, &status, 0) < 0 && errno == EINTR) { + } +#endif + return 86; /* the descendant ended: end the tree */ +} + /* Installation tests use this executable as a structurally real candidate. * Mirror the production binary's minimal verification contract. */ static int tf_role_version(int argc, char **argv) { @@ -2060,6 +2155,8 @@ typedef struct { static const tf_role_t tf_roles[] = { TF_ROLE(TF_PHASE_RAW, "__cbm_env_values", tf_role_env_values), TF_ROLE(TF_PHASE_RAW, "__cbm_env_isolation", tf_role_env_isolation), + TF_ROLE(TF_PHASE_RAW, "__cbm_rss_tree_root", tf_role_rss_tree_root), + TF_ROLE(TF_PHASE_RAW, "__cbm_rss_tree_hold", tf_role_rss_tree_hold), TF_ROLE(TF_PHASE_GIT_CLEAN, "__cbm_subprocess_stdin_probe", tf_role_subprocess_stdin_probe), TF_ROLE(TF_PHASE_GIT_CLEAN, "__cbm_subprocess_stdout_probe", tf_role_subprocess_stdout_probe), TF_ROLE(TF_PHASE_GIT_CLEAN, "__cbm_git_facts_diff_probe", tf_role_git_facts_diff_probe), diff --git a/tests/test_subprocess.c b/tests/test_subprocess.c index 385293e08..1c5039e87 100644 --- a/tests/test_subprocess.c +++ b/tests/test_subprocess.c @@ -807,43 +807,123 @@ TEST(subprocess_spawn_returns_while_child_is_running) { #endif } -TEST(subprocess_tree_rss_measures_contained_descendants) { -#ifdef _WIN32 - SKIP_PLATFORM("native Job Object process-tree RSS runs on Windows CI"); -#else - char pid_path[64]; - ASSERT_TRUE(make_tree_pid_path(pid_path)); - cbm_subprocess_t *process = NULL; - ASSERT_EQ(spawn_ignoring_tree(pid_path, 0, 100, &process), 0); - pid_t parent_pid = 0; - pid_t grandchild_pid = 0; - bool ready = wait_for_tree_pids(pid_path, process, &parent_pid, &grandchild_pid, 3000); - uint64_t rss_bytes = 0; - cbm_proc_tree_rss_status_t rss_status = - ready ? cbm_subprocess_tree_rss_bytes(process, &rss_bytes) : CBM_PROC_TREE_RSS_ERROR; - bool cancel_accepted = cbm_subprocess_request_cancel(process); - cbm_proc_result_t result; - bool terminal = poll_until_terminal(process, 5000, &result); - uint64_t final_rss = UINT64_MAX; - cbm_proc_tree_rss_status_t final_rss_status = - terminal ? cbm_subprocess_tree_rss_bytes(process, &final_rss) : CBM_PROC_TREE_RSS_ERROR; - if (terminal) { - cbm_subprocess_destroy(process); - } else { - force_probe_cleanup(parent_pid, grandchild_pid); - } - (void)unlink(pid_path); +/* ── Process-tree memory probe (every platform) ───────────────────────────── + * The test runner's __cbm_rss_tree_root role starts one __cbm_rss_tree_hold + * descendant that touches a controlled allocation and then publishes its pid. + * Waits below have no deadline: they end when the descendant is ready or the + * tree stops running, so the outcome, not a timer, decides the verdict. */ - ASSERT_TRUE(ready); - ASSERT_EQ(rss_status, CBM_PROC_TREE_RSS_OK); - ASSERT_TRUE(rss_bytes > 0); - ASSERT_TRUE(cancel_accepted); - ASSERT_TRUE(terminal); - ASSERT_TRUE(result.tree_quiesced); - ASSERT_EQ(final_rss_status, CBM_PROC_TREE_RSS_EMPTY); - ASSERT_EQ(final_rss, 0); +#define RSS_TREE_MIB UINT64_C(1048576) + +typedef struct { + bool spawned; + bool ready; + cbm_proc_tree_rss_status_t status; + uint64_t rss; + bool quiesced; + cbm_proc_tree_rss_status_t final_status; + uint64_t final_rss; +} rss_tree_sample_t; + +static void rss_tree_pause(void) { + cbm_usleep(10000); /* poll interval only; nothing here is a deadline */ +} + +static bool rss_tree_wait_ready(const char *ready_path, cbm_subprocess_t *process) { + for (;;) { + FILE *ready = cbm_fopen(ready_path, "rb"); + if (ready) { + long pid = 0; + int fields = fscanf(ready, "%ld", &pid); + (void)fclose(ready); + if (fields == 1 && pid > 0) { + return true; + } + } + cbm_proc_result_t ignored; + if (cbm_subprocess_poll(process, &ignored) != CBM_PROC_POLL_RUNNING) { + return false; + } + rss_tree_pause(); + } +} + +static bool rss_tree_finish(cbm_subprocess_t *process, rss_tree_sample_t *sample) { + (void)cbm_subprocess_request_cancel(process); + cbm_proc_result_t result; + for (;;) { + cbm_proc_poll_t state = cbm_subprocess_poll(process, &result); + if (state == CBM_PROC_POLL_TERMINAL) { + break; + } + if (state == CBM_PROC_POLL_ERROR) { + return false; + } + rss_tree_pause(); + } + sample->final_status = cbm_subprocess_tree_rss_bytes(process, &sample->final_rss); + cbm_subprocess_destroy(process); + return result.tree_quiesced; +} + +/* Measure one tree whose descendant holds `mib` MiB. */ +static rss_tree_sample_t rss_tree_sample(int mib) { + rss_tree_sample_t sample = {.status = CBM_PROC_TREE_RSS_ERROR, + .final_status = CBM_PROC_TREE_RSS_ERROR, + .final_rss = UINT64_MAX}; + const char *made = th_mktempdir("cbm_rss_tree"); + char dir[256]; + char ready_path[300]; + char mib_text[16]; + if (!made || snprintf(dir, sizeof(dir), "%s", made) >= (int)sizeof(dir) || + snprintf(ready_path, sizeof(ready_path), "%s/ready", dir) >= (int)sizeof(ready_path)) { + return sample; + } + (void)snprintf(mib_text, sizeof(mib_text), "%d", mib); + const char *argv[] = {cbm_test_program_path, "__cbm_rss_tree_root", ready_path, mib_text, + NULL}; + cbm_proc_opts_t opts = {0}; + opts.bin = cbm_test_program_path; + opts.argv = argv; + opts.cancel_grace_ms = 100; + cbm_subprocess_t *process = NULL; + sample.spawned = cbm_subprocess_spawn(&opts, &process) == 0; + if (sample.spawned) { + sample.ready = rss_tree_wait_ready(ready_path, process); + if (sample.ready) { + sample.status = cbm_subprocess_tree_rss_bytes(process, &sample.rss); + } + sample.quiesced = rss_tree_finish(process, &sample); + } + th_cleanup(dir); + return sample; +} + +/* Only the DESCENDANT allocates, so the difference between two otherwise + * identical trees is memory the probe can only have counted by measuring past + * the root. 128 MiB touched against a 96 MiB floor leaves room for allocator + * and page-rounding noise and none for a root-only total. */ +TEST(subprocess_tree_rss_measures_contained_descendants) { + rss_tree_sample_t idle = rss_tree_sample(0); + rss_tree_sample_t loaded = rss_tree_sample(128); + if (loaded.status == CBM_PROC_TREE_RSS_OK && idle.status == CBM_PROC_TREE_RSS_OK) { + printf(" tree rss: idle=%llu MiB loaded=%llu MiB\n", + (unsigned long long)(idle.rss / RSS_TREE_MIB), + (unsigned long long)(loaded.rss / RSS_TREE_MIB)); + } + ASSERT_TRUE(idle.spawned); + ASSERT_TRUE(idle.ready); + ASSERT_EQ(idle.status, CBM_PROC_TREE_RSS_OK); + ASSERT_TRUE(idle.rss > 0); + ASSERT_TRUE(loaded.spawned); + ASSERT_TRUE(loaded.ready); + ASSERT_EQ(loaded.status, CBM_PROC_TREE_RSS_OK); + ASSERT_TRUE(loaded.rss >= idle.rss + 96 * RSS_TREE_MIB); + ASSERT_TRUE(idle.quiesced); + ASSERT_TRUE(loaded.quiesced); + ASSERT_EQ(loaded.final_status, CBM_PROC_TREE_RSS_EMPTY); + ASSERT_EQ(loaded.final_rss, 0); PASS(); -#endif } TEST(subprocess_natural_completion_is_cached_across_polls) { From c78c077a507100201a067b1ed49588df4d06f1f7 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 00:27:33 +0100 Subject: [PATCH 6/8] fix(subprocess): fail the tree probe on a live member it cannot measure cbm_subprocess_tree_rss_bytes skipped any non-root process it could not measure and still returned OK with a smaller total, so a descendant the platform refused to measure silently dropped out of the RSS watchdog. Now a member is skipped only when the platform says it no longer exists: ESRCH from proc_pid_rusage on macOS, ENOENT or ESRCH for /proc//stat on Linux, ERROR_INVALID_PARAMETER from OpenProcess (or an exit code from a handle whose memory query failed) on Windows. Any other failure fails the probe, which the supervisor already turns into resource_probe_failed after three consecutive errors. The root stays strict as before. On macOS the members now come from proc_listpids(PROC_PGRP_ONLY): the old all-process scan read every process with proc_pidinfo, which answers EPERM for every process of another user (178 of 6438 on this machine), so it could not tell a foreign process from an unreadable member and fail-closed semantics would have failed every probe there. Linux keeps its /proc scan; stat files are world-readable on a default mount, and a hidepid=1 mount now fails closed, which the doc states. A test seam makes every non-root measurement fail with one platform error. RED (old skip-any-failure decision): the live-member test returns OK where ERROR is required. GREEN with the fix; the exited-member test passes in both, so verified exit races are still ignored. Signed-off-by: Martin Vogel --- docs/INDEX_RESOURCE_LIMITS.md | 6 +- src/foundation/subprocess.c | 208 +++++++++++++++++++++++++--------- src/foundation/subprocess.h | 9 +- tests/test_subprocess.c | 47 +++++++- 4 files changed, 208 insertions(+), 62 deletions(-) diff --git a/docs/INDEX_RESOURCE_LIMITS.md b/docs/INDEX_RESOURCE_LIMITS.md index 152599360..11fcf4a3d 100644 --- a/docs/INDEX_RESOURCE_LIMITS.md +++ b/docs/INDEX_RESOURCE_LIMITS.md @@ -60,7 +60,11 @@ quarantine a source file. If RSS is enabled and three consecutive probes cannot obtain any trustworthy tree measurement while the root worker is still running, CBM fails closed with -`code=resource_probe_failed`. +`code=resource_probe_failed`. A probe is untrustworthy as soon as one member of +the tree cannot be measured: only a process that exits between the listing and +its measurement is skipped, because a total without a live member would +under-report. On Linux the probe reads every `/proc//stat` to find the +tree's members, so a `/proc` mounted with `hidepid=1` makes it fail closed. ## Counting and failure semantics diff --git a/src/foundation/subprocess.c b/src/foundation/subprocess.c index 519a554e2..bce08c6bf 100644 --- a/src/foundation/subprocess.c +++ b/src/foundation/subprocess.c @@ -476,9 +476,53 @@ uint64_t cbm_subprocess_rss_sum_for_testing(const uint64_t *values, size_t count } return total; } + +static atomic_int g_tree_rss_member_fault; + +void cbm_subprocess_tree_rss_fault_members_for_testing(int platform_error) { + atomic_store_explicit(&g_tree_rss_member_fault, platform_error, memory_order_release); +} + +/* Test builds can make every NON-root member's measurement fail with one + * platform error (errno on POSIX, GetLastError() on Windows); 0 = real call. + * Production builds compile the check out entirely. */ +static int cbm_tree_rss_member_fault(bool root) { + return root ? 0 : atomic_load_explicit(&g_tree_rss_member_fault, memory_order_acquire); +} #endif +/* A member that cannot be measured is skipped only when the platform says it + * no longer exists (it exited between the listing and the measurement, and + * holds nothing). Any other failure is a live member whose memory a total + * would silently leave out, so the probe fails instead -- a watchdog that + * under-reports is one that quietly stopped watching. The root is strict + * either way. */ #ifdef _WIN32 +static HANDLE cbm_tree_rss_open_member(DWORD pid, bool root, DWORD *error_out) { +#ifdef CBM_ENABLE_TEST_SEAMS + int fault = cbm_tree_rss_member_fault(root); + if (fault != 0) { + *error_out = (DWORD)fault; + return NULL; + } +#else + (void)root; +#endif + /* GetProcessMemoryInfo documents PROCESS_QUERY_INFORMATION | + * PROCESS_VM_READ. PROCESS_QUERY_LIMITED_INFORMATION is a lesser + * alternative on newer Windows; the Job Object membership list + * already closes most of the pid-reuse window between listing and + * opening. */ + HANDLE member = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid); + *error_out = member ? ERROR_SUCCESS : GetLastError(); + return member; +} + +static bool cbm_tree_rss_member_exited(HANDLE member) { + DWORD exit_code = 0; + return GetExitCodeProcess(member, &exit_code) && exit_code != STILL_ACTIVE; +} + static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, uint64_t *rss_bytes) { DWORD capacity = 32; @@ -510,18 +554,15 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces uint64_t total = 0; DWORD measured = 0; - bool root_failed = false; - for (DWORD index = 0; index < processes->NumberOfProcessIdsInList; index++) { + bool failed = false; + for (DWORD index = 0; index < processes->NumberOfProcessIdsInList && !failed; index++) { DWORD pid = (DWORD)processes->ProcessIdList[index]; - /* GetProcessMemoryInfo documents PROCESS_QUERY_INFORMATION | - * PROCESS_VM_READ. PROCESS_QUERY_LIMITED_INFORMATION is a lesser - * alternative on newer Windows; the Job Object membership list - * already closes most of the pid-reuse window between listing and - * opening. A member we cannot open is a probe failure when it is - * the root. */ - HANDLE member = OpenProcess(PROCESS_QUERY_INFORMATION | PROCESS_VM_READ, FALSE, pid); + bool root = pid == process->process_id; + DWORD open_error = ERROR_SUCCESS; + HANDLE member = cbm_tree_rss_open_member(pid, root, &open_error); if (!member) { - root_failed = root_failed || pid == process->process_id; + /* ERROR_INVALID_PARAMETER: no process has this id any more. */ + failed = root || open_error != ERROR_INVALID_PARAMETER; continue; } PROCESS_MEMORY_COUNTERS memory; @@ -529,14 +570,14 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces if (GetProcessMemoryInfo(member, &memory, sizeof(memory))) { cbm_rss_add_saturated(&total, (uint64_t)memory.WorkingSetSize); measured++; - } else if (pid == process->process_id) { - root_failed = true; + } else { + failed = root || !cbm_tree_rss_member_exited(member); } CloseHandle(member); } DWORD listed = processes->NumberOfProcessIdsInList; cbm_free(CBM_MEM_CLASS_OTHER, processes); - if (root_failed || (measured == 0 && listed > 0)) { + if (failed || (measured == 0 && listed > 0)) { return CBM_PROC_TREE_RSS_ERROR; } if (measured == 0) { @@ -546,56 +587,73 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces return CBM_PROC_TREE_RSS_OK; } #elif defined(__APPLE__) -static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, - uint64_t *rss_bytes) { - int capacity = proc_listallpids(NULL, 0); - if (capacity <= 0 || capacity > INT_MAX / (int)sizeof(pid_t) - 64) { - return CBM_PROC_TREE_RSS_ERROR; - } - capacity += 64; +/* The members of the contained process group, as the kernel lists them. An + * all-process scan cannot decide membership: proc_pidinfo answers EPERM for + * every process of another user, so "not in our group" and "in our group but + * unreadable" would look the same. Returns the member count, or -1. */ +static int cbm_tree_rss_list_group(pid_t pgid, pid_t **pids_out) { + int capacity = 64; pid_t *pids = NULL; - int count = 0; for (;;) { pid_t *grown = cbm_realloc(CBM_MEM_CLASS_OTHER, pids, (size_t)capacity * sizeof(*pids)); if (!grown) { cbm_free(CBM_MEM_CLASS_OTHER, pids); - return CBM_PROC_TREE_RSS_ERROR; + return -1; } pids = grown; - count = proc_listallpids(pids, capacity * (int)sizeof(*pids)); - if (count <= 0) { + errno = 0; + int bytes = + proc_listpids(PROC_PGRP_ONLY, (uint32_t)pgid, pids, capacity * (int)sizeof(*pids)); + if (bytes < 0 || (bytes == 0 && errno != 0)) { cbm_free(CBM_MEM_CLASS_OTHER, pids); - return CBM_PROC_TREE_RSS_ERROR; + return -1; } + int count = bytes / (int)sizeof(*pids); if (count < capacity) { - break; + *pids_out = pids; + return count; } if (capacity > INT_MAX / (int)sizeof(*pids) / 2) { cbm_free(CBM_MEM_CLASS_OTHER, pids); - return CBM_PROC_TREE_RSS_ERROR; + return -1; } capacity *= 2; } +} + +static int cbm_tree_rss_member_rusage(pid_t pid, bool root, struct rusage_info_v2 *usage) { +#ifdef CBM_ENABLE_TEST_SEAMS + int fault = cbm_tree_rss_member_fault(root); + if (fault != 0) { + errno = fault; + return -1; + } +#else + (void)root; +#endif + return proc_pid_rusage(pid, RUSAGE_INFO_V2, (rusage_info_t *)usage); +} + +static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, + uint64_t *rss_bytes) { + pid_t *pids = NULL; + int count = cbm_tree_rss_list_group(process->pgid, &pids); + if (count < 0) { + return CBM_PROC_TREE_RSS_ERROR; + } uint64_t total = 0; int measured = 0; - bool root_failed = false; - for (int index = 0; index < count; index++) { - struct proc_bsdinfo info; + bool failed = false; + for (int index = 0; index < count && !failed; index++) { if (pids[index] <= 0) { continue; } - if (proc_pidinfo(pids[index], PROC_PIDTBSDINFO, 0, &info, sizeof(info)) != - (int)sizeof(info)) { - root_failed = root_failed || pids[index] == process->pid; - continue; - } - if ((pid_t)info.pbi_pgid != process->pgid) { - continue; - } + bool root = pids[index] == process->pid; struct rusage_info_v2 usage; - if (proc_pid_rusage(pids[index], RUSAGE_INFO_V2, (rusage_info_t *)&usage) != 0) { - root_failed = root_failed || pids[index] == process->pid; + if (cbm_tree_rss_member_rusage(pids[index], root, &usage) != 0) { + /* ESRCH: the member exited after the listing. */ + failed = root || errno != ESRCH; continue; } /* phys_footprint is what the OS charges, matching cbm_mem_charged(). @@ -606,7 +664,7 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces measured++; } cbm_free(CBM_MEM_CLASS_OTHER, pids); - if (root_failed) { + if (failed) { return CBM_PROC_TREE_RSS_ERROR; } if (measured == 0) { @@ -628,22 +686,61 @@ static bool cbm_proc_pid_name(const char *name) { return true; } -static bool cbm_linux_proc_stat(const char *pid_name, pid_t *group, int64_t *rss_pages) { +typedef enum { + CBM_LINUX_STAT_OK = 0, + CBM_LINUX_STAT_GONE, /* the process exited after the /proc listing */ + CBM_LINUX_STAT_UNREADABLE, /* a process whose stat could not be read or parsed */ +} cbm_linux_stat_t; + +/* /proc//stat is world-readable, so on a default /proc mount the only + * expected failure is the exit race: ENOENT at open, ESRCH at read. Anything + * else (EACCES under hidepid=1, a parse failure) leaves the process's group + * unknown, and the probe fails closed rather than guess it is not ours. */ +static cbm_linux_stat_t cbm_linux_stat_failure(int error) { + return error == ENOENT || error == ESRCH ? CBM_LINUX_STAT_GONE : CBM_LINUX_STAT_UNREADABLE; +} + +static cbm_linux_stat_t cbm_linux_proc_stat_line(const char *pid_name, bool root, char *line, + size_t line_size) { char path[64]; int length = snprintf(path, sizeof(path), "/proc/%s/stat", pid_name); if (length <= 0 || length >= (int)sizeof(path)) { - return false; + return CBM_LINUX_STAT_UNREADABLE; } +#ifdef CBM_ENABLE_TEST_SEAMS + int fault = cbm_tree_rss_member_fault(root); + if (fault != 0) { + return cbm_linux_stat_failure(fault); + } +#else + (void)root; +#endif + errno = 0; FILE *stat_file = cbm_fopen(path, "rb"); if (!stat_file) { - return false; + return cbm_linux_stat_failure(errno); } - char stat_line[4096]; - bool read = fgets(stat_line, sizeof(stat_line), stat_file) != NULL; + errno = 0; + bool read = fgets(line, (int)line_size, stat_file) != NULL; + int read_error = errno; (void)fclose(stat_file); - char *command_end = read ? strrchr(stat_line, ')') : NULL; + if (!read) { + return read_error == ESRCH ? CBM_LINUX_STAT_GONE : CBM_LINUX_STAT_UNREADABLE; + } + return CBM_LINUX_STAT_OK; +} + +static cbm_linux_stat_t cbm_linux_proc_stat(const char *pid_name, bool root, pid_t *group, + int64_t *rss_pages) { + char stat_line[4096]; + cbm_linux_stat_t status = + cbm_linux_proc_stat_line(pid_name, root, stat_line, sizeof(stat_line)); + if (status != CBM_LINUX_STAT_OK) { + return status; + } + char *command_end = strrchr(stat_line, ')'); if (!command_end || command_end[1] != ' ') { - return false; + return CBM_LINUX_STAT_UNREADABLE; } char *save = NULL; @@ -656,7 +753,7 @@ static bool cbm_linux_proc_stat(const char *pid_name, pid_t *group, int64_t *rss char *end = NULL; long long value = strtoll(token, &end, 10); if (!end || *end != '\0') { - return false; + return CBM_LINUX_STAT_UNREADABLE; } if (field == 5) { *group = (pid_t)value; @@ -670,7 +767,7 @@ static bool cbm_linux_proc_stat(const char *pid_name, pid_t *group, int64_t *rss token = strtok_r(NULL, " ", &save); field++; } - return have_group && have_rss; + return have_group && have_rss ? CBM_LINUX_STAT_OK : CBM_LINUX_STAT_UNREADABLE; } static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subprocess_t *process, @@ -687,17 +784,18 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces uint64_t total = 0; int measured = 0; - bool root_failed = false; + bool failed = false; struct dirent *entry; - while ((entry = readdir(proc)) != NULL) { + while (!failed && (entry = readdir(proc)) != NULL) { if (!cbm_proc_pid_name(entry->d_name)) { continue; } pid_t group = 0; int64_t pages = 0; bool root_entry = strtol(entry->d_name, NULL, 10) == (long)process->pid; - if (!cbm_linux_proc_stat(entry->d_name, &group, &pages)) { - root_failed = root_failed || root_entry; + cbm_linux_stat_t status = cbm_linux_proc_stat(entry->d_name, root_entry, &group, &pages); + if (status != CBM_LINUX_STAT_OK) { + failed = root_entry || status != CBM_LINUX_STAT_GONE; continue; } if (group != process->pgid || pages < 0) { @@ -711,7 +809,7 @@ static cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_platform(cbm_subproces measured++; } (void)closedir(proc); - if (root_failed) { + if (failed) { return CBM_PROC_TREE_RSS_ERROR; } if (measured == 0) { diff --git a/src/foundation/subprocess.h b/src/foundation/subprocess.h index 94a8a2ed0..a5b44474a 100644 --- a/src/foundation/subprocess.h +++ b/src/foundation/subprocess.h @@ -159,8 +159,9 @@ cbm_proc_poll_t cbm_subprocess_poll(cbm_subprocess_t *process, cbm_proc_result_t * On macOS this is phys_footprint (what the OS holds against the process); * elsewhere it is RSS. OK returns an overflow-safe byte total, EMPTY means * the owned tree currently has no observable members, and ERROR means no - * trustworthy measurement could be obtained. Individual processes that exit - * during enumeration are ignored. */ + * trustworthy measurement could be obtained. A member that exits between the + * listing and its measurement is ignored (the platform reports it gone); any + * other member that cannot be measured is ERROR, never a smaller total. */ cbm_proc_tree_rss_status_t cbm_subprocess_tree_rss_bytes(cbm_subprocess_t *process, uint64_t *rss_bytes); bool cbm_subprocess_root_running(const cbm_subprocess_t *process); @@ -243,6 +244,10 @@ void cbm_subprocess_hold_parent_until_child_exits_for_testing(int spawns); * waitid does not report it yet. Test builds only. */ void cbm_subprocess_observe_exiting_child_for_testing(int spawns); uint64_t cbm_subprocess_rss_sum_for_testing(const uint64_t *values, size_t count); +/* Make the tree probe's measurement of every NON-root member fail with + * platform_error (errno on POSIX, GetLastError() on Windows), as a live or an + * exiting member would; 0 restores the real calls. Test builds only. */ +void cbm_subprocess_tree_rss_fault_members_for_testing(int platform_error); #endif /* How the POSIX fork+exec child closed its inherited descriptors (#1484). */ diff --git a/tests/test_subprocess.c b/tests/test_subprocess.c index 1c5039e87..e214a2c36 100644 --- a/tests/test_subprocess.c +++ b/tests/test_subprocess.c @@ -813,6 +813,17 @@ TEST(subprocess_spawn_returns_while_child_is_running) { * Waits below have no deadline: they end when the descendant is ready or the * tree stops running, so the outcome, not a timer, decides the verdict. */ +#ifdef _WIN32 +#define RSS_TREE_FAULT_GONE ERROR_INVALID_PARAMETER /* no process with that id any more */ +#define RSS_TREE_FAULT_LIVE ERROR_ACCESS_DENIED /* a member we may not open */ +#elif defined(__APPLE__) +#define RSS_TREE_FAULT_GONE ESRCH +#define RSS_TREE_FAULT_LIVE EPERM +#else +#define RSS_TREE_FAULT_GONE ENOENT +#define RSS_TREE_FAULT_LIVE EACCES +#endif + #define RSS_TREE_MIB UINT64_C(1048576) typedef struct { @@ -866,8 +877,9 @@ static bool rss_tree_finish(cbm_subprocess_t *process, rss_tree_sample_t *sample return result.tree_quiesced; } -/* Measure one tree whose descendant holds `mib` MiB. */ -static rss_tree_sample_t rss_tree_sample(int mib) { +/* Measure one tree whose descendant holds `mib` MiB, with every non-root + * member's measurement failing with `member_fault` (0 = real calls). */ +static rss_tree_sample_t rss_tree_sample(int mib, int member_fault) { rss_tree_sample_t sample = {.status = CBM_PROC_TREE_RSS_ERROR, .final_status = CBM_PROC_TREE_RSS_ERROR, .final_rss = UINT64_MAX}; @@ -891,7 +903,9 @@ static rss_tree_sample_t rss_tree_sample(int mib) { if (sample.spawned) { sample.ready = rss_tree_wait_ready(ready_path, process); if (sample.ready) { + cbm_subprocess_tree_rss_fault_members_for_testing(member_fault); sample.status = cbm_subprocess_tree_rss_bytes(process, &sample.rss); + cbm_subprocess_tree_rss_fault_members_for_testing(0); } sample.quiesced = rss_tree_finish(process, &sample); } @@ -904,8 +918,8 @@ static rss_tree_sample_t rss_tree_sample(int mib) { * the root. 128 MiB touched against a 96 MiB floor leaves room for allocator * and page-rounding noise and none for a root-only total. */ TEST(subprocess_tree_rss_measures_contained_descendants) { - rss_tree_sample_t idle = rss_tree_sample(0); - rss_tree_sample_t loaded = rss_tree_sample(128); + rss_tree_sample_t idle = rss_tree_sample(0, 0); + rss_tree_sample_t loaded = rss_tree_sample(128, 0); if (loaded.status == CBM_PROC_TREE_RSS_OK && idle.status == CBM_PROC_TREE_RSS_OK) { printf(" tree rss: idle=%llu MiB loaded=%llu MiB\n", (unsigned long long)(idle.rss / RSS_TREE_MIB), @@ -926,6 +940,29 @@ TEST(subprocess_tree_rss_measures_contained_descendants) { PASS(); } +/* A live member the platform will not let us measure must fail the probe: a + * total without it would under-report the tree. */ +TEST(subprocess_tree_rss_fails_when_a_live_member_cannot_be_measured) { + rss_tree_sample_t sample = rss_tree_sample(0, RSS_TREE_FAULT_LIVE); + ASSERT_TRUE(sample.spawned); + ASSERT_TRUE(sample.ready); + ASSERT_EQ(sample.status, CBM_PROC_TREE_RSS_ERROR); + ASSERT_TRUE(sample.quiesced); + PASS(); +} + +/* A member that exited between the listing and its measurement holds nothing: + * it is skipped and the rest of the tree (here the root) is still measured. */ +TEST(subprocess_tree_rss_skips_a_member_that_exited_mid_probe) { + rss_tree_sample_t sample = rss_tree_sample(0, RSS_TREE_FAULT_GONE); + ASSERT_TRUE(sample.spawned); + ASSERT_TRUE(sample.ready); + ASSERT_EQ(sample.status, CBM_PROC_TREE_RSS_OK); + ASSERT_TRUE(sample.rss > 0); + ASSERT_TRUE(sample.quiesced); + PASS(); +} + TEST(subprocess_natural_completion_is_cached_across_polls) { #ifdef _WIN32 SKIP_PLATFORM("POSIX /bin/sh completion-cache probe; native Job Object coverage pending"); @@ -2775,6 +2812,8 @@ SUITE(subprocess) { RUN_TEST(subprocess_run_null_bin_rejected); RUN_TEST(subprocess_spawn_returns_while_child_is_running); RUN_TEST(subprocess_tree_rss_measures_contained_descendants); + RUN_TEST(subprocess_tree_rss_fails_when_a_live_member_cannot_be_measured); + RUN_TEST(subprocess_tree_rss_skips_a_member_that_exited_mid_probe); RUN_TEST(subprocess_natural_completion_is_cached_across_polls); RUN_TEST(subprocess_cancel_is_idempotent_and_kills_ignoring_tree); RUN_TEST(subprocess_quiet_timeout_kills_ignoring_tree); From 435a88be6271a392f22487e41b5e2f5867e78313 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Mon, 2 Nov 2026 18:12:22 +0100 Subject: [PATCH 7/8] fix(index): coordinate publication between the worker and its supervisor A worker resource limit is enforced by the supervisor, but only the worker replaces the serving database, and it does so before the persistence artifact export. A limit observed in that window stopped a worker that had already published, and the response still said serving_index_preserved=true while the previous index had been replaced. Publication and resource termination now coordinate through a per-attempt publication fence: a never-created path next to a reserved unique file in the private worker directory, which both sides claim by creating it exclusively (cbm_fopen "wbx": O_EXCL on POSIX, CREATE_NEW on Windows), so exactly one of them can win. - The worker claims it through a new pipeline publication gate that cbm_pipeline_run calls once, after the last pre-publication check and before the serving database is touched. Losing means the supervisor already stopped the attempt for a limit: the run aborts before publication and the previous generation stays in place. - The supervisor claims it before it terminates the worker for a limit or a failed RSS probe. Losing means the worker already committed to publishing: the limit is no longer enforced for that attempt, which runs to completion, and index.supervisor.resource_limit_after_publication is logged instead. A claim that can be neither won nor lost is retried on the next poll rather than risk stopping a committed worker. - At terminal the supervisor settles the fence into result.publication_committed. An unquiesced tree keeps the fence, so a surviving member can never publish after the report. serving_index_preserved now describes the request, not only its last attempt: a servable index existed when the request began, no attempt of the request committed to publishing (an earlier attempt can publish and then crash into recovery), and that index is still servable. The direct path and the daemon job both fold every attempt into these request facts; cbm_mcp_index_worker_resource_response takes them as a third argument, and NULL claims nothing. The fence path travels as an optional last pair of the exact worker grammar (--index-worker-publish-fence), accepted by the worker argv parser and by the daemon bootstrap role check, stripped by the CLI and installed by both worker entries. Limits stay opt-in: without index_max_rss_mb or index_max_duration_seconds there is no fence, no extra argv, no claim and no extra database read. Graph content is unchanged either way. RED (c78c077a plus only the new end-to-end test): a FIFO at the artifact temp path holds the real worker after the rename, and the duration clock crosses its limit only once the serving database has been replaced. The supervisor stops the worker there and answers resource_limit_exceeded with serving_index_preserved=true over a replaced database. GREEN: the worker claimed first, the supervisor logs the limit as not enforced, and the released worker finishes and reports the published index. Signed-off-by: Martin Vogel --- docs/CONFIGURATION.md | 2 +- docs/INDEX_RESOURCE_LIMITS.md | 17 +- src/daemon/application.c | 24 ++- src/daemon/bootstrap.c | 6 + src/main.c | 4 + src/mcp/index_supervisor.c | 214 ++++++++++++++++++++++++- src/mcp/index_supervisor.h | 37 +++++ src/mcp/mcp.c | 70 +++++++-- src/mcp/mcp_internal.h | 21 ++- src/pipeline/pipeline.c | 22 +++ src/pipeline/pipeline.h | 11 ++ tests/test_index_supervisor.c | 225 ++++++++++++++++++++++++++- tests/test_main.c | 7 +- tests/test_mcp.c | 285 ++++++++++++++++++++++++++++++++++ tests/test_pipeline.c | 67 ++++++++ 15 files changed, 992 insertions(+), 20 deletions(-) diff --git a/docs/CONFIGURATION.md b/docs/CONFIGURATION.md index a02459b4a..2f0d6717a 100644 --- a/docs/CONFIGURATION.md +++ b/docs/CONFIGURATION.md @@ -94,7 +94,7 @@ Current keys: | `index_max_files` | `off` | Optional maximum number of accepted source files in one discovery run. | | `index_max_source_mb` | `off` | Optional maximum accepted source size in MiB in one discovery run. | | `index_max_rss_mb` | `off` | Optional maximum charged memory in MiB for the complete contained index-worker process tree (`64..1048576`). On macOS this is phys_footprint, not resident size. | -| `index_max_duration_seconds` | `off` | Optional maximum wall-clock duration in seconds for the whole index request, including crash/hang recovery (`1..86400`). | +| `index_max_duration_seconds` | `off` | Optional maximum wall-clock duration in seconds for the whole index request, including crash/hang recovery (`1..86400`). Like `index_max_rss_mb`, it stops a worker only before it starts to publish the new index; see [INDEX_RESOURCE_LIMITS.md](INDEX_RESOURCE_LIMITS.md). | For a watched project, `index_status` names the polling method in `watch.strategy`: `git` (HEAD and dirty-state polling), `tree` (a non-git root polled by the diff --git a/docs/INDEX_RESOURCE_LIMITS.md b/docs/INDEX_RESOURCE_LIMITS.md index 11fcf4a3d..d76b162d7 100644 --- a/docs/INDEX_RESOURCE_LIMITS.md +++ b/docs/INDEX_RESOURCE_LIMITS.md @@ -58,6 +58,18 @@ terminal only after the tree is quiescent or a bounded containment failure is explicitly surfaced. Resource termination is not retried and does not quarantine a source file. +A worker limit never stops a worker after it has started to publish. The +supervisor enforces the limits, but only the worker replaces the serving +database, so the two coordinate through a per-attempt publication claim that +exactly one of them can win. The worker claims it immediately before it +replaces the serving database; the supervisor claims it before it stops the +worker for a limit. If the limit is reached first, the worker can no longer +publish and the previous index stays untouched. If the worker claimed first, +the limits no longer apply to that attempt: the new index is published and the +rest of the attempt, including the persistence artifact export, runs to +completion. The supervisor then logs `index.supervisor.resource_limit_after_publication` +instead of stopping it. With both worker limits `off` there is no claim. + If RSS is enabled and three consecutive probes cannot obtain any trustworthy tree measurement while the root worker is still running, CBM fails closed with `code=resource_probe_failed`. A probe is untrustworthy as soon as one member of @@ -102,7 +114,10 @@ Worker limit failures use the same shape with `stage=worker`, `resource=rss_bytes` and `unit=bytes`, or `resource=duration_ms` and `unit=milliseconds`. RSS measurement failures use `code=resource_probe_failed` and omit `observed`, `limit`, and `unit` because no trustworthy observation was -available. +available. For a worker failure `serving_index_preserved` describes the whole +request: it is true only when a servable index existed when the request began, +no attempt of the request (including an earlier one that crashed and was +recovered) claimed publication, and that index still serves. ## Trust and compatibility diff --git a/src/daemon/application.c b/src/daemon/application.c index 0f696d59c..207735e04 100644 --- a/src/daemon/application.c +++ b/src/daemon/application.c @@ -160,6 +160,10 @@ struct cbm_daemon_application_job { bool async_owned; bool worker_started; uint64_t request_started_ms; + /* What a worker resource failure of this job is reported against; set + * before the first attempt, folded over every attempt (mcp_internal.h). */ + bool request_facts_begun; + cbm_mcp_index_request_facts_t request_facts; cbm_daemon_application_job_t *next; }; @@ -1346,6 +1350,17 @@ static void application_cancelled_result(cbm_index_worker_result_t *result) { result->tree_quiesced = true; } +/* Only an enabled worker limit can end a job in a worker resource failure, the + * one response the request facts feed; without one nothing is read. */ +static void application_job_request_facts_begin(cbm_daemon_application_job_t *job) { + cbm_index_resource_policy_t policy; + char error[CBM_SZ_256] = {0}; + if (cbm_mcp_index_policy_from_internal_args(job->args_json, &policy, error, sizeof(error)) && + cbm_index_policy_worker_enabled(&policy)) { + cbm_mcp_index_request_facts_begin(job->args_json, &job->request_facts); + } +} + static application_attempt_status_t application_job_run_attempt(cbm_daemon_application_job_t *job, const char *marker_path, const char *quarantine_path, @@ -1382,6 +1397,10 @@ static application_attempt_status_t application_job_run_attempt(cbm_daemon_appli if (job->request_started_ms == 0) { job->request_started_ms = cbm_index_worker_now_ms(); } + if (!job->request_facts_begun) { + job->request_facts_begun = true; + application_job_request_facts_begin(job); + } application_tmp_lock(); int start_result = application->worker_ops.start( application->worker_ops.context, job->args_json, memory_budget_bytes, marker_path, @@ -1573,6 +1592,7 @@ static application_attempt_decision_t application_consume_attempt( application_attempt_disposition(job, attempt); application_record_attempt(attempt, &execution->last_result, &execution->have_last_result, execution->last_log); + cbm_mcp_index_request_facts_record(&job->request_facts, &attempt->result); if (disposition == CBM_MCP_SUPERVISED_RESULT_SUCCESS) { execution->response = attempt->result.response; attempt->result.response = NULL; @@ -1581,8 +1601,8 @@ static application_attempt_decision_t application_consume_attempt( return APPLICATION_ATTEMPT_DECISION_SUCCESS; } if (disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { - execution->response = - cbm_mcp_index_worker_resource_response(job->args_json, &attempt->result); + execution->response = cbm_mcp_index_worker_resource_response( + job->args_json, &attempt->result, &job->request_facts); application_attempt_free(attempt); return APPLICATION_ATTEMPT_DECISION_STOP; } diff --git a/src/daemon/bootstrap.c b/src/daemon/bootstrap.c index cb384cd20..617b9c06f 100644 --- a/src/daemon/bootstrap.c +++ b/src/daemon/bootstrap.c @@ -141,6 +141,12 @@ static bool bootstrap_worker_argv_exact(int argc, char *const argv[]) { } next += 2; } + if (next < argc && bootstrap_arg_is(argv[next], "--index-worker-publish-fence")) { + if (next + 1 >= argc || !argv[next + 1] || !argv[next + 1][0]) { + return false; + } + next += 2; + } return next == argc; } diff --git a/src/main.c b/src/main.c index 54bad875e..82be46e03 100644 --- a/src/main.c +++ b/src/main.c @@ -897,6 +897,8 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo const char *worker_marker = cli_strip_flag_value(&argc, argv, CBM_INDEX_WORKER_MARKER_ARG); const char *worker_quarantine = cli_strip_flag_value(&argc, argv, CBM_INDEX_WORKER_QUARANTINE_ARG); + const char *worker_publish_fence = + cli_strip_flag_value(&argc, argv, CBM_INDEX_WORKER_PUBLISH_FENCE_ARG); if (index_worker) { /* The graph lives on this process's heaps: SQLite gets heaps of its * own here, and only here (cbm_sqlite_dedicated_heap). */ @@ -906,6 +908,7 @@ static int run_cli(int argc, char **argv, cbm_project_lock_manager_t *project_lo cbm_index_set_worker_role_options(index_worker, response_out, worker_single_thread, worker_marker, worker_quarantine, cbm_index_worker_memory_budget_bytes()); + cbm_index_set_worker_publish_fence(index_worker ? worker_publish_fence : NULL); if (argc < MAIN_MIN_ARGC) { (void)fprintf(stderr, CLI_USAGE); @@ -3188,6 +3191,7 @@ int main(int argc, char **argv) { cbm_index_set_worker_role_options(true, invocation.response_out, invocation.single_thread, invocation.marker_file, invocation.quarantine_file, invocation.memory_budget_bytes); + cbm_index_set_worker_publish_fence(invocation.publish_fence_file); #ifndef _WIN32 /* Split into three ordered steps rather than one condition, because the * ORDER is load-bearing and the middle step only exists in test builds: diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index e84dff755..c8c4224ad 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -92,6 +92,63 @@ size_t cbm_index_worker_memory_budget_bytes(void) { return g_worker_memory_budget_bytes; } +/* ── Publication fence (see index_supervisor.h) ───────────────────── */ + +typedef enum { + INDEX_PUBLICATION_CLAIMED = 0, /* this caller created the fence file */ + INDEX_PUBLICATION_TAKEN, /* the other side created it first */ + INDEX_PUBLICATION_CLAIM_ERROR, /* neither could be established */ +} index_publication_claim_t; + +/* Exclusive creation is the single atomic decision both processes race for: + * "x" is O_CREAT|O_EXCL on POSIX and CREATE_NEW on Windows, so exactly one + * creator succeeds and the other gets EEXIST. A deletion-based claim would + * be ambiguous on Windows, where a delete-pending file reports access denied. */ +static index_publication_claim_t index_publication_claim(const char *fence_file) { + if (!fence_file || !fence_file[0]) { + return INDEX_PUBLICATION_CLAIM_ERROR; + } + errno = 0; + FILE *claim = cbm_fopen(fence_file, "wbx"); + if (claim) { + (void)fclose(claim); + return INDEX_PUBLICATION_CLAIMED; + } + return errno == EEXIST ? INDEX_PUBLICATION_TAKEN : INDEX_PUBLICATION_CLAIM_ERROR; +} + +static char g_worker_publish_fence[CBM_SZ_4K] = {0}; +static bool g_worker_publication_claimed = false; + +void cbm_index_set_worker_publish_fence(const char *fence_file) { + if (fence_file && fence_file[0]) { + (void)snprintf(g_worker_publish_fence, sizeof(g_worker_publish_fence), "%s", fence_file); + } else { + g_worker_publish_fence[0] = '\0'; + } + g_worker_publication_claimed = false; +} + +const char *cbm_index_worker_publish_fence(void) { + return g_worker_publish_fence[0] ? g_worker_publish_fence : NULL; +} + +bool cbm_index_worker_claim_publication(void) { + if (!g_worker_publish_fence[0] || g_worker_publication_claimed) { + return true; + } + index_publication_claim_t claim = index_publication_claim(g_worker_publish_fence); + if (claim == INDEX_PUBLICATION_CLAIMED) { + g_worker_publication_claimed = true; + cbm_log_info("index.worker.publication_claimed", "fence", g_worker_publish_fence); + return true; + } + cbm_log_warn("index.worker.publication_refused", "reason", + claim == INDEX_PUBLICATION_TAKEN ? "resource_limit" : "claim_failed", "fence", + g_worker_publish_fence); + return false; +} + /* Worker log startup header — see index_supervisor.h. */ static atomic_flag g_worker_log_begun = ATOMIC_FLAG_INIT; @@ -250,6 +307,13 @@ cbm_index_worker_argv_status_t cbm_index_worker_parse_process_argv( parsed.quarantine_file = argv[next + 1]; next += 2; } + if (next < argc && argv[next] && strcmp(argv[next], CBM_INDEX_WORKER_PUBLISH_FENCE_ARG) == 0) { + if (next + 1 >= argc || !argv[next + 1] || !argv[next + 1][0]) { + return CBM_INDEX_WORKER_ARGV_INVALID; + } + parsed.publish_fence_file = argv[next + 1]; + next += 2; + } if (next != argc) { return CBM_INDEX_WORKER_ARGV_INVALID; } @@ -477,7 +541,7 @@ static void worker_log_last_phase(const char *log_path, char *out, size_t out_si enum { INDEX_WORKER_PATH_CAP = CBM_SZ_4K, - INDEX_WORKER_ARGV_CAP = 17, + INDEX_WORKER_ARGV_CAP = 19, INDEX_WORKER_SYNC_POLL_NS = 10000000, INDEX_WORKER_RELAY_LINES_PER_POLL = 64, INDEX_WORKER_RELAY_BYTES_PER_POLL = 64 * 1024, @@ -492,6 +556,14 @@ typedef enum { INDEX_WORKER_TERMINATION_RESOURCE, } index_worker_termination_reason_t; +/* Who owns the publication fence of one attempt. Only the polling owner thread + * reads or writes it. */ +typedef enum { + INDEX_WORKER_PUBLICATION_OPEN = 0, /* unclaimed, or no fence at all */ + INDEX_WORKER_PUBLICATION_SUPERVISOR, + INDEX_WORKER_PUBLICATION_WORKER, +} index_worker_publication_owner_t; + #ifdef CBM_ENABLE_TEST_SEAMS static cbm_index_supervisor_clock_fn g_resource_clock; static cbm_index_supervisor_rss_fn g_resource_rss; @@ -545,11 +617,24 @@ struct cbm_index_worker_handle { uint64_t last_rss_probe_ms; unsigned int rss_probe_failures; bool rss_probe_started; + /* Publication fence: a never-created path next to a reserved unique file + * (both removed at terminal). Empty when no worker limit is enabled. */ + char publish_fence_reserve[INDEX_WORKER_PATH_CAP]; + char publish_fence_path[INDEX_WORKER_PATH_CAP]; + index_worker_publication_owner_t publication_owner; + bool publication_claim_error_logged; atomic_int termination_reason; atomic_bool terminal; cbm_index_worker_result_t result; }; +#ifdef CBM_ENABLE_TEST_SEAMS +const char *cbm_index_worker_publish_fence_path_for_testing( + const cbm_index_worker_handle_t *handle) { + return handle && handle->publish_fence_path[0] ? handle->publish_fence_path : NULL; +} +#endif + /* The generic subprocess supervisor tails logs in bounded batches to preserve * its nonblocking poll contract. A short-lived worker can therefore become * terminal with more completed lines still on disk. Keep the request callback @@ -801,9 +886,58 @@ size_t cbm_index_worker_job_memory_limit(size_t memory_budget_bytes) { return memory_budget_bytes > SIZE_MAX - headroom ? SIZE_MAX : memory_budget_bytes + headroom; } +/* Claim the publication fence before stopping the worker for a resource limit + * (see index_supervisor.h). True when the termination may proceed. */ +static bool worker_claim_publication_for_limit(cbm_index_worker_handle_t *handle, + cbm_index_resource_t resource, uint64_t observed, + uint64_t limit, bool probe_failed) { + if (!handle->publish_fence_path[0] || + handle->publication_owner == INDEX_WORKER_PUBLICATION_SUPERVISOR) { + return true; + } + if (handle->publication_owner == INDEX_WORKER_PUBLICATION_WORKER) { + return false; + } + index_publication_claim_t claim = index_publication_claim(handle->publish_fence_path); + if (claim == INDEX_PUBLICATION_CLAIMED) { + handle->publication_owner = INDEX_WORKER_PUBLICATION_SUPERVISOR; + return true; + } + if (claim == INDEX_PUBLICATION_TAKEN) { + /* The worker committed to publishing before the limit was observed. + * Stopping it now could leave a new generation serving behind a report + * that the previous index was preserved, so this attempt is no longer + * limited; the crossing is recorded here instead. */ + handle->publication_owner = INDEX_WORKER_PUBLICATION_WORKER; + char observed_text[CBM_SZ_32]; + char limit_text[CBM_SZ_32]; + (void)snprintf(observed_text, sizeof(observed_text), "%llu", (unsigned long long)observed); + (void)snprintf(limit_text, sizeof(limit_text), "%llu", (unsigned long long)limit); + cbm_log_warn("index.supervisor.resource_limit_after_publication", "resource", + cbm_index_resource_name(resource), "observed", + probe_failed ? "unmeasured" : observed_text, "limit", limit_text, "action", + "not_enforced", "log", handle->log_path); + return false; + } + /* Neither side holds the fence and it could not be created. Terminating + * now could race a later successful claim by the worker, so the decision + * is retried on the next poll instead. */ + if (!handle->publication_claim_error_logged) { + handle->publication_claim_error_logged = true; + char errno_text[CBM_SZ_32]; + (void)snprintf(errno_text, sizeof(errno_text), "%d", errno); + cbm_log_error("index.supervisor.publication_claim_failed", "errno", errno_text, "action", + "retry_next_poll", "log", handle->log_path); + } + return false; +} + static void worker_request_resource_termination(cbm_index_worker_handle_t *handle, cbm_index_resource_t resource, uint64_t observed, uint64_t limit, bool probe_failed) { + if (!worker_claim_publication_for_limit(handle, resource, observed, limit, probe_failed)) { + return; + } int expected = INDEX_WORKER_TERMINATION_NONE; if (!atomic_compare_exchange_strong_explicit(&handle->termination_reason, &expected, INDEX_WORKER_TERMINATION_RESOURCE, @@ -824,7 +958,9 @@ static void worker_request_resource_termination(cbm_index_worker_handle_t *handl } static bool worker_check_resource_limits(cbm_index_worker_handle_t *handle) { - if (atomic_load_explicit(&handle->termination_reason, memory_order_acquire) != + /* A worker that committed to publishing is no longer resource-limited. */ + if (handle->publication_owner == INDEX_WORKER_PUBLICATION_WORKER || + atomic_load_explicit(&handle->termination_reason, memory_order_acquire) != INDEX_WORKER_TERMINATION_NONE || cbm_subprocess_termination_pending(handle->process) || !cbm_subprocess_supervision_active(handle->process)) { @@ -877,6 +1013,56 @@ duration_check: return rss_probe_failed; } +/* Reserve a unique private name and derive the fence path from it. The fence + * itself is never created here: creating it IS the claim. */ +static bool worker_publish_fence_create(cbm_index_worker_handle_t *handle) { + if (!worker_unique_file(handle->publish_fence_reserve, sizeof(handle->publish_fence_reserve), + "publish")) { + return false; + } + int written = snprintf(handle->publish_fence_path, sizeof(handle->publish_fence_path), + "%s.claim", handle->publish_fence_reserve); + if (written <= 0 || written >= (int)sizeof(handle->publish_fence_path)) { + (void)cbm_unlink(handle->publish_fence_reserve); + handle->publish_fence_reserve[0] = '\0'; + handle->publish_fence_path[0] = '\0'; + return false; + } + return true; +} + +static void worker_publish_fence_remove(cbm_index_worker_handle_t *handle) { + if (handle->publish_fence_path[0]) { + (void)cbm_unlink(handle->publish_fence_path); + } + if (handle->publish_fence_reserve[0]) { + (void)cbm_unlink(handle->publish_fence_reserve); + } +} + +/* Settle who held the fence once the attempt is terminal. Claiming an unowned + * fence now also bars any later claim, so a stray member of an unquiesced tree + * can never publish after this report. */ +static bool worker_publication_committed_at_terminal(cbm_index_worker_handle_t *handle) { + if (!handle->publish_fence_path[0] || + handle->publication_owner == INDEX_WORKER_PUBLICATION_SUPERVISOR) { + return false; + } + if (handle->publication_owner == INDEX_WORKER_PUBLICATION_WORKER) { + return true; + } + index_publication_claim_t claim = index_publication_claim(handle->publish_fence_path); + if (claim == INDEX_PUBLICATION_CLAIMED) { + handle->publication_owner = INDEX_WORKER_PUBLICATION_SUPERVISOR; + return false; + } + if (claim == INDEX_PUBLICATION_TAKEN) { + handle->publication_owner = INDEX_WORKER_PUBLICATION_WORKER; + } + /* Taken: the worker committed. Unreadable: preservation cannot be proven. */ + return true; +} + static int worker_start_internal(const char *args_json, size_t memory_budget_bytes, const cbm_index_resource_policy_t *resource_policy, bool single_thread, const char *marker_file, @@ -935,6 +1121,19 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt free(handle); return -1; } + /* Opt-in: only an enabled worker limit can stop a worker, so only then is + * there a publication to coordinate. */ + if (cbm_index_policy_worker_enabled(&handle->resource_policy) && + !worker_publish_fence_create(handle)) { + char error_text[CBM_SZ_32]; + (void)snprintf(error_text, sizeof(error_text), "%d", errno); + cbm_log_error("index.supervisor.artifact_create_failed", "artifact", "publish_fence", + "errno", error_text); + (void)cbm_unlink(handle->response_path); + (void)cbm_unlink(handle->log_path); + free(handle); + return -1; + } const char *argv[INDEX_WORKER_ARGV_CAP]; size_t argc = 0; @@ -964,6 +1163,10 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt argv[argc++] = CBM_INDEX_WORKER_QUARANTINE_ARG; argv[argc++] = quarantine_file; } + if (handle->publish_fence_path[0]) { + argv[argc++] = CBM_INDEX_WORKER_PUBLISH_FENCE_ARG; + argv[argc++] = handle->publish_fence_path; + } argv[argc] = NULL; cbm_proc_opts_t options = {0}; @@ -981,6 +1184,7 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt if (cbm_subprocess_spawn(&options, &handle->process) != 0) { (void)cbm_unlink(handle->response_path); (void)cbm_unlink(handle->log_path); + worker_publish_fence_remove(handle); free(handle); cbm_log_error("index.supervisor.spawn_failed", "action", "fail_closed"); return -1; @@ -1089,6 +1293,12 @@ cbm_index_worker_poll_t cbm_index_worker_poll(cbm_index_worker_handle_t *handle, handle->result.forced = process_result->forced; handle->result.tree_quiesced = process_result->tree_quiesced; handle->result.supervision_failed = process_result->supervision_failed; + handle->result.publication_committed = worker_publication_committed_at_terminal(handle); + if (handle->result.tree_quiesced) { + /* An unquiesced tree keeps the fence as it is: removing a claim the + * supervisor holds would let a surviving member publish later. */ + worker_publish_fence_remove(handle); + } if (worker_result_succeeded(&handle->result)) { worker_response_read_status_t response_status; handle->result.response = slurp_worker_response(handle->response_path, &response_status); diff --git a/src/mcp/index_supervisor.h b/src/mcp/index_supervisor.h index 2cb244a7e..359a3e25e 100644 --- a/src/mcp/index_supervisor.h +++ b/src/mcp/index_supervisor.h @@ -42,6 +42,7 @@ void cbm_index_set_worker_role(bool is_worker, const char *response_out); #define CBM_INDEX_WORKER_QUARANTINE_ARG "--index-worker-quarantine" #define CBM_INDEX_WORKER_MEMORY_BUDGET_ARG "--index-worker-memory-budget-bytes" #define CBM_INDEX_WORKER_BUILD_ARG "--index-worker-build" +#define CBM_INDEX_WORKER_PUBLISH_FENCE_ARG "--index-worker-publish-fence" #define CBM_INDEX_WORKER_BUILD_FINGERPRINT_LENGTH 64U #define CBM_INDEX_WORKER_BUILD_FINGERPRINT_SIZE 65U void cbm_index_set_worker_role_options(bool is_worker, const char *response_out, bool single_thread, @@ -51,6 +52,32 @@ bool cbm_index_worker_active(void); const char *cbm_index_worker_response_out(void); size_t cbm_index_worker_memory_budget_bytes(void); +/* Publication fence. A worker resource limit (index_max_rss_mb, + * index_max_duration_seconds) is enforced by the supervisor, while only the + * worker replaces the serving database. When such a limit is enabled, every + * worker attempt gets a fence file path, and publication and resource + * termination both CLAIM it by creating that file exclusively, so exactly one + * of them can win: + * - the worker claims immediately before it replaces the serving database; + * losing means the supervisor already decided to stop it for a limit, so + * the run aborts before publication and the previous index stays intact; + * - the supervisor claims before it terminates the worker for a limit; + * losing means the worker already committed to publishing, so the limit is + * no longer enforced for that attempt and the attempt runs to completion. + * A reported resource failure therefore never follows a publication by the + * same attempt. Without an enabled worker limit there is no fence and nothing + * changes. The supervisor passes the path on the worker argv; the worker entry + * installs it here (NULL clears it). */ +void cbm_index_set_worker_publish_fence(const char *fence_file); +const char *cbm_index_worker_publish_fence(void); + +/* Worker side: claim publication. True when publishing may proceed: no fence + * is installed, or this call created the fence file. False when the + * supervisor already claimed it (a resource limit stopped this attempt) or the + * claim could not be made; the caller must then leave the serving database + * untouched. Idempotent after a successful claim. */ +bool cbm_index_worker_claim_publication(void); + /* Event name of the worker log's startup header. Shared with the tests so the * contract has exactly one spelling. */ #define CBM_INDEX_WORKER_LOG_START_EVENT "index.worker.start" @@ -96,6 +123,7 @@ typedef struct { const char *marker_file; const char *quarantine_file; size_t memory_budget_bytes; + const char *publish_fence_file; } cbm_index_worker_invocation_t; typedef enum { @@ -160,6 +188,12 @@ typedef struct { bool response_rejected; /* clean worker exceeded the bounded response protocol */ cbm_index_resource_violation_t resource_violation; bool resource_probe_failed; + /* The worker claimed the publication fence: it committed to replacing the + * serving database (whether or not it got that far). Also true when the + * fence state could not be read, because a report may only claim the + * previous index was preserved when it can prove it. Always false without + * a fence (no worker limit enabled) and for an attempt stopped by a limit. */ + bool publication_committed; char *response; /* worker result only after a contained, uncancelled CLEAN exit; * borrowed for async polls, caller-owned from the sync wrapper */ } cbm_index_worker_result_t; @@ -283,6 +317,9 @@ void cbm_index_supervisor_set_resource_hooks_for_testing(cbm_index_supervisor_cl cbm_index_supervisor_rss_fn rss_fn, void *context); void cbm_index_supervisor_reset_resource_hooks_for_testing(void); +/* Borrowed fence path of an attempt; NULL when it has no fence. */ +const char *cbm_index_worker_publish_fence_path_for_testing( + const cbm_index_worker_handle_t *handle); #endif #endif /* CBM_INDEX_SUPERVISOR_H */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index f23a00e5c..3dc5e72ee 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11277,16 +11277,50 @@ static char *build_worker_unsafe_terminal_response(const char *args, cbm_proc_ou return index_worker_json_result(doc, repo_path, NULL, NULL); } -char *cbm_mcp_index_worker_resource_response(const char *args, - const cbm_index_worker_result_t *worker_result) { +/* Whether the project named by internal worker args has a servable index now. */ +static bool index_args_serving_index_servable(const char *args) { char *repo_path = cbm_mcp_get_string_arg(args, "repo_path"); char *name_override = cbm_mcp_get_string_arg(args, "name"); char *project_name = cbm_project_name_from_path(name_override && name_override[0] ? name_override : repo_path); - char db_path[CBM_SZ_1K] = {0}; + bool servable = false; if (project_name) { + char db_path[CBM_SZ_1K] = {0}; project_db_path(project_name, db_path, sizeof(db_path)); + servable = project_db_is_servable(project_name, db_path); } + free(repo_path); + free(name_override); + free(project_name); + return servable; +} + +void cbm_mcp_index_request_facts_begin(const char *args, cbm_mcp_index_request_facts_t *facts) { + if (!facts) { + return; + } + facts->serving_index_existed = index_args_serving_index_servable(args); + facts->publication_committed = false; +} + +void cbm_mcp_index_request_facts_record(cbm_mcp_index_request_facts_t *facts, + const cbm_index_worker_result_t *worker_result) { + if (facts && worker_result && worker_result->publication_committed) { + facts->publication_committed = true; + } +} + +char *cbm_mcp_index_worker_resource_response(const char *args, + const cbm_index_worker_result_t *worker_result, + const cbm_mcp_index_request_facts_t *facts) { + char *repo_path = cbm_mcp_get_string_arg(args, "repo_path"); + /* The publication fence makes a limit-stopped attempt and a publication by + * that attempt mutually exclusive; an EARLIER attempt of the request may + * still have published (it crashed after committing), and an index that + * appeared during the request (artifact bootstrap) was not preserved. */ + bool serving_index_preserved = + facts && facts->serving_index_existed && !facts->publication_committed && + !worker_result->publication_committed && index_args_serving_index_servable(args); const cbm_index_resource_violation_t *violation = &worker_result->resource_violation; const char *config_key = cbm_index_resource_config_key(violation->resource); @@ -11311,13 +11345,12 @@ char *cbm_mcp_index_worker_resource_response(const char *args, yyjson_mut_obj_add_str(doc, root, "unit", cbm_index_resource_unit(violation->resource)); } yyjson_mut_obj_add_bool(doc, root, "retryable", true); - yyjson_mut_obj_add_bool(doc, root, "serving_index_preserved", - project_name && project_db_is_servable(project_name, db_path)); + yyjson_mut_obj_add_bool(doc, root, "serving_index_preserved", serving_index_preserved); yyjson_mut_obj_add_strcpy(doc, root, "message", message); if (repo_path) { yyjson_mut_obj_add_strcpy(doc, root, "repo_path", repo_path); } - return index_worker_json_result(doc, repo_path, name_override, project_name); + return index_worker_json_result(doc, repo_path, NULL, NULL); } /* Drop the cached store so the next query reopens whatever the worker wrote (each @@ -11618,6 +11651,12 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, const cbm_index_resource_policy_t *resource_policy) { invalidate_cached_store(srv); uint64_t request_started_ms = cbm_index_worker_now_ms(); + /* Only an enabled worker limit can produce a worker resource failure, the + * one response these facts are reported in. */ + cbm_mcp_index_request_facts_t facts = {0}; + if (cbm_index_policy_worker_enabled(resource_policy)) { + cbm_mcp_index_request_facts_begin(args, &facts); + } /* First attempt: normal parallel run. */ cbm_index_worker_result_t wr; @@ -11627,6 +11666,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, request_started_ms, &wr); cbm_mcp_supervised_result_disposition_t disposition = cbm_mcp_supervised_result_disposition(rc, &wr); + cbm_mcp_index_request_facts_record(&facts, &wr); if (wr.response_missing) { /* #1300: a clean exit without a response is a named failure. */ @@ -11649,7 +11689,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, return failure; } if (disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { - char *failure = cbm_mcp_index_worker_resource_response(args, &wr); + char *failure = cbm_mcp_index_worker_resource_response(args, &wr, &facts); cbm_index_worker_result_free(&wr); invalidate_cached_store(srv); return failure; @@ -11709,6 +11749,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, srv ? &srv->pipeline_cancel_requested : NULL, request_started_ms, &wr2); cbm_mcp_supervised_result_disposition_t recovery_disposition = cbm_mcp_supervised_result_disposition(rc2, &wr2); + cbm_mcp_index_request_facts_record(&facts, &wr2); if (wr2.response_missing) { /* #1300: a recovery run that exits cleanly without a response is * reported as such, not as a failed recovery start. */ @@ -11732,7 +11773,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, break; } if (recovery_disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { - resp = cbm_mcp_index_worker_resource_response(args, &wr2); + resp = cbm_mcp_index_worker_resource_response(args, &wr2, &facts); cbm_index_worker_result_free(&wr2); break; } @@ -11836,6 +11877,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, srv ? &srv->pipeline_cancel_requested : NULL, request_started_ms, &wrp); cbm_mcp_supervised_result_disposition_t partial_disposition = cbm_mcp_supervised_result_disposition(rcp, &wrp); + cbm_mcp_index_request_facts_record(&facts, &wrp); if (partial_disposition == CBM_MCP_SUPERVISED_RESULT_SUCCESS) { resp = wrp.response; /* transfer ownership to caller */ wrp.response = NULL; @@ -11848,7 +11890,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args, unsafe_terminal = true; terminal_cancelled = wrp.cancellation_requested; } else if (partial_disposition == CBM_MCP_SUPERVISED_RESULT_RESOURCE_FAILURE) { - resp = cbm_mcp_index_worker_resource_response(args, &wrp); + resp = cbm_mcp_index_worker_resource_response(args, &wrp, &facts); } cbm_index_worker_result_free(&wrp); } @@ -12293,6 +12335,11 @@ static char *handle_index_repository_status(cbm_mcp_server_t *srv, const char *a return result ? result : cbm_mcp_text_result("index job status is unavailable", true); } +static bool index_worker_publish_gate(void *context) { + (void)context; + return cbm_index_worker_claim_publication(); +} + static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { bool async_mode = false; bool status_mode = false; @@ -12492,6 +12539,11 @@ static char *handle_index_repository(cbm_mcp_server_t *srv, const char *args) { free(name_override); cbm_pipeline_set_persistence(p, persistence); cbm_pipeline_set_resource_policy(p, &resource_policy); + if (cbm_index_worker_publish_fence()) { + /* Supervised worker with a worker limit: claim publication from the + * supervisor before the serving database is replaced. */ + cbm_pipeline_set_publish_gate(p, index_worker_publish_gate, NULL); + } index_doc_candidates_apply(p, doc_candidates); char *project_name = heap_strdup(cbm_pipeline_project_name(p)); diff --git a/src/mcp/mcp_internal.h b/src/mcp/mcp_internal.h index 3ca7c6ee9..59e769dd9 100644 --- a/src/mcp/mcp_internal.h +++ b/src/mcp/mcp_internal.h @@ -62,8 +62,27 @@ bool cbm_mcp_index_policy_add_to_args(yyjson_mut_doc *doc, yyjson_mut_val *root, const cbm_index_resource_policy_t *policy); bool cbm_mcp_index_policy_from_internal_args(const char *args, cbm_index_resource_policy_t *policy, char *error, size_t error_size); + +/* Request-scoped facts a worker resource failure is reported against. One + * supervised request can run several worker attempts (crash/hang recovery), and + * serving_index_preserved describes the request, not only its last attempt: + * it is true only when a servable index served the project when the request + * began AND no attempt of the request committed to replacing it (the + * publication fence, index_supervisor.h) AND that index is still servable. */ +typedef struct { + bool serving_index_existed; + bool publication_committed; +} cbm_mcp_index_request_facts_t; + +/* Capture the facts at the start of a request, before its first worker. */ +void cbm_mcp_index_request_facts_begin(const char *args, cbm_mcp_index_request_facts_t *facts); +/* Fold one finished attempt into the request facts. */ +void cbm_mcp_index_request_facts_record(cbm_mcp_index_request_facts_t *facts, + const cbm_index_worker_result_t *worker_result); +/* NULL facts claim nothing: serving_index_preserved is then false. */ char *cbm_mcp_index_worker_resource_response(const char *args, - const cbm_index_worker_result_t *worker_result); + const cbm_index_worker_result_t *worker_result, + const cbm_mcp_index_request_facts_t *facts); /* Encode a doc_candidates setting the user set to a boolean on an internal * worker request (the worker reads no config). Unset or unparseable adds no diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index e14bfb7f7..f1965ad4b 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -303,6 +303,9 @@ struct cbm_pipeline { * per pipeline so concurrent test/process activity cannot cross-trigger. */ void (*before_publish_hook)(cbm_pipeline_t *, const char *, void *); void *before_publish_hook_ctx; + /* Production publication gate (cbm_pipeline_set_publish_gate). */ + cbm_pipeline_publish_gate_fn publish_gate; + void *publish_gate_ctx; /* Test-only: sees the complete graph buffer before the dump releases its indexes. */ void (*before_dump_hook)(const cbm_gbuf_t *, void *); void *before_dump_hook_ctx; @@ -897,6 +900,14 @@ void cbm_pipeline_set_persistence(cbm_pipeline_t *p, bool enabled) { } } +void cbm_pipeline_set_publish_gate(cbm_pipeline_t *p, cbm_pipeline_publish_gate_fn gate, + void *context) { + if (p) { + p->publish_gate = gate; + p->publish_gate_ctx = gate ? context : NULL; + } +} + void cbm_pipeline_set_doc_candidates_enabled(cbm_pipeline_t *p, bool enabled) { if (p) { p->doc_candidates_choice = @@ -4763,6 +4774,17 @@ int cbm_pipeline_run(cbm_pipeline_t *p) { return CBM_PIPELINE_ABORT_PRESERVE_DB; } + /* Last point at which the serving database is still untouched: the + * destination preparation below may already move it aside. */ + if (p->publish_gate && !p->publish_gate(p->publish_gate_ctx)) { + cbm_log_warn("pipeline.publish_refused", "path", final_path); + cleanup_staging_db(staging_path); + free(staging_path); + free(final_path); + pipeline_release_test_config(p); + return CBM_PIPELINE_ABORT_PRESERVE_DB; + } + cbm_replacement_prepare_t prepared = {0}; int prepare_rc = prepare_publish_destination(final_path, final_existed, backup_succeeded, &prepared); diff --git a/src/pipeline/pipeline.h b/src/pipeline/pipeline.h index 072674d8d..bbc856b39 100644 --- a/src/pipeline/pipeline.h +++ b/src/pipeline/pipeline.h @@ -56,6 +56,17 @@ void cbm_pipeline_set_persistence(cbm_pipeline_t *p, bool enabled); /* Apply a validated discovery resource policy. The value is copied. */ void cbm_pipeline_set_resource_policy(cbm_pipeline_t *p, const cbm_index_resource_policy_t *policy); +/* Publication gate. cbm_pipeline_run calls it once, after the last + * pre-publication check and immediately before the serving database is + * touched (never for a live no-op, which replaces nothing). Returning false + * aborts the run with CBM_PIPELINE_ABORT_PRESERVE_DB and the previous + * generation stays exactly as it was. A supervised index worker uses it to + * claim publication from its supervisor (see the publication fence in + * mcp/index_supervisor.h). NULL removes the gate. */ +typedef bool (*cbm_pipeline_publish_gate_fn)(void *context); +void cbm_pipeline_set_publish_gate(cbm_pipeline_t *p, cbm_pipeline_publish_gate_fn gate, + void *context); + /* Doc -> code candidates (the doc_candidates setting). Never set, the index * mode decides: on for full, off for moderate and fast. The setter makes an * explicit choice that wins in every mode. Off skips scoring documentation diff --git a/tests/test_index_supervisor.c b/tests/test_index_supervisor.c index bda503238..f3218fdb1 100644 --- a/tests/test_index_supervisor.c +++ b/tests/test_index_supervisor.c @@ -239,18 +239,59 @@ TEST(index_supervisor_worker_argv_requires_exact_build_bound_grammar) { "/tmp/m", CBM_INDEX_WORKER_QUARANTINE_ARG, "/tmp/q", + CBM_INDEX_WORKER_PUBLISH_FENCE_ARG, + "/tmp/f", NULL, }; cbm_index_worker_invocation_t invocation; - ASSERT_EQ(cbm_index_worker_parse_process_argv(16, valid, &invocation), + ASSERT_EQ(cbm_index_worker_parse_process_argv(18, valid, &invocation), CBM_INDEX_WORKER_ARGV_VALID); - ASSERT_EQ(cbm_daemon_process_role(16, valid), CBM_DAEMON_PROCESS_WORKER); + ASSERT_EQ(cbm_daemon_process_role(18, valid), CBM_DAEMON_PROCESS_WORKER); ASSERT_STR_EQ(invocation.args_json, valid[6]); ASSERT_STR_EQ(invocation.response_out, valid[8]); ASSERT_EQ(invocation.memory_budget_bytes, 1024); ASSERT_TRUE(invocation.single_thread); ASSERT_STR_EQ(invocation.marker_file, valid[13]); ASSERT_STR_EQ(invocation.quarantine_file, valid[15]); + ASSERT_STR_EQ(invocation.publish_fence_file, valid[17]); + /* Without a worker limit there is no fence: the shorter grammar stays valid. */ + ASSERT_EQ(cbm_index_worker_parse_process_argv(16, valid, &invocation), + CBM_INDEX_WORKER_ARGV_VALID); + ASSERT_EQ(cbm_daemon_process_role(16, valid), CBM_DAEMON_PROCESS_WORKER); + ASSERT_NULL(invocation.publish_fence_file); + + char *fence_before_quarantine[] = {"test-runner", + "cli", + "--index-worker", + CBM_INDEX_WORKER_BUILD_ARG, + (char *)captured, + "index_repository", + "{}", + "--response-out", + "/tmp/r", + CBM_INDEX_WORKER_PUBLISH_FENCE_ARG, + "/tmp/f", + CBM_INDEX_WORKER_QUARANTINE_ARG, + "/tmp/q", + NULL}; + char *empty_fence[] = {"test-runner", + "cli", + "--index-worker", + CBM_INDEX_WORKER_BUILD_ARG, + (char *)captured, + "index_repository", + "{}", + "--response-out", + "/tmp/r", + CBM_INDEX_WORKER_PUBLISH_FENCE_ARG, + "", + NULL}; + ASSERT_EQ(cbm_index_worker_parse_process_argv(13, fence_before_quarantine, &invocation), + CBM_INDEX_WORKER_ARGV_INVALID); + ASSERT_EQ(cbm_daemon_process_role(13, fence_before_quarantine), CBM_DAEMON_PROCESS_INVALID); + ASSERT_EQ(cbm_index_worker_parse_process_argv(11, empty_fence, &invocation), + CBM_INDEX_WORKER_ARGV_INVALID); + ASSERT_EQ(cbm_daemon_process_role(11, empty_fence), CBM_DAEMON_PROCESS_INVALID); char *missing_build[] = {"test-runner", "cli", "--index-worker", "index_repository", "{}", "--response-out", @@ -1071,12 +1112,15 @@ TEST(index_supervisor_disabled_limits_do_not_probe) { cbm_index_worker_handle_t *handle = NULL; int start_rc = cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"clean\"}", 0, &policy, false, NULL, NULL, 0, &handle); + /* Opt-in: no worker limit, no publication fence, no claim. */ + bool fenced = !handle || cbm_index_worker_publish_fence_path_for_testing(handle) != NULL; const cbm_index_worker_result_t *result = NULL; bool terminal = handle && index_supervisor_test_poll_terminal( handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); cbm_index_resource_t resource = result ? result->resource_violation.resource : CBM_INDEX_RESOURCE_RSS_BYTES; bool probe_failed = result && result->resource_probe_failed; + bool committed = !result || result->publication_committed; if (terminal) { cbm_index_worker_destroy(handle); } else { @@ -1090,6 +1134,8 @@ TEST(index_supervisor_disabled_limits_do_not_probe) { ASSERT_EQ(fake.clock_calls, 0); ASSERT_EQ(resource, CBM_INDEX_RESOURCE_NONE); ASSERT_FALSE(probe_failed); + ASSERT_FALSE(fenced); + ASSERT_FALSE(committed); PASS(); } @@ -1352,6 +1398,179 @@ TEST(index_supervisor_three_failed_rss_probes_fail_closed) { PASS(); } +/* Publication fence (index_supervisor.h). The hang-tree probe reports the fence + * path it received on its argv in its marker, and the test plays the worker's + * publication claim through the production entry point with that path. */ +typedef struct { + char directory[INDEX_SUPERVISOR_TEST_PATH_CAP]; + char marker[INDEX_SUPERVISOR_TEST_PATH_CAP]; + char fence[INDEX_SUPERVISOR_TEST_PATH_CAP]; +} index_supervisor_fence_fixture_t; + +static bool index_supervisor_fence_fixture_init(index_supervisor_fence_fixture_t *fixture) { + memset(fixture, 0, sizeof(*fixture)); + (void)snprintf(fixture->directory, sizeof(fixture->directory), "%s/cbm-index-fence-XXXXXX", + cbm_tmpdir()); + if (!cbm_mkdtemp(fixture->directory)) { + return false; + } + (void)snprintf(fixture->marker, sizeof(fixture->marker), "%s/marker", fixture->directory); + return true; +} + +/* True once the worker started with exactly this attempt's fence. */ +static bool index_supervisor_fence_fixture_started(index_supervisor_fence_fixture_t *fixture, + cbm_index_worker_handle_t *handle) { + const char *fence = handle ? cbm_index_worker_publish_fence_path_for_testing(handle) : NULL; + if (!fence) { + return false; + } + (void)snprintf(fixture->fence, sizeof(fixture->fence), "%s", fence); + char expected[INDEX_SUPERVISOR_TEST_PATH_CAP + 16]; + (void)snprintf(expected, sizeof(expected), "fence=%s\n", fixture->fence); + return index_supervisor_test_wait_file_text(fixture->marker, expected, + INDEX_SUPERVISOR_TEST_READY_MS); +} + +static bool index_supervisor_fence_fixture_worker_claims( + index_supervisor_fence_fixture_t *fixture) { + cbm_index_set_worker_publish_fence(fixture->fence); + bool claimed = cbm_index_worker_claim_publication(); + cbm_index_set_worker_publish_fence(NULL); + return claimed; +} + +static void index_supervisor_fence_fixture_cleanup(index_supervisor_fence_fixture_t *fixture) { + (void)cbm_unlink(fixture->marker); + (void)cbm_rmdir(fixture->directory); +} + +static int g_index_supervisor_after_publication_logs; + +static void index_supervisor_test_after_publication_sink(const char *line) { + if (line && strstr(line, "index.supervisor.resource_limit_after_publication")) { + g_index_supervisor_after_publication_logs++; + } +} + +TEST(index_supervisor_limit_claims_publication_before_terminating) { + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_EMPTY, + }; + index_supervisor_fence_fixture_t fixture; + bool fixture_ready = index_supervisor_fence_fixture_init(&fixture); + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_charged_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = + fixture_ready + ? cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, fixture.marker, NULL, 100, &handle) + : -1; + bool started = start_rc == 0 && index_supervisor_fence_fixture_started(&fixture, handle); + /* The limit is crossed while the worker has not claimed publication: the + * supervisor claims the fence in this poll, before it stops the worker. */ + fake.now_ms = 1101; + const cbm_index_worker_result_t *result = NULL; + cbm_index_worker_poll_t decided = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + /* Whatever the worker does next, it can no longer publish. */ + bool worker_barred = started && !index_supervisor_fence_fixture_worker_claims(&fixture); + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool limited = terminal && result && + result->resource_violation.resource == CBM_INDEX_RESOURCE_DURATION_MS && + result->resource_violation.observed == 1001 && !result->publication_committed && + !result->cancellation_requested && result->tree_quiesced; + bool fence_removed = terminal && fixture.fence[0] && cbm_file_size(fixture.fence) < 0; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + if (!started) { + index_supervisor_test_dump("marker", fixture.marker); + } + index_supervisor_fence_fixture_cleanup(&fixture); + + ASSERT_TRUE(fixture_ready); + ASSERT_EQ(start_rc, 0); + ASSERT_TRUE(started); + ASSERT_EQ(decided, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(worker_barred); + ASSERT_TRUE(limited); + ASSERT_TRUE(fence_removed); + PASS(); +} + +TEST(index_supervisor_limit_after_worker_publication_claim_is_not_enforced) { + index_supervisor_resource_fake_t fake = { + .now_ms = 100, + .rss_status = CBM_PROC_TREE_RSS_EMPTY, + }; + index_supervisor_fence_fixture_t fixture; + bool fixture_ready = index_supervisor_fence_fixture_init(&fixture); + CBMLogLevel saved_level = cbm_log_get_level(); + cbm_log_set_level(CBM_LOG_INFO); + g_index_supervisor_after_publication_logs = 0; + cbm_log_set_sink_ex(index_supervisor_test_after_publication_sink, CBM_LOG_SINK_TEE); + cbm_index_supervisor_set_resource_hooks_for_testing(index_supervisor_fake_clock, + index_supervisor_fake_charged_rss, &fake); + cbm_index_resource_policy_t policy = index_supervisor_test_worker_policy(0, 1000); + cbm_index_worker_handle_t *handle = NULL; + int start_rc = + fixture_ready + ? cbm_index_worker_start_with_policy("{\"__cbm_test_worker\":\"hang-tree\"}", 0, + &policy, false, fixture.marker, NULL, 100, &handle) + : -1; + bool started = start_rc == 0 && index_supervisor_fence_fixture_started(&fixture, handle); + /* The worker commits to publishing first; only then is the limit crossed. */ + bool worker_claimed = started && index_supervisor_fence_fixture_worker_claims(&fixture); + fake.now_ms = 1101; + const cbm_index_worker_result_t *result = NULL; + cbm_index_worker_poll_t decided = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + int clock_calls_at_decision = fake.clock_calls; + cbm_index_worker_poll_t after = + handle ? cbm_index_worker_poll(handle, &result) : CBM_INDEX_WORKER_POLL_ERROR; + bool disarmed = fake.clock_calls == clock_calls_at_decision; + /* The attempt runs on; the test ends it with an ordinary cancellation. */ + bool cancel_accepted = handle && cbm_index_worker_request_cancel(handle); + bool terminal = handle && index_supervisor_test_poll_terminal( + handle, INDEX_SUPERVISOR_TEST_TERMINAL_MS, &result); + bool committed = terminal && result && result->cancellation_requested && + result->resource_violation.resource == CBM_INDEX_RESOURCE_NONE && + !result->resource_probe_failed && result->publication_committed && + result->tree_quiesced; + if (terminal) { + cbm_index_worker_destroy(handle); + } else { + index_supervisor_test_cleanup_handle(handle); + } + cbm_index_supervisor_reset_resource_hooks_for_testing(); + cbm_log_set_sink(NULL); + cbm_log_set_level(saved_level); + if (!started) { + index_supervisor_test_dump("marker", fixture.marker); + } + index_supervisor_fence_fixture_cleanup(&fixture); + + ASSERT_TRUE(fixture_ready); + ASSERT_EQ(start_rc, 0); + ASSERT_TRUE(started); + ASSERT_TRUE(worker_claimed); + ASSERT_EQ(decided, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_EQ(after, CBM_INDEX_WORKER_POLL_RUNNING); + ASSERT_TRUE(disarmed); + ASSERT_EQ(g_index_supervisor_after_publication_logs, 1); + ASSERT_TRUE(cancel_accepted); + ASSERT_TRUE(committed); + PASS(); +} + SUITE(index_supervisor) { RUN_TEST(index_supervisor_job_memory_limit_has_floor_headroom_and_no_overflow); RUN_TEST(index_supervisor_worker_argv_requires_exact_build_bound_grammar); @@ -1370,4 +1589,6 @@ SUITE(index_supervisor) { RUN_TEST(index_supervisor_quiet_timeout_remains_hang_with_duration_enabled); RUN_TEST(index_supervisor_cancel_precedes_resource_probe); RUN_TEST(index_supervisor_three_failed_rss_probes_fail_closed); + RUN_TEST(index_supervisor_limit_claims_publication_before_terminating); + RUN_TEST(index_supervisor_limit_after_worker_publication_claim_is_not_enforced); } diff --git a/tests/test_main.c b/tests/test_main.c index 23b2621ee..652893ce1 100644 --- a/tests/test_main.c +++ b/tests/test_main.c @@ -455,10 +455,12 @@ static void tf_index_worker_probe(const char *args_json, const char *response_ou FILE *ready = marker ? cbm_fopen(marker, "wb") : NULL; if (ready) { (void)fprintf( - ready, "single=%s\nmarker=%s\nquarantine=%s\nbudget=%zu\ndescendant=%ld\n", + ready, + "single=%s\nmarker=%s\nquarantine=%s\nbudget=%zu\ndescendant=%ld\nfence=%s\n", getenv("CBM_INDEX_SINGLE_THREAD") ? getenv("CBM_INDEX_SINGLE_THREAD") : "", marker, getenv("CBM_INDEX_QUARANTINE_FILE") ? getenv("CBM_INDEX_QUARANTINE_FILE") : "", - cbm_mem_budget(), descendant); + cbm_mem_budget(), descendant, + cbm_index_worker_publish_fence() ? cbm_index_worker_publish_fence() : ""); (void)fclose(ready); } (void)fprintf(stderr, "async worker hang-tree probe\n"); @@ -509,6 +511,7 @@ static int tf_maybe_run_index_worker(int argc, char **argv) { cbm_index_set_worker_role_options(true, invocation.response_out, invocation.single_thread, invocation.marker_file, invocation.quarantine_file, invocation.memory_budget_bytes); + cbm_index_set_worker_publish_fence(invocation.publish_fence_file); cbm_mem_init_with_cap(0.5, invocation.memory_budget_bytes); cbm_log_init_for_process(false, true); tf_index_worker_probe(invocation.args_json, invocation.response_out); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 3bfcb018f..7b4c69d91 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -21677,6 +21677,289 @@ TEST(index_worker_resource_limit_is_trusted_structured_and_not_retried) { #endif } +#ifndef _WIN32 +#include + +/* A worker limit must never be reported after the same attempt published. + * The real worker replaces the serving database and only then exports the + * persistence artifact. A FIFO planted at the artifact's temp path holds it + * inside that export, after the rename, for as long as nobody reads the FIFO, + * and the duration clock crosses its limit only once the serving database has + * been replaced. The limit is therefore observed in the post-publication + * window by construction, never by timing: + * - uncoordinated, the supervisor stops the worker there and answers + * resource_limit_exceeded with serving_index_preserved=true although the + * previous generation is already gone; + * - with the publication fence the worker claimed publication before the + * rename, so the supervisor leaves the limit unenforced and logs that. + * Only that log line releases the FIFO; the worker then finishes and + * reports the published index. The artifact path is a directory, so the + * export itself fails afterwards, which the #1665 response names. */ +typedef struct { + char live_db[CBM_SZ_1K]; + ino_t previous_inode; + int over_limit_calls; +} index_publication_race_clock_t; + +static char g_publication_race_fifo[CBM_SZ_1K]; +static bool g_publication_race_released; + +static uint64_t index_publication_race_clock(void *context) { + index_publication_race_clock_t *clock = context; + struct stat status; + if (stat(clock->live_db, &status) != 0 || status.st_ino == clock->previous_inode) { + return 100; + } + clock->over_limit_calls++; + return 1101; +} + +static void index_publication_race_sink(const char *line) { + if (g_publication_race_released || !line || + !strstr(line, "index.supervisor.resource_limit_after_publication")) { + return; + } + g_publication_race_released = true; + /* Blocks only until the held worker opens its end, then reads to EOF. */ + int fifo = open(g_publication_race_fifo, O_RDONLY); + if (fifo >= 0) { + char buffer[CBM_SZ_4K]; + ssize_t got = 0; + do { + got = read(fifo, buffer, sizeof(buffer)); + } while (got > 0); + (void)close(fifo); + } +} + +enum { + IDXPUB_OK = 0, + IDXPUB_SETUP = 81, + IDXPUB_FIRST_INDEX = 82, + IDXPUB_HOLD_SETUP = 83, + IDXPUB_NO_RESPONSE = 84, + IDXPUB_NOT_REPLACED = 85, + IDXPUB_NOT_CROSSED = 86, + IDXPUB_LIMIT_AFTER_PUBLICATION = 87, + IDXPUB_NOT_RELEASED = 88, + IDXPUB_NOT_REPORTED_PUBLISHED = 89, +}; + +static int index_publication_race_first_generation(cbm_mcp_server_t *server, const char *repo, + const char *cache, + index_publication_race_clock_t *clock) { + char args[CBM_SZ_2K]; + (void)snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"mode\":\"fast\"}", repo); + char *response = cbm_mcp_handle_tool(server, "index_repository", args); + char *text = extract_text_content(response); + yyjson_doc *doc = text ? yyjson_read(text, strlen(text), 0) : NULL; + yyjson_val *root = doc ? yyjson_doc_get_root(doc) : NULL; + const char *status = yyjson_get_str(yyjson_obj_get(root, "status")); + const char *project = yyjson_get_str(yyjson_obj_get(root, "project")); + struct stat live; + bool indexed = status && strcmp(status, "indexed") == 0 && project && + snprintf(clock->live_db, sizeof(clock->live_db), "%s/%s.db", cache, project) < + (int)sizeof(clock->live_db) && + stat(clock->live_db, &live) == 0; + if (indexed) { + clock->previous_inode = live.st_ino; + } + yyjson_doc_free(doc); + free(text); + free(response); + return indexed ? IDXPUB_OK : IDXPUB_FIRST_INDEX; +} + +static int index_publication_race_check(const char *repo, const char *cache) { + (void)cbm_setenv("CBM_CACHE_DIR", cache, 1); + cbm_unsetenv("CBM_INDEX_SUPERVISOR"); + cbm_index_supervisor_mark_host(); + cbm_config_t *config = cbm_config_open(cache); + cbm_mcp_server_t *server = cbm_mcp_server_new(NULL); + if (!config || !server) { + cbm_mcp_server_free(server); + cbm_config_close(config); + return IDXPUB_SETUP; + } + cbm_mcp_server_set_config(server, config); + index_publication_race_clock_t clock = {0}; + int result = index_publication_race_first_generation(server, repo, cache, &clock); + + /* A second generation to publish, the hold, and a 1 s duration limit. */ + char art_dir[CBM_SZ_1K]; + char art_path[CBM_SZ_1K]; + (void)snprintf(art_dir, sizeof(art_dir), "%s/.codebase-memory", repo); + (void)snprintf(art_path, sizeof(art_path), "%s/graph.db.zst", art_dir); + (void)snprintf(g_publication_race_fifo, sizeof(g_publication_race_fifo), "%s.tmp", art_path); + if (result == IDXPUB_OK && + (th_write_file(TH_PATH(repo, "extra.c"), "int extra(void) { return 1; }\n") != 0 || + mkdir(art_dir, 0700) != 0 || mkdir(art_path, 0700) != 0 || + mkfifo(g_publication_race_fifo, 0600) != 0 || + cbm_config_set(config, CBM_INDEX_CONFIG_MAX_DURATION_SECONDS, "1") != 0)) { + result = IDXPUB_HOLD_SETUP; + } + char *response = NULL; + if (result == IDXPUB_OK) { + char args[CBM_SZ_2K]; + (void)snprintf(args, sizeof(args), + "{\"repo_path\":\"%s\",\"mode\":\"fast\",\"persistence\":true}", repo); + cbm_log_set_level(CBM_LOG_WARN); + cbm_log_set_sink_ex(index_publication_race_sink, CBM_LOG_SINK_TEE); + cbm_index_supervisor_set_resource_hooks_for_testing(index_publication_race_clock, NULL, + &clock); + response = cbm_mcp_handle_tool(server, "index_repository", args); + cbm_index_supervisor_reset_resource_hooks_for_testing(); + cbm_log_set_sink(NULL); + } + struct stat live; + bool replaced = stat(clock.live_db, &live) == 0 && live.st_ino != clock.previous_inode; + if (result != IDXPUB_OK) { + /* setup failure already recorded */ + } else if (!response) { + result = IDXPUB_NO_RESPONSE; + } else if (!replaced) { + result = IDXPUB_NOT_REPLACED; + } else if (clock.over_limit_calls == 0) { + result = IDXPUB_NOT_CROSSED; + } else if (strstr(response, "resource_limit_exceeded")) { + result = IDXPUB_LIMIT_AFTER_PUBLICATION; + } else if (!g_publication_race_released) { + result = IDXPUB_NOT_RELEASED; + } else if (!strstr(response, "Index database was published")) { + result = IDXPUB_NOT_REPORTED_PUBLISHED; + } + if (result != IDXPUB_OK && response) { + printf(" second response: %.600s\n", response); + fflush(stdout); + } + free(response); + cbm_mcp_server_free(server); + cbm_config_close(config); + return result; +} +#endif + +TEST(index_worker_limit_never_reported_after_its_own_publication) { +#ifdef _WIN32 + SKIP_PLATFORM("fork + FIFO hold harness; the fence state machine is cross-platform and " + "covered by the index_supervisor suite"); +#else + /* th_mktempdir returns one static buffer: keep each path in its own copy. */ + const char *made = th_mktempdir("cbm_publication_race_repo"); + char *repo = made ? strdup(made) : NULL; + made = th_mktempdir("cbm_publication_race_cache"); + char *cache = made ? strdup(made) : NULL; + ASSERT_NOT_NULL(repo); + ASSERT_NOT_NULL(cache); + ASSERT_EQ(th_write_file(TH_PATH(repo, "main.c"), "int main(void) { return 0; }\n"), 0); + fflush(NULL); + pid_t pid = fork(); + if (pid == 0) { + /* Hang guard only: two supervised runs that each start an ASan worker. */ + alarm(180); + _exit(index_publication_race_check(repo, cache)); + } + int status = 0; + bool waited = waitpid(pid, &status, 0) == pid; + int exit_code = waited && WIFEXITED(status) ? WEXITSTATUS(status) : -1; + int signal_code = waited && WIFSIGNALED(status) ? WTERMSIG(status) : 0; + th_cleanup(repo); + th_cleanup(cache); + free(repo); + free(cache); + if (exit_code != IDXPUB_OK) { + printf(" publication race child exit=%d signal=%d\n", exit_code, signal_code); + } + ASSERT_TRUE(waited); + ASSERT_EQ(signal_code, 0); + ASSERT_EQ(exit_code, IDXPUB_OK); + PASS(); +#endif +} + +/* serving_index_preserved describes the request, not only its last attempt: + * an earlier attempt may have published before it crashed (recovery then runs + * another attempt), and an index that appeared during the request (artifact + * bootstrap) was not preserved. */ +TEST(index_worker_resource_response_reports_preservation_for_the_request) { + const char *made = th_mktempdir("cbm_request_facts_repo"); + char *repo = made ? strdup(made) : NULL; + made = th_mktempdir("cbm_request_facts_cache"); + char *cache = made ? strdup(made) : NULL; + ASSERT_NOT_NULL(repo); + ASSERT_NOT_NULL(cache); + ASSERT_EQ(th_write_file(TH_PATH(repo, "main.c"), "int main(void) { return 0; }\n"), 0); + const char *old_cache = getenv("CBM_CACHE_DIR"); + char *saved_cache = old_cache ? strdup(old_cache) : NULL; + (void)cbm_setenv("CBM_CACHE_DIR", cache, 1); + char args[CBM_SZ_2K]; + (void)snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"name\":\"RequestFactsFixture\"}", + repo); + + cbm_mcp_index_request_facts_t before_index = {0}; + cbm_mcp_index_request_facts_begin(args, &before_index); + cbm_mcp_server_t *server = cbm_mcp_server_new(NULL); + char *indexed = server ? cbm_mcp_handle_tool(server, "index_repository", args) : NULL; + cbm_mcp_server_free(server); + cbm_mcp_index_request_facts_t facts = {0}; + cbm_mcp_index_request_facts_begin(args, &facts); + + cbm_index_worker_result_t limited; + memset(&limited, 0, sizeof(limited)); + limited.outcome = CBM_PROC_KILLED; + limited.tree_quiesced = true; + limited.resource_violation = (cbm_index_resource_violation_t){ + .resource = CBM_INDEX_RESOURCE_DURATION_MS, .observed = 1001, .limit = 1000}; + cbm_index_worker_result_t published_then_crashed; + memset(&published_then_crashed, 0, sizeof(published_then_crashed)); + published_then_crashed.outcome = CBM_PROC_CRASH; + published_then_crashed.publication_committed = true; + cbm_mcp_index_request_facts_t after_recovery = facts; + cbm_mcp_index_request_facts_record(&after_recovery, &limited); + bool limited_attempt_commits_nothing = !after_recovery.publication_committed; + cbm_mcp_index_request_facts_record(&after_recovery, &published_then_crashed); + + char *preserved = cbm_mcp_index_worker_resource_response(args, &limited, &facts); + char *earlier_published = + cbm_mcp_index_worker_resource_response(args, &limited, &after_recovery); + char *appeared = cbm_mcp_index_worker_resource_response(args, &limited, &before_index); + char *no_facts = cbm_mcp_index_worker_resource_response(args, &limited, NULL); + const char *kept = "\\\"serving_index_preserved\\\":true"; + const char *not_kept = "\\\"serving_index_preserved\\\":false"; + bool indexed_ok = indexed && strstr(indexed, "\\\"status\\\":\\\"indexed\\\""); + bool preserved_ok = preserved && strstr(preserved, kept); + bool earlier_ok = earlier_published && strstr(earlier_published, not_kept); + bool appeared_ok = appeared && strstr(appeared, not_kept); + bool no_facts_ok = no_facts && strstr(no_facts, not_kept); + + free(indexed); + free(preserved); + free(earlier_published); + free(appeared); + free(no_facts); + if (saved_cache) { + (void)cbm_setenv("CBM_CACHE_DIR", saved_cache, 1); + } else { + cbm_unsetenv("CBM_CACHE_DIR"); + } + free(saved_cache); + th_cleanup(repo); + th_cleanup(cache); + free(repo); + free(cache); + + ASSERT_FALSE(before_index.serving_index_existed); + ASSERT_TRUE(indexed_ok); + ASSERT_TRUE(facts.serving_index_existed); + ASSERT_FALSE(facts.publication_committed); + ASSERT_TRUE(limited_attempt_commits_nothing); + ASSERT_TRUE(after_recovery.publication_committed); + ASSERT_TRUE(preserved_ok); + ASSERT_TRUE(earlier_ok); + ASSERT_TRUE(appeared_ok); + ASSERT_TRUE(no_facts_ok); + PASS(); +} /* Child-side check: index a tiny fixture and verify it ran IN-PROCESS. * Distinct exit codes so the parent can report the exact failure mode. */ @@ -25238,6 +25521,8 @@ SUITE(mcp) { RUN_TEST(index_supervisor_start_failure_is_fail_closed_in_real_host); RUN_TEST(index_supervisor_clean_exit_without_response_response_issue1300); RUN_TEST(index_worker_resource_limit_is_trusted_structured_and_not_retried); + RUN_TEST(index_worker_limit_never_reported_after_its_own_publication); + RUN_TEST(index_worker_resource_response_reports_preservation_for_the_request); RUN_TEST(index_bg_paths_route_through_supervisor_issue832); RUN_TEST(sequential_service_edge_props_are_valid_json_issue898); RUN_TEST(index_second_inprocess_run_survives_issue773); diff --git a/tests/test_pipeline.c b/tests/test_pipeline.c index 2b80f51bd..2441c205e 100644 --- a/tests/test_pipeline.c +++ b/tests/test_pipeline.c @@ -21733,6 +21733,72 @@ TEST(cancelled_incremental_reindex_preserves_committed_db) { PASS(); } +typedef struct { + bool allow; + int calls; +} publish_gate_ctx_t; + +static bool publish_gate_decide(void *context) { + publish_gate_ctx_t *gate = context; + gate->calls++; + return gate->allow; +} + +/* The publication gate (a supervised worker's claim on its publication fence) + * is the last word before the serving database is touched: refused, the run + * stops with the previous generation's file in place; allowed, it publishes + * as before. Asked exactly once either way. */ +TEST(publish_gate_decides_before_the_committed_db_is_touched) { + if (setup_incremental_repo() != 0) { + FAIL("setup failed"); + } + cbm_pipeline_t *p = cbm_pipeline_new(g_incr_tmpdir, g_incr_dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + ASSERT_EQ(cbm_pipeline_run(p), 0); + char *project = strdup(cbm_pipeline_project_name(p)); + cbm_pipeline_free(p); + ASSERT_NOT_NULL(project); + struct stat baseline; + ASSERT_EQ(stat(g_incr_dbpath, &baseline), 0); + + ASSERT_TRUE(write_go_file(g_incr_tmpdir, "gate_candidate.go", + "package main\n\nfunc GateCandidate() int { return 7 }\n")); + publish_gate_ctx_t refused = {.allow = false}; + p = cbm_pipeline_new(g_incr_tmpdir, g_incr_dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_publish_gate(p, publish_gate_decide, &refused); + int refused_rc = cbm_pipeline_run(p); + cbm_pipeline_free(p); + struct stat after_refusal; + bool same_file = + stat(g_incr_dbpath, &after_refusal) == 0 && after_refusal.st_ino == baseline.st_ino; + cbm_store_t *live = cbm_store_open_path(g_incr_dbpath); + int refused_candidates = live ? count_nodes_named(live, project, "GateCandidate") : -1; + cbm_store_close(live); + + publish_gate_ctx_t allowed = {.allow = true}; + p = cbm_pipeline_new(g_incr_tmpdir, g_incr_dbpath, CBM_MODE_FULL); + ASSERT_NOT_NULL(p); + cbm_pipeline_set_publish_gate(p, publish_gate_decide, &allowed); + int allowed_rc = cbm_pipeline_run(p); + cbm_pipeline_free(p); + live = cbm_store_open_path(g_incr_dbpath); + int allowed_candidates = live ? count_nodes_named(live, project, "GateCandidate") : -1; + cbm_store_close(live); + + free(project); + cleanup_incremental_repo(); + + ASSERT_EQ(refused.calls, 1); + ASSERT_EQ(refused_rc, CBM_PIPELINE_ABORT_PRESERVE_DB); + ASSERT_TRUE(same_file); + ASSERT_EQ(refused_candidates, 0); + ASSERT_EQ(allowed.calls, 1); + ASSERT_EQ(allowed_rc, 0); + ASSERT_GT(allowed_candidates, 0); + PASS(); +} + TEST(backup_failed_publish_failure_preserves_final_sidecars) { if (setup_incremental_repo() != 0) { FAIL("setup failed"); @@ -26421,6 +26487,7 @@ SUITE(pipeline) { RUN_TEST(incremental_new_file_added); RUN_TEST(cancelled_full_reindex_preserves_committed_db); RUN_TEST(cancelled_incremental_reindex_preserves_committed_db); + RUN_TEST(publish_gate_decides_before_the_committed_db_is_touched); RUN_TEST(backup_failed_publish_failure_preserves_final_sidecars); RUN_TEST(backup_failed_rename_failure_preserves_corrupt_main); #ifdef __linux__ From 7a1bee25486b245b9fc6a75fd6d69c758bbb8a9d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 3 Nov 2026 01:05:17 +0100 Subject: [PATCH 8/8] fix(index): keep the publication fence off raw allocator sites The memory-core ratchet (lint-memory-core) failed on the publication fence commit: it added six raw free() calls, each releasing a block that existing code allocates with raw calloc, malloc or strdup. cbm_free cannot release such a block without the core class counters drifting, so every new exit now reuses an existing release point instead: - index_supervisor.c: worker_artifacts_create() creates the response, log and publish fence files and names the one that failed, so all three failures share the single artifact_create_failed exit. - pipeline.c: a refused publication gate takes the existing source-changed abort exit (same CBM_PIPELINE_ABORT_PRESERVE_DB result and cleanup). - mcp.c: index_request_names_free() is the one release point for the request name strings, used by index_worker_json_result() and index_args_serving_index_servable(). Raw-site counts return to the origin/main values (index_supervisor.c 8, pipeline.c 129, mcp.c 765) with no baseline change. No behaviour change. Signed-off-by: Martin Vogel --- src/mcp/index_supervisor.c | 38 ++++++++++++++++++++++---------------- src/mcp/mcp.c | 20 +++++++++++++------- src/pipeline/pipeline.c | 17 +++++++---------- 3 files changed, 42 insertions(+), 33 deletions(-) diff --git a/src/mcp/index_supervisor.c b/src/mcp/index_supervisor.c index c8c4224ad..701ac5743 100644 --- a/src/mcp/index_supervisor.c +++ b/src/mcp/index_supervisor.c @@ -1040,6 +1040,25 @@ static void worker_publish_fence_remove(cbm_index_worker_handle_t *handle) { } } +/* Create the private files of one attempt. NULL on success; otherwise the name + * of the artifact that could not be created, with errno as that step left it + * and every earlier artifact still in place for the caller to remove. */ +static const char *worker_artifacts_create(cbm_index_worker_handle_t *handle) { + if (!worker_unique_file(handle->response_path, sizeof(handle->response_path), "response")) { + return "response"; + } + if (!worker_unique_file(handle->log_path, sizeof(handle->log_path), "log")) { + return "log"; + } + /* Opt-in: only an enabled worker limit can stop a worker, so only then is + * there a publication to coordinate. */ + if (cbm_index_policy_worker_enabled(&handle->resource_policy) && + !worker_publish_fence_create(handle)) { + return "publish_fence"; + } + return NULL; +} + /* Settle who held the fence once the attempt is terminal. Claiming an unowned * fence now also bars any later claim, so a stray member of an unquiesced tree * can never publish after this report. */ @@ -1109,25 +1128,12 @@ static int worker_start_internal(const char *args_json, size_t memory_budget_byt handle->log_callback = log_callback; handle->log_context = log_context; worker_result_init(&handle->result); - if (!worker_unique_file(handle->response_path, sizeof(handle->response_path), "response") || - !worker_unique_file(handle->log_path, sizeof(handle->log_path), "log")) { + const char *failed_artifact = worker_artifacts_create(handle); + if (failed_artifact) { int saved_errno = errno; char error_text[CBM_SZ_32]; (void)snprintf(error_text, sizeof(error_text), "%d", saved_errno); - cbm_log_error("index.supervisor.artifact_create_failed", "artifact", - handle->response_path[0] ? "log" : "response", "errno", error_text); - (void)cbm_unlink(handle->response_path); - (void)cbm_unlink(handle->log_path); - free(handle); - return -1; - } - /* Opt-in: only an enabled worker limit can stop a worker, so only then is - * there a publication to coordinate. */ - if (cbm_index_policy_worker_enabled(&handle->resource_policy) && - !worker_publish_fence_create(handle)) { - char error_text[CBM_SZ_32]; - (void)snprintf(error_text, sizeof(error_text), "%d", errno); - cbm_log_error("index.supervisor.artifact_create_failed", "artifact", "publish_fence", + cbm_log_error("index.supervisor.artifact_create_failed", "artifact", failed_artifact, "errno", error_text); (void)cbm_unlink(handle->response_path); (void)cbm_unlink(handle->log_path); diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index ddaba7e9d..a8593d946 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -11952,15 +11952,23 @@ static bool build_index_success_response(cbm_mcp_server_t *srv, yyjson_mut_doc * static bool project_db_is_servable(const char *project, const char *db_path); +/* The heap strings that name the project of an internal worker request + * (repo_path and name from cbm_mcp_get_string_arg, the project name derived from + * them). One release point for every helper that reads them; free(NULL) is a + * no-op, so any of them may be NULL. */ +static void index_request_names_free(char *repo_path, char *name_override, char *project_name) { + free(repo_path); + free(name_override); + free(project_name); +} + /* Serialize a worker JSON document and release the heap strings it borrowed. - * Extra arguments may be NULL; free(NULL) is a no-op. */ + * Extra arguments may be NULL. */ static char *index_worker_json_result(yyjson_mut_doc *doc, char *repo_path, char *name_override, char *project_name) { char *json = yy_doc_to_str(doc); yyjson_mut_doc_free(doc); - free(repo_path); - free(name_override); - free(project_name); + index_request_names_free(repo_path, name_override, project_name); char *result = cbm_mcp_text_result(json, true); free(json); return result; @@ -12034,9 +12042,7 @@ static bool index_args_serving_index_servable(const char *args) { project_db_path(project_name, db_path, sizeof(db_path)); servable = project_db_is_servable(project_name, db_path); } - free(repo_path); - free(name_override); - free(project_name); + index_request_names_free(repo_path, name_override, project_name); return servable; } diff --git a/src/pipeline/pipeline.c b/src/pipeline/pipeline.c index e393278ca..519fc7d73 100644 --- a/src/pipeline/pipeline.c +++ b/src/pipeline/pipeline.c @@ -5240,19 +5240,16 @@ int cbm_pipeline_run(cbm_pipeline_t *p) { cbm_userconfig_t *fresh_config = cbm_userconfig_load_with_source(p->repo_path); bool source_matches = cbm_pipeline_project_source_matches(p, fresh_config); cbm_userconfig_free(fresh_config); + bool publish_allowed = source_matches; if (!source_matches) { cbm_log_error("pipeline.err", "phase", "test_declarations_source_changed"); - cleanup_staging_db(staging_path); - free(staging_path); - free(final_path); - pipeline_release_test_config(p); - return CBM_PIPELINE_ABORT_PRESERVE_DB; - } - - /* Last point at which the serving database is still untouched: the - * destination preparation below may already move it aside. */ - if (p->publish_gate && !p->publish_gate(p->publish_gate_ctx)) { + } else if (p->publish_gate && !p->publish_gate(p->publish_gate_ctx)) { + /* Last point at which the serving database is still untouched: the + * destination preparation below may already move it aside. */ cbm_log_warn("pipeline.publish_refused", "path", final_path); + publish_allowed = false; + } + if (!publish_allowed) { cleanup_staging_db(staging_path); free(staging_path); free(final_path);