From 7a1bee25486b245b9fc6a75fd6d69c758bbb8a9d Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Tue, 3 Nov 2026 01:05:17 +0100 Subject: [PATCH] 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);