Skip to content

Commit 2f7befb

Browse files
DeusDataDeusData
committed
fix(pipeline): destroy TLS parsers at allocator-epoch boundaries
SIGABRT (free of a non-libmalloc pointer in ts_stack_delete) on the second index_repository in one process, whenever a sequential run is followed by a parallel one. The sequential path parses on the CALLING thread and left its TLS parser alive; the next parallel run switches the global tree-sitter allocator to the slab (cbm_slab_install), the calling thread participates in extraction, reuses the stale parser, and its per-file teardown frees mimalloc-epoch memory through slab_free's foreign-pointer fallback -> plain free() -> libmalloc abort. The v0.9.0 supervisor masks this on the default MCP path (fresh worker process per index); the in-process path and every embedded consumer died. Root-caused with allocator-epoch instrumentation: the victim parser is created at epoch 0 in cbm_pipeline_pass_definitions and deleted at epoch 1 in extract_worker. Two boundary fixes: - run_sequential_pipeline destroys the calling thread's parser at run end (also bounds its memory like the parallel workers already do); - cbm_parallel_extract destroys the calling thread's parser BEFORE cbm_slab_install, so no parser ever crosses the allocator switch. Guards: prod-tier smoke invariant inv_second_index_inprocess (sequential-then-parallel double index over stdio, RED with signal 6 on the unfixed binary — ASan builds mask the allocator seam, so the unit tier cannot catch this class) + suite-tier fork-isolated index_second_inprocess_run_survives_issue773 pinning the in-process double-index contract. Repro 5/5 clean after the fix (was 5/5 abort). Closes #773 Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
1 parent 751d229 commit 2f7befb

4 files changed

Lines changed: 177 additions & 1 deletion

File tree

‎scripts/smoke-invariants.sh‎

Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,48 @@ inv_index_status_cli() {
345345
fi
346346
}
347347

348+
# ── #773: second in-process index must not SIGABRT ─────────────────────────
349+
# Sequential run 1 (mimalloc-epoch parser on the calling thread) followed by
350+
# parallel run 2 (global ts allocator switched to the slab). The stale-parser
351+
# teardown used to free mi pointers through slab_free -> plain free() and
352+
# libmalloc aborted. ASan builds mask the allocator seam — this guard needs
353+
# the REAL binary.
354+
inv_second_index_inprocess() {
355+
local base
356+
base=$(mktemp -d "${TMPDIR:-/tmp}/cbm_inv773.XXXXXX")
357+
mkdir -p "$base/dirA" "$base/dirB"
358+
local i
359+
for i in 1 2 3 4 5; do
360+
printf 'class H%s:\n def run(self, x):\n return x + %s\n' "$i" "$i" > "$base/dirA/m$i.py"
361+
done
362+
for i in $(seq 1 60); do
363+
printf 'def u_%s(y):\n return y * %s\n' "$i" "$i" > "$base/dirB/u$i.py"
364+
done
365+
local infile="$base/in.jsonl"
366+
{
367+
printf '%s\n' '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{"protocolVersion":"2024-11-05","capabilities":{},"clientInfo":{"name":"inv773","version":"1"}}}'
368+
printf '%s\n' '{"jsonrpc":"2.0","method":"notifications/initialized"}'
369+
printf '{"jsonrpc":"2.0","id":2,"method":"tools/call","params":{"name":"index_repository","arguments":{"repo_path":"%s/dirA","mode":"full"}}}\n' "$base"
370+
printf '{"jsonrpc":"2.0","id":3,"method":"tools/call","params":{"name":"index_repository","arguments":{"repo_path":"%s/dirB","mode":"full"}}}\n' "$base"
371+
} > "$infile"
372+
# Re-open stdin from the file INSIDE the child: run_bounded's no-timeout
373+
# fallback backgrounds the command, and background jobs get /dev/null
374+
# stdin (POSIX), which would EOF the server before it answers.
375+
run_bounded 120 env CBM_INDEX_SUPERVISOR=0 sh -c 'exec "$1" < "$2"' _ "$BINARY" "$infile"
376+
local rc="$RB_RC"
377+
local out="$RB_OUT"
378+
rm -rf "$base"
379+
if [ "$rc" -gt 128 ]; then
380+
fail "second-index-inprocess" "server died (signal $((rc-128))) on the second in-process index (#773)"
381+
return
382+
fi
383+
if printf '%s' "$out" | grep -q '"id":2' && printf '%s' "$out" | grep -q '"id":3'; then
384+
pass "second-index-inprocess (both runs answered, no abort)"
385+
else
386+
fail "second-index-inprocess" "missing response (id2/id3) rc=$rc"
387+
fi
388+
}
389+
348390
# ══════════════════════════════════════════════════════════════════════════
349391
# MCP STDIO SERVER LIFECYCLE
350392
# ══════════════════════════════════════════════════════════════════════════
@@ -1007,6 +1049,7 @@ inv_empty_repo_cli
10071049
inv_garbage_files_cli
10081050
inv_crasher_skipped_cli
10091051
inv_hanger_skipped_cli
1052+
inv_second_index_inprocess
10101053

