Skip to content

Commit 4b3ab38

Browse files
authored
Merge pull request #1438 from shashwatMishra96/fix/quarantine-exit-nonzero
fix: quarantine files on EXIT_NONZERO instead of aborting chunk
2 parents f899770 + 3ffcbc9 commit 4b3ab38

5 files changed

Lines changed: 280 additions & 12 deletions

File tree

‎internal/cbm/cbm.c‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -725,6 +725,10 @@ static void cbm_test_fault_inject(const char *rel_path) {
725725
/* Busy-spin: the supervisor's quiet-timeout kills + reports us. */
726726
}
727727
}
728+
const char *exit_on = getenv("CBM_TEST_EXIT_ON");
729+
if (exit_on && exit_on[0] && strstr(rel_path, exit_on)) {
730+
exit(1); /* Nonzero exit code → CBM_PROC_EXIT_NONZERO → classified as "error" */
731+
}
728732
}
729733
#endif
730734

‎src/mcp/mcp.c‎

Lines changed: 29 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7756,7 +7756,7 @@ static bool supervisor_suspect_contains(char **s, int n, const char *rel) {
77567756
return false;
77577757
}
77587758

7759-
/* Append one quarantine entry "rel\tphase\n" (phase = "crash"|"hang") to the
7759+
/* Append one quarantine entry "rel\tphase\n" (phase = "crash"|"hang"|"error") to the
77607760
* quarantine list. The worker's loader parses this back and reports the skip's
77617761
* phase in skipped[]; a bare "rel" line is still tolerated there (defaults crash). */
77627762
static bool supervisor_append_quarantine(const char *path, const char *rel, const char *phase) {
@@ -7790,7 +7790,7 @@ cbm_mcp_supervised_result_disposition_t cbm_mcp_supervised_result_disposition(
77907790
* - the worker's own response on a clean first run (the common path);
77917791
* - after a crash/hang, the response from a clean single-threaded RECOVERY run
77927792
* that quarantines the culprit file(s) — status="indexed" with them listed in
7793-
* skipped[] as phase="crash"/"hang", and the good files indexed;
7793+
* skipped[] as phase="crash"/"hang"/"error", and the good files indexed;
77947794
* - a best-effort PARTIAL index (one final quarantine-only run) if the recovery
77957795
* loop cannot converge but at least one file was quarantined;
77967796
* - a contained-failure response only if even that cannot produce a clean run.
@@ -7839,7 +7839,7 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) {
78397839
* (for a hang the oldest still-open file IS the stuck one; for a crash
78407840
* it is the longest-running suspect — the best single deterministic
78417841
* pick). A clean run then indexes the good files and reports the
7842-
* quarantined ones as phase="crash"/"hang" skips via the ordinary
7842+
* quarantined ones as phase="crash"/"hang"/"error" skips via the ordinary
78437843
* Stage-2 skip plumbing. The old design re-ran SINGLE-THREADED to keep
78447844
* one exact marker; at scale that fell into the sequential crawl, went
78457845
* quiet, was killed as a hang mid-pass, and the stale marker got FOUR
@@ -7900,13 +7900,34 @@ static char *index_run_supervised(cbm_mcp_server_t *srv, const char *args) {
79007900
cbm_index_worker_result_free(&wr2);
79017901
break; /* good files indexed; quarantined files reported as crash/hang */
79027902
}
7903-
if (wr2.outcome == CBM_PROC_CRASH || wr2.outcome == CBM_PROC_HANG) {
7903+
if (wr2.outcome == CBM_PROC_CRASH || wr2.outcome == CBM_PROC_HANG ||
7904+
wr2.outcome == CBM_PROC_EXIT_NONZERO) {
79047905
last_outcome = wr2.outcome;
79057906
cbm_index_worker_result_free(&wr2);
7906-
/* crash vs hang: the phase this file is quarantined under and
7907-
* reported as in skipped[]. A fault signal → "crash"; a
7908-
* no-progress kill → "hang". */
7909-
const char *phase = (last_outcome == CBM_PROC_HANG) ? "hang" : "crash";
7907+
/* crash vs hang vs nonzero-exit: the phase this file is quarantined
7908+
* under and reported as in skipped[]. A fault signal → "crash"; a
7909+
* no-progress kill → "hang"; a graceful nonzero exit (e.g. an
7910+
* internal parse-limit/abort on a pathological file) → "error".
7911+
* EXIT_NONZERO is attributed via the SAME marker-journal suspect
7912+
* mechanism as a crash: the two-consecutive-strikes intersection
7913+
* below still guards against quarantining an innocent file, and a
7914+
* SYSTEMIC nonzero exit (e.g. a bad arg) produces no recurring
7915+
* suspect → the intersection is empty → give_up (correct). This
7916+
* makes a single pathological file skip-and-continue instead of
7917+
* aborting the whole chunk.
7918+
*
7919+
* Note: A deterministic first-file failure (e.g. worker crashing/exiting
7920+
* on the very first file every run) will progressively quarantine in-flight
7921+
* files until reaching the culprit. This is an existing property accepted
7922+
* by the crash/hang path, accepted here as a considered tradeoff. */
7923+
const char *phase;
7924+
if (last_outcome == CBM_PROC_HANG) {
7925+
phase = "hang";
7926+
} else if (last_outcome == CBM_PROC_EXIT_NONZERO) {
7927+
phase = "error";
7928+
} else {
7929+
phase = "crash";
7930+
}
79107931
int sus_n = 0;
79117932
char **suspects = supervisor_read_suspects(marker_path, &sus_n);
79127933
(void)remove(marker_path); /* fresh journal for the next re-run */

‎src/pipeline/pass_definitions.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -723,8 +723,9 @@ int cbm_pipeline_pass_definitions(cbm_pipeline_ctx_t *ctx, const cbm_file_info_t
723723
if (!phase) {
724724
phase = "crash";
725725
}
726-
const char *reason =
727-
(strcmp(phase, "hang") == 0) ? "quarantined after hang" : "quarantined after crash";
726+
const char *reason = (strcmp(phase, "hang") == 0) ? "quarantined after hang"
727+
: (strcmp(phase, "error") == 0) ? "quarantined after error"
728+
: "quarantined after crash";
728729
cbm_pipeline_add_file_error(ctx->pipeline, rel, reason, phase);
729730
errors++;
730731
continue;

‎src/pipeline/pass_parallel.c‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -800,8 +800,9 @@ static void extract_worker(int worker_id, void *ctx_ptr) {
800800
if (!phase) {
801801
phase = "crash";
802802
}
803-
const char *reason =
804-
(strcmp(phase, "hang") == 0) ? "quarantined after hang" : "quarantined after crash";
803+
const char *reason = (strcmp(phase, "hang") == 0) ? "quarantined after hang"
804+
: (strcmp(phase, "error") == 0) ? "quarantined after error"
805+
: "quarantined after crash";
805806
pp_err_add(errs, fi->rel_path, reason, phase);
806807
ws->errors++;
807808
continue;

‎tests/test_mcp.c‎

Lines changed: 241 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10048,6 +10048,8 @@ enum {
1004810048
IDXPAR_NO_QUARANTINE = 64, /* crasher missing from skipped[] */
1004910049
IDXPAR_INNOCENT_HIT = 65, /* a good file was quarantined/skipped */
1005010050
IDXPAR_GOOD_MISSING = 66, /* good file's Function absent from the store */
10051+
IDXPAR_NOT_ERROR = 67, /* systemic failure did not report status error */
10052+
IDXPAR_OUTCOME_WRONG = 68, /* systemic failure outcome is not exit_nonzero */
1005110053
};
1005210054

1005310055
#ifndef _WIN32
@@ -10107,6 +10109,94 @@ static int idxpar_recovery_check(const char *repo_dir) {
1010710109
free(project);
1010810110
return code;
1010910111
}
10112+
10113+
static int idxpar_exit_nonzero_recovery_check(const char *repo_dir) {
10114+
cbm_index_supervisor_mark_host();
10115+
cbm_unsetenv("CBM_INDEX_SUPERVISOR");
10116+
cbm_setenv("CBM_INDEX_MAX_RESTARTS", "5", 1);
10117+
cbm_setenv("CBM_INDEX_WORKER_TIMEOUT_S", "30", 1);
10118+
cbm_setenv("CBM_TEST_EXIT_ON", "idxpar_exit_nonzero", 1);
10119+
10120+
int st_before = cbm_index_supervisor_spawn_st_count();
10121+
char *resp = cbm_mcp_index_run_supervised_path(repo_dir);
10122+
int st_after = cbm_index_supervisor_spawn_st_count();
10123+
cbm_unsetenv("CBM_TEST_EXIT_ON");
10124+
10125+
if (st_after != st_before) {
10126+
free(resp);
10127+
return IDXPAR_ST_SPAWN;
10128+
}
10129+
if (!resp) {
10130+
return IDXPAR_NULL_RESP;
10131+
}
10132+
bool indexed = response_contains_json_fragment(resp, "\"status\":\"indexed\"");
10133+
bool offender_skipped = strstr(resp, "idxpar_exit_nonzero.py") != NULL;
10134+
bool innocent_hit =
10135+
strstr(resp, "idxpar_good_a.py") != NULL || strstr(resp, "idxpar_good_b.py") != NULL;
10136+
bool phase_error = strstr(resp, "\"phase\":\"error\"") != NULL ||
10137+
strstr(resp, "quarantined after error") != NULL;
10138+
free(resp);
10139+
if (!indexed) {
10140+
return IDXPAR_NOT_INDEXED;
10141+
}
10142+
if (!offender_skipped || !phase_error) {
10143+
return IDXPAR_NO_QUARANTINE;
10144+
}
10145+
if (innocent_hit) {
10146+
return IDXPAR_INNOCENT_HIT;
10147+
}
10148+
10149+
char *project = cbm_project_name_from_path(repo_dir);
10150+
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
10151+
int code = IDXPAR_OK;
10152+
if (srv && project) {
10153+
char q[512];
10154+
snprintf(q, sizeof(q),
10155+
"{\"project\":\"%s\",\"name_pattern\":\"idxpar_good_fn\",\"label\":\"Function\"}",
10156+
project);
10157+
char *sr = cbm_mcp_handle_tool(srv, "search_graph", q);
10158+
if (!sr || !strstr(sr, "idxpar_good_fn")) {
10159+
code = IDXPAR_GOOD_MISSING;
10160+
}
10161+
free(sr);
10162+
}
10163+
if (srv) {
10164+
cbm_mcp_server_free(srv);
10165+
}
10166+
free(project);
10167+
return code;
10168+
}
10169+
10170+
static int idxpar_systemic_exit_nonzero_give_up_check(const char *repo_dir) {
10171+
cbm_index_supervisor_mark_host();
10172+
cbm_unsetenv("CBM_INDEX_SUPERVISOR");
10173+
cbm_setenv("CBM_INDEX_MAX_RESTARTS", "5", 1);
10174+
cbm_setenv("CBM_INDEX_WORKER_TIMEOUT_S", "30", 1);
10175+
cbm_setenv("CBM_TEST_EXIT_ON", "idxpar_", 1);
10176+
10177+
char *resp = cbm_mcp_index_run_supervised_path(repo_dir);
10178+
cbm_unsetenv("CBM_TEST_EXIT_ON");
10179+
10180+
if (!resp) {
10181+
return IDXPAR_NULL_RESP;
10182+
}
10183+
bool is_error = response_contains_json_fragment(resp, "\"status\":\"error\"");
10184+
bool is_exit_nonzero = response_contains_json_fragment(resp, "\"outcome\":\"exit_nonzero\"");
10185+
bool innocent_hit =
10186+
strstr(resp, "idxpar_good_a.py") != NULL || strstr(resp, "idxpar_good_b.py") != NULL;
10187+
free(resp);
10188+
10189+
if (!is_error) {
10190+
return IDXPAR_NOT_ERROR;
10191+
}
10192+
if (!is_exit_nonzero) {
10193+
return IDXPAR_OUTCOME_WRONG;
10194+
}
10195+
if (innocent_hit) {
10196+
return IDXPAR_INNOCENT_HIT;
10197+
}
10198+
return IDXPAR_OK;
10199+
}
1011010200
#endif /* !_WIN32 */
1011110201

1011210202
/* #773: SIGABRT (invalid free in ts_stack_delete via
@@ -10422,6 +10512,155 @@ TEST(index_recovery_parallel_quarantines_crasher) {
1042210512
#endif
1042310513
}
1042410514

10515+
TEST(index_recovery_quarantines_exit_nonzero) {
10516+
#ifdef _WIN32
10517+
SKIP_PLATFORM("parallel-recovery guard needs fork isolation (POSIX-only)");
10518+
#else
10519+
char tmp_dir[CBM_SZ_256];
10520+
snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-idxpar-exit-XXXXXX");
10521+
if (!cbm_mkdtemp(tmp_dir)) {
10522+
FAIL("mkdtemp failed");
10523+
}
10524+
char cache[CBM_SZ_256];
10525+
snprintf(cache, sizeof(cache), "/tmp/cbm-idxpar-exit-cache-XXXXXX");
10526+
if (!cbm_mkdtemp(cache)) {
10527+
FAIL("mkdtemp cache failed");
10528+
}
10529+
const char *saved_cache = getenv("CBM_CACHE_DIR");
10530+
char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL;
10531+
cbm_setenv("CBM_CACHE_DIR", cache, 1);
10532+
10533+
char p1[CBM_SZ_512];
10534+
char p2[CBM_SZ_512];
10535+
char pc[CBM_SZ_512];
10536+
snprintf(p1, sizeof(p1), "%s/idxpar_good_a.py", tmp_dir);
10537+
snprintf(p2, sizeof(p2), "%s/idxpar_good_b.py", tmp_dir);
10538+
snprintf(pc, sizeof(pc), "%s/idxpar_exit_nonzero.py", tmp_dir);
10539+
FILE *f = fopen(p1, "w");
10540+
ASSERT_NOT_NULL(f);
10541+
fputs("def idxpar_good_fn():\n return 'ok'\n", f);
10542+
fclose(f);
10543+
f = fopen(p2, "w");
10544+
ASSERT_NOT_NULL(f);
10545+
fputs("def idxpar_good_fn_b():\n return 'ok'\n", f);
10546+
fclose(f);
10547+
f = fopen(pc, "w");
10548+
ASSERT_NOT_NULL(f);
10549+
fputs("def idxpar_bad_fn():\n return 'exit'\n", f);
10550+
fclose(f);
10551+
10552+
int code = -1;
10553+
bool signalled = false;
10554+
int sig = 0;
10555+
fflush(NULL);
10556+
pid_t pid = fork();
10557+
if (pid == 0) {
10558+
alarm(120);
10559+
_exit(idxpar_exit_nonzero_recovery_check(tmp_dir));
10560+
}
10561+
ASSERT_TRUE(pid > 0);
10562+
int status = 0;
10563+
(void)waitpid(pid, &status, 0);
10564+
if (WIFEXITED(status)) {
10565+
code = WEXITSTATUS(status);
10566+
} else if (WIFSIGNALED(status)) {
10567+
signalled = true;
10568+
sig = WTERMSIG(status);
10569+
}
10570+
10571+
char *project = cbm_project_name_from_path(tmp_dir);
10572+
cleanup_project_db(cache, project);
10573+
free(project);
10574+
restore_cache_dir(saved_cache_copy);
10575+
free(saved_cache_copy);
10576+
remove(p1);
10577+
remove(p2);
10578+
remove(pc);
10579+
cbm_rmdir(cache);
10580+
cbm_rmdir(tmp_dir);
10581+
10582+
if (signalled) {
10583+
printf(" child killed by signal %d\n", sig);
10584+
} else if (code != IDXPAR_OK) {
10585+
printf(" child exit code %d\n", code);
10586+
}
10587+
ASSERT_FALSE(signalled);
10588+
ASSERT_EQ(code, IDXPAR_OK);
10589+
PASS();
10590+
#endif
10591+
}
10592+
10593+
TEST(index_recovery_systemic_exit_nonzero_gives_up) {
10594+
#ifdef _WIN32
10595+
SKIP_PLATFORM("parallel-recovery guard needs fork isolation (POSIX-only)");
10596+
#else
10597+
char tmp_dir[CBM_SZ_256];
10598+
snprintf(tmp_dir, sizeof(tmp_dir), "/tmp/cbm-idxpar-sys-XXXXXX");
10599+
if (!cbm_mkdtemp(tmp_dir)) {
10600+
FAIL("mkdtemp failed");
10601+
}
10602+
char cache[CBM_SZ_256];
10603+
snprintf(cache, sizeof(cache), "/tmp/cbm-idxpar-sys-cache-XXXXXX");
10604+
if (!cbm_mkdtemp(cache)) {
10605+
FAIL("mkdtemp cache failed");
10606+
}
10607+
const char *saved_cache = getenv("CBM_CACHE_DIR");
10608+
char *saved_cache_copy = saved_cache ? cbm_strdup(saved_cache) : NULL;
10609+
cbm_setenv("CBM_CACHE_DIR", cache, 1);
10610+
10611+
char p1[CBM_SZ_512];
10612+
char p2[CBM_SZ_512];
10613+
snprintf(p1, sizeof(p1), "%s/idxpar_good_a.py", tmp_dir);
10614+
snprintf(p2, sizeof(p2), "%s/idxpar_good_b.py", tmp_dir);
10615+
FILE *f = fopen(p1, "w");
10616+
ASSERT_NOT_NULL(f);
10617+
fputs("def idxpar_good_fn():\n return 'ok'\n", f);
10618+
fclose(f);
10619+
f = fopen(p2, "w");
10620+
ASSERT_NOT_NULL(f);
10621+
fputs("def idxpar_good_fn_b():\n return 'ok'\n", f);
10622+
fclose(f);
10623+
10624+
int code = -1;
10625+
bool signalled = false;
10626+
int sig = 0;
10627+
fflush(NULL);
10628+
pid_t pid = fork();
10629+
if (pid == 0) {
10630+
alarm(120);
10631+
_exit(idxpar_systemic_exit_nonzero_give_up_check(tmp_dir));
10632+
}
10633+
ASSERT_TRUE(pid > 0);
10634+
int status = 0;
10635+
(void)waitpid(pid, &status, 0);
10636+
if (WIFEXITED(status)) {
10637+
code = WEXITSTATUS(status);
10638+
} else if (WIFSIGNALED(status)) {
10639+
signalled = true;
10640+
sig = WTERMSIG(status);
10641+
}
10642+
10643+
char *project = cbm_project_name_from_path(tmp_dir);
10644+
cleanup_project_db(cache, project);
10645+
free(project);
10646+
restore_cache_dir(saved_cache_copy);
10647+
free(saved_cache_copy);
10648+
remove(p1);
10649+
remove(p2);
10650+
cbm_rmdir(cache);
10651+
cbm_rmdir(tmp_dir);
10652+
10653+
if (signalled) {
10654+
printf(" child killed by signal %d\n", sig);
10655+
} else if (code != IDXPAR_OK) {
10656+
printf(" child exit code %d\n", code);
10657+
}
10658+
ASSERT_FALSE(signalled);
10659+
ASSERT_EQ(code, IDXPAR_OK);
10660+
PASS();
10661+
#endif
10662+
}
10663+
1042510664
/* ══════════════════════════════════════════════════════════════════
1042610665
* AUTO_WATCH GATE (distilled from PR #625)
1042710666
*
@@ -11407,6 +11646,8 @@ SUITE(mcp) {
1140711646
RUN_TEST(sequential_service_edge_props_are_valid_json_issue898);
1140811647
RUN_TEST(index_second_inprocess_run_survives_issue773);
1140911648
RUN_TEST(index_recovery_parallel_quarantines_crasher);
11649+
RUN_TEST(index_recovery_quarantines_exit_nonzero);
11650+
RUN_TEST(index_recovery_systemic_exit_nonzero_gives_up);
1141011651
RUN_TEST(tool_manage_adr_not_found_rich_error);
1141111652
RUN_TEST(tool_manage_adr_get_accepts_abs_path);
1141211653
RUN_TEST(tool_manage_adr_get_accepts_symlink_path);

0 commit comments

Comments
 (0)