10111054
# MCP server-lifecycle invariants (one shared server instance).
10121055
inv_mcp_initialize

‎src/pipeline/pass_parallel.c‎

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1067,7 +1067,13 @@ int cbm_parallel_extract_ex(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t *file
10671067
CBM_PROF_START(t_init);
10681068
cbm_init();
10691069

1070-
/* Slab allocator for tree-sitter (thread-safe via TLS). */
1070+
/* Slab allocator for tree-sitter (thread-safe via TLS). Destroy any
1071+
* parser this thread still holds BEFORE switching the global ts
1072+
* allocator: a parser created in the mimalloc epoch (sequential run,
1073+
* watcher tick) must be freed by the allocator that created it, or its
1074+
* teardown after the switch routes mi pointers into plain free()
1075+
* (#773). */
1076+
cbm_destroy_thread_parser();
10711077
cbm_slab_install();
10721078
CBM_PROF_END("parallel_extract", "1_init_libs", t_init);
10731079

‎src/pipeline/pipeline.c‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -832,6 +832,14 @@ static int run_sequential_pipeline(cbm_pipeline_t *p, cbm_pipeline_ctx_t *ctx,
832832
cbm_arena_destroy(&ctx->seq_cross_arena);
833833
ctx->seq_cross_arena_live = false;
834834
}
835+
/* Destroy this thread's TLS parser: the sequential path parses on the
836+
* CALLING thread (usually main), and a parser left alive here was
837+
* allocated in the current tree-sitter allocator epoch. A later
838+
* parallel run switches the global ts allocator to the slab
839+
* (cbm_slab_install); destroying the stale parser then frees
840+
* mimalloc-epoch memory through slab_free -> plain free() and libmalloc
841+
* aborts — the #773 second-index SIGABRT. */
842+
cbm_destroy_thread_parser();
835843
return rc;
836844
}
837845

‎tests/test_mcp.c‎

Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4870,6 +4870,124 @@ static int idxpar_recovery_check(const char *repo_dir) {
48704870
}
48714871
#endif /* !_WIN32 */
48724872

4873+
/* #773: SIGABRT (invalid free in ts_stack_delete via
4874+
* cbm_destroy_thread_parser) on the SECOND index_repository in one server
4875+
* process, once both repos take the PARALLEL path (~30+ files). The
4876+
* supervisor masks this on the default MCP path (fresh worker process per
4877+
* index); the in-process pipeline — CBM_INDEX_SUPERVISOR=0, and every
4878+
* embedded/test consumer — dies. Forked child so the abort cannot kill the
4879+
* runner; ASan legs print the exact bad free. */
4880+
enum {
4881+
IDX773_OK = 0,
4882+
IDX773_FIRST_FAILED = 71, /* first index didn't return indexed */
4883+
IDX773_SECOND_FAILED = 72, /* second index didn't return indexed */
4884+
};
4885+
4886+
#ifndef _WIN32
4887+
static void idx773_write_py_repo(const char *dir, int files, int variant) {
4888+
for (int i = 0; i < files; i++) {
4889+
char path[CBM_SZ_512];
4890+
snprintf(path, sizeof(path), "%s/mod_%d_%03d.py", dir, variant, i);
4891+
FILE *f = fopen(path, "w");
4892+
if (!f) {
4893+
continue;
4894+
}
4895+
fprintf(f,
4896+
"class Handler%d:\n"
4897+
" def run(self, x):\n"
4898+
" return self.helper(x) + %d\n"
4899+
" def helper(self, x):\n"
4900+
" for i in range(10):\n"
4901+
" x += i\n"
4902+
" return x\n"
4903+
"\n"
4904+
"def main_%d(x):\n"
4905+
" return Handler%d().run(x)\n",
4906+
i, i, i, i);
4907+
fclose(f);
4908+
}
4909+
}
4910+
4911+
static int idx773_double_index_check(const char *dir_a, const char *dir_b) {
4912+
cbm_setenv("CBM_INDEX_SUPERVISOR", "0", 1);
4913+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
4914+
if (!srv) {
4915+
return IDX773_FIRST_FAILED;
4916+
}
4917+
char args[CBM_SZ_512];
4918+
snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"mode\":\"full\"}", dir_a);
4919+
char *r1 = cbm_mcp_handle_tool(srv, "index_repository", args);
4920+
bool ok1 = r1 && strstr(r1, "indexed") != NULL;
4921+
free(r1);
4922+
if (!ok1) {
4923+
cbm_mcp_server_free(srv);
4924+
return IDX773_FIRST_FAILED;
4925+
}
4926+
snprintf(args, sizeof(args), "{\"repo_path\":\"%s\",\"mode\":\"full\"}", dir_b);
4927+
char *r2 = cbm_mcp_handle_tool(srv, "index_repository", args); /* SIGABRT here (RED) */
4928+
bool ok2 = r2 && strstr(r2, "indexed") != NULL;
4929+
free(r2);
4930+
cbm_mcp_server_free(srv);
4931+
return ok2 ? IDX773_OK : IDX773_SECOND_FAILED;
4932+
}
4933+
#endif /* !_WIN32 */
4934+
4935+
TEST(index_second_inprocess_run_survives_issue773) {
4936+
#ifdef _WIN32
4937+
SKIP_PLATFORM("fork-isolated crash guard (POSIX-only)");
4938+
#else
4939+
char dir_a[CBM_SZ_256];
4940+
char dir_b[CBM_SZ_256];
4941+
char cache[CBM_SZ_256];
4942+
snprintf(dir_a, sizeof(dir_a), "/tmp/cbm-idx773a-XXXXXX");
4943+
snprintf(dir_b, sizeof(dir_b), "/tmp/cbm-idx773b-XXXXXX");
4944+
snprintf(cache, sizeof(cache), "/tmp/cbm-idx773c-XXXXXX");
4945+
if (!cbm_mkdtemp(dir_a) || !cbm_mkdtemp(dir_b) || !cbm_mkdtemp(cache)) {
4946+
FAIL("mkdtemp failed");
4947+
}
4948+
const char *saved_cache = getenv("CBM_CACHE_DIR");
4949+
char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL;
4950+
cbm_setenv("CBM_CACHE_DIR", cache, 1);
4951+
4952+
/* Trigger shape: run 1 small enough for the SEQUENTIAL path (parses on
4953+
* the calling thread, mimalloc epoch), run 2 large enough for the
4954+
* PARALLEL path (switches the global ts allocator to the slab). */
4955+
idx773_write_py_repo(dir_a, 5, 0);
4956+
idx773_write_py_repo(dir_b, 60, 1);
4957+
4958+
int code = -1;
4959+
bool signalled = false;
4960+
int sig = 0;
4961+
fflush(NULL);
4962+
pid_t pid = fork();
4963+
if (pid == 0) {
4964+
alarm(180); /* generous: two full parallel indexes */
4965+
_exit(idx773_double_index_check(dir_a, dir_b));
4966+
}
4967+
ASSERT_TRUE(pid > 0);
4968+
int status = 0;
4969+
(void)waitpid(pid, &status, 0);
4970+
if (WIFEXITED(status)) {
4971+
code = WEXITSTATUS(status);
4972+
} else if (WIFSIGNALED(status)) {
4973+
signalled = true;
4974+
sig = WTERMSIG(status);
4975+
}
4976+
4977+
restore_cache_dir(saved_cache_copy);
4978+
free(saved_cache_copy);
4979+
4980+
if (signalled) {
4981+
printf(" child killed by signal %d (SIGABRT = the #773 invalid free)\n", sig);
4982+
} else if (code != IDX773_OK) {
4983+
printf(" child exit code %d (71=first index failed, 72=second failed)\n", code);
4984+
}
4985+
ASSERT_FALSE(signalled);
4986+
ASSERT_EQ(code, IDX773_OK);
4987+
PASS();
4988+
#endif
4989+
}
4990+
48734991
TEST(index_recovery_parallel_quarantines_crasher) {
48744992
#ifdef _WIN32
48754993
SKIP_PLATFORM("parallel-recovery guard needs fork isolation (POSIX-only)");
@@ -5464,6 +5582,7 @@ SUITE(mcp) {
54645582
RUN_TEST(index_repository_cli_name_override_issue823);
54655583
RUN_TEST(index_supervisor_gate_requires_marked_host_issue845);
54665584
RUN_TEST(index_bg_paths_route_through_supervisor_issue832);
5585+
RUN_TEST(index_second_inprocess_run_survives_issue773);
54675586
RUN_TEST(index_recovery_parallel_quarantines_crasher);
54685587
RUN_TEST(tool_manage_adr_not_found_rich_error);
54695588
RUN_TEST(tool_manage_adr_get_accepts_abs_path);

0 commit comments

Comments
 (0)