diff --git a/tests/test_cli.c b/tests/test_cli.c index c325f399e..8fa842b21 100644 --- a/tests/test_cli.c +++ b/tests/test_cli.c @@ -1572,15 +1572,19 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) { * install under test and asks the daemon itself afterwards. */ #define CLI_SCOPE_HOST_DRAINED 3 #define CLI_SCOPE_TIMEOUT_MS 5000U -/* The host child's readiness has NO deadline (USER RULE: no timeout decides a - * test). The child answers 'R' or 'E', or the pipe reaches EOF when it dies, - * and the fixture names the failing step in every case. A child that wedges - * before answering is left to the harness's per-suite ceiling. A 90 s bound - * used to decide this wait, and on a starved machine (2026-10-09, during a - * kernel index) ASSERT(ready) failed with nothing in the log to say why. - * Generous, bounded waits remain for reaping and status probes below. */ -#define CLI_SCOPE_HOST_REAP_TIMEOUT_MS 60000U -#define CLI_SCOPE_HOST_SERVING_TIMEOUT_MS 15000U +/* No timeout decides a cli_scope test (USER RULE). That covers the host child's + * readiness, its cohort join, the parent's client connect, the "still serving" + * probe, and the reap: + * - the child answers 'R' or 'E', or the pipe reaches EOF when it dies, and the + * fixture names the failing step in every case; + * - each IPC attempt keeps the API's finite CLI_SCOPE_TIMEOUT_MS budget, but an + * attempt that reaches a live, mute endpoint (a loaded runner answering + * slowly) is repeated, and only a reply or a definite refusal decides + * (cli_scope_until_answered); + * - a child that wedges is left to the harness's per-suite ceiling. + * A 90 s readiness bound, a 15 s status budget and a 60 s reap used to decide + * these; on a starved machine (2026-10-09, during a kernel index) + * ASSERT(ready) failed with nothing in the log to say why. */ /* Teardown budget for a child the activation ALREADY drained: its service is * gone, so each free/release succeeds immediately and this is only the * give-up point if one unexpectedly does not. The undrained path keeps the @@ -1588,9 +1592,10 @@ TEST(cli_install_recovers_markerless_stale_rendezvous) { #define CLI_SCOPE_CLEANUP_DRAINED_MS 1000U /* Budget for probing that an ALREADY-drained host has stopped serving. * - * The generous CLI_SCOPE_HOST_SERVING_TIMEOUT_MS exists for the POSITIVE - * question -- "is this daemon still up?" -- where a slow reply on a loaded - * runner must not be misread as drained. Asked in the negative it inverts: + * The positive question -- "is this daemon still up?" -- repeats a mute + * attempt until the daemon answers (cli_scope_host_serving), because a slow + * reply on a loaded runner must not be misread as drained. Asked in the + * negative it inverts: * there is no reply coming, so the whole budget is spent proving silence, and * the assertion is decided by a timeout expiring rather than by the system's * own behaviour. That was 15 s of the drain test's wall clock and the largest @@ -1620,6 +1625,7 @@ typedef struct { char *old_home; char *old_cache; char *old_shell; + bool env_saved; /* HOME, CBM_CACHE_DIR and SHELL were saved for finish */ char failed_step[160]; /* the setup step that failed, "" when ready */ } cli_scope_fixture_t; @@ -1634,8 +1640,10 @@ static _Noreturn void cli_scope_host_child(const cli_scope_fixture_t *fixture, i cbm_daemon_runtime_service_t *service = NULL; int acquire_status = -1; if (endpoint && manager) { - acquire_status = (int)cbm_version_cohort_acquire(manager, &fixture->identity, - cbm_now_ms() + 45000U, &lease, &conflict); + /* UINT64_MAX: wait as long as a lock is held, never for a conflicting + * holder (that is answered at once), so no deadline decides the join. */ + acquire_status = (int)cbm_version_cohort_acquire(manager, &fixture->identity, UINT64_MAX, + &lease, &conflict); } bool admitted = acquire_status == (int)CBM_VERSION_COHORT_OK; if (admitted) { @@ -1745,32 +1753,60 @@ static bool cli_scope_setup_failed(cli_scope_fixture_t *fixture, const char *tag return false; } -/* Reap the host child within a bound: the release signal makes a healthy child - * break within ~50 ms and finish teardown in a few seconds, so a child still - * alive past the deadline is wedged — SIGKILL it and reap so the suite always - * makes progress. Returns the child's exit code, or -1 when it had to be - * killed or did not exit cleanly; a -1 fails the caller's host_exit assertion - * cleanly rather than hanging. */ -static int cli_scope_reap_host(pid_t host, uint32_t timeout_ms) { - uint64_t deadline = cbm_now_ms() + timeout_ms; +/* Reap the host child, with no deadline: the release signal makes a healthy + * child exit within seconds, and a wedged one is left to the harness's + * per-suite ceiling (see CLI_SCOPE_TIMEOUT_MS). Returns the child's exit code, + * or -1 when it did not exit normally. */ +static int cli_scope_reap_host(pid_t host) { int status = 0; for (;;) { - pid_t reaped = waitpid(host, &status, WNOHANG); + pid_t reaped = waitpid(host, &status, 0); if (reaped == host) { return WIFEXITED(status) ? WEXITSTATUS(status) : -1; } if (reaped < 0 && errno != EINTR) { return -1; } - if (cbm_now_ms() >= deadline) { - (void)kill(host, SIGKILL); - (void)waitpid(host, &status, 0); - return -1; + } +} + +/* One IPC attempt's outcome for cli_scope_until_answered. */ +typedef enum { + CLI_SCOPE_ANSWERED, /* a reply arrived */ + CLI_SCOPE_REFUSED, /* definite: no endpoint, a refusal, or a conflict */ + CLI_SCOPE_MUTE, /* a live process holds the endpoint but did not answer in time */ +} cli_scope_attempt_t; + +typedef cli_scope_attempt_t (*cli_scope_attempt_fn)(void *context); + +/* Repeat an IPC attempt while it reaches a live but mute endpoint, so a slow + * reply on a loaded runner is waited for instead of failing the test. A reply + * or a definite refusal ends it. Each attempt keeps the API's finite budget. + * Returns true when an attempt was answered. */ +static bool cli_scope_until_answered(cli_scope_attempt_fn attempt, void *context) { + for (;;) { + cli_scope_attempt_t outcome = attempt(context); + if (outcome != CLI_SCOPE_MUTE) { + return outcome == CLI_SCOPE_ANSWERED; } - cbm_usleep(2000); } } +/* One attempt to connect the parent's committed client to the host. */ +static cli_scope_attempt_t cli_scope_connect_attempt(void *context) { + cli_scope_fixture_t *fixture = context; + cbm_daemon_runtime_connect_result_t result = {0}; + fixture->client = cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity, + CLI_SCOPE_TIMEOUT_MS, &result); + if (fixture->client) { + return CLI_SCOPE_ANSWERED; + } + return result.status == CBM_DAEMON_RUNTIME_CONNECT_ERROR && + result.muted_endpoint_holder_pid != 0 + ? CLI_SCOPE_MUTE + : CLI_SCOPE_REFUSED; +} + static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *tag) { memset(fixture, 0, sizeof(*fixture)); fixture->host = -1; @@ -1842,6 +1878,7 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta cli_activation_save_env(&fixture->old_home, &fixture->old_cache); const char *shell = getenv("SHELL"); fixture->old_shell = shell ? strdup(shell) : NULL; + fixture->env_saved = true; cbm_setenv("SHELL", "/bin/zsh", 1); if (child < 0) { return cli_scope_setup_failed(fixture, tag, "forking the host", fork_error); @@ -1854,11 +1891,9 @@ static bool cli_scope_fixture_start(cli_scope_fixture_t *fixture, const char *ta 0); } fixture->endpoint = cbm_daemon_bootstrap_endpoint_new(fixture->runtime_parent); - cbm_daemon_runtime_connect_result_t connect_result = {0}; - fixture->client = fixture->endpoint - ? cbm_daemon_runtime_client_connect(fixture->endpoint, &fixture->identity, - CLI_SCOPE_TIMEOUT_MS, &connect_result) - : NULL; + if (fixture->endpoint) { + (void)cli_scope_until_answered(cli_scope_connect_attempt, fixture); + } if (!fixture->client) { return cli_scope_setup_failed(fixture, tag, fixture->endpoint ? "connecting the parent's client" @@ -1876,15 +1911,33 @@ static bool cli_scope_host_serving_within(const cli_scope_fixture_t *fixture, ui !status.stopping && status.committed_clients == 1; } +typedef struct { + const cli_scope_fixture_t *fixture; + cbm_daemon_runtime_status_t status; +} cli_scope_status_probe_t; + +/* One status request. A failed request still reports the PID of a live but + * mute endpoint holder, which is what tells slow apart from gone. */ +static cli_scope_attempt_t cli_scope_status_attempt(void *context) { + cli_scope_status_probe_t *probe = context; + memset(&probe->status, 0, sizeof(probe->status)); + if (cbm_daemon_runtime_request_status(probe->fixture->endpoint, &probe->fixture->identity, + CLI_SCOPE_TIMEOUT_MS, &probe->status)) { + return CLI_SCOPE_ANSWERED; + } + return probe->status.muted_endpoint_holder_pid != 0 ? CLI_SCOPE_MUTE : CLI_SCOPE_REFUSED; +} + /* Ask the host daemon itself: still running, not stopping, and the parent's - * committed client still admitted. */ + * committed client still admitted. A foreign-namespace install leaves this + * daemon serving, so a slow reply on a loaded runner must not be misread as + * "drained" (the flaky failure this fixture showed): a mute attempt is + * repeated until the daemon answers, while a drained daemon is unreachable or + * reports stopping, which decides at once. */ static bool cli_scope_host_serving(const cli_scope_fixture_t *fixture) { - /* A generous, bounded status deadline: a foreign-namespace install leaves - * this daemon serving, so a slow response on a loaded runner must not be - * misread as "drained" (the flaky failure this fixture showed). The call - * still fails cleanly — a genuinely drained daemon is unreachable or - * reports stopping — it just no longer decides survival on a 5 s budget. */ - return cli_scope_host_serving_within(fixture, CLI_SCOPE_HOST_SERVING_TIMEOUT_MS); + cli_scope_status_probe_t probe = {.fixture = fixture}; + return fixture->endpoint && cli_scope_until_answered(cli_scope_status_attempt, &probe) && + !probe.status.stopping && probe.status.committed_clients == 1; } static int cli_scope_install(cli_scope_fixture_t *fixture, const char *home, const char *cache, @@ -1917,7 +1970,7 @@ static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) { } int host_exit = -1; if (fixture->host > 0) { - host_exit = cli_scope_reap_host(fixture->host, CLI_SCOPE_HOST_REAP_TIMEOUT_MS); + host_exit = cli_scope_reap_host(fixture->host); fixture->host = -1; } cbm_daemon_ipc_endpoint_free(fixture->endpoint); @@ -1925,14 +1978,20 @@ static int cli_scope_fixture_finish(cli_scope_fixture_t *fixture) { if (fixture->previous_supervisor_build[0]) { cbm_index_supervisor_set_build_fingerprint_for_test(fixture->previous_supervisor_build); } - if (fixture->old_shell) { - cbm_setenv("SHELL", fixture->old_shell, 1); - } else { - cbm_unsetenv("SHELL"); + /* Restore only what start saved: a setup that failed before saving left + * HOME, CBM_CACHE_DIR and SHELL untouched, and restoring "nothing saved" + * would unset them for every later test in the suite. */ + if (fixture->env_saved) { + if (fixture->old_shell) { + cbm_setenv("SHELL", fixture->old_shell, 1); + } else { + cbm_unsetenv("SHELL"); + } + cli_activation_restore_env(fixture->old_home, fixture->old_cache); + fixture->env_saved = false; } free(fixture->old_shell); fixture->old_shell = NULL; - cli_activation_restore_env(fixture->old_home, fixture->old_cache); fixture->old_home = NULL; fixture->old_cache = NULL; test_rmdir_r(fixture->tmpdir); @@ -2080,6 +2139,71 @@ TEST(cli_scope_fixture_names_its_failing_setup_step) { PASS(); } +/* Every cli_scope test calls finish even when start failed. A start that + * failed before saving HOME, CBM_CACHE_DIR and SHELL used to have finish + * "restore" them to nothing, unsetting them for every later test. */ +TEST(cli_scope_fixture_finish_keeps_env_after_an_early_failure) { + char *saved_home = save_test_env("HOME"); + char *saved_cache = save_test_env("CBM_CACHE_DIR"); + char *saved_shell = save_test_env("SHELL"); + cbm_setenv("HOME", "/tmp/cli-scope-sentinel-home", 1); + cbm_setenv("CBM_CACHE_DIR", "/tmp/cli-scope-sentinel-cache", 1); + cbm_setenv("SHELL", "/bin/sentinel-shell", 1); + cli_scope_fixture_t fixture; + bool ready = cli_scope_fixture_start(&fixture, "no-such-parent/y"); + (void)cli_scope_fixture_finish(&fixture); + const char *home = getenv("HOME"); + const char *cache = getenv("CBM_CACHE_DIR"); + const char *shell = getenv("SHELL"); + bool home_kept = home && strcmp(home, "/tmp/cli-scope-sentinel-home") == 0; + bool cache_kept = cache && strcmp(cache, "/tmp/cli-scope-sentinel-cache") == 0; + bool shell_kept = shell && strcmp(shell, "/bin/sentinel-shell") == 0; + restore_test_env("HOME", saved_home); + restore_test_env("CBM_CACHE_DIR", saved_cache); + restore_test_env("SHELL", saved_shell); + + ASSERT_FALSE(ready); + ASSERT_TRUE(home_kept); + ASSERT_TRUE(cache_kept); + ASSERT_TRUE(shell_kept); + PASS(); +} + +/* A scripted sequence of attempt outcomes for cli_scope_until_answered. */ +typedef struct { + const cli_scope_attempt_t *outcomes; + int count; + int calls; +} cli_scope_scripted_attempts_t; + +static cli_scope_attempt_t cli_scope_scripted_attempt(void *context) { + cli_scope_scripted_attempts_t *script = context; + int index = script->calls < script->count ? script->calls : script->count - 1; + script->calls++; + return script->outcomes[index]; +} + +/* The rule that replaced the status and connect timeouts: a mute endpoint is + * asked again, and only a reply or a definite refusal decides. */ +TEST(cli_scope_retries_a_mute_endpoint_until_it_answers) { + static const cli_scope_attempt_t slow_then_answer[] = {CLI_SCOPE_MUTE, CLI_SCOPE_MUTE, + CLI_SCOPE_MUTE, CLI_SCOPE_ANSWERED}; + cli_scope_scripted_attempts_t slow = {slow_then_answer, 4, 0}; + ASSERT_TRUE(cli_scope_until_answered(cli_scope_scripted_attempt, &slow)); + ASSERT_EQ(slow.calls, 4); + + static const cli_scope_attempt_t slow_then_refused[] = {CLI_SCOPE_MUTE, CLI_SCOPE_REFUSED}; + cli_scope_scripted_attempts_t refused = {slow_then_refused, 2, 0}; + ASSERT_FALSE(cli_scope_until_answered(cli_scope_scripted_attempt, &refused)); + ASSERT_EQ(refused.calls, 2); + + static const cli_scope_attempt_t answered_at_once[] = {CLI_SCOPE_ANSWERED}; + cli_scope_scripted_attempts_t at_once = {answered_at_once, 1, 0}; + ASSERT_TRUE(cli_scope_until_answered(cli_scope_scripted_attempt, &at_once)); + ASSERT_EQ(at_once.calls, 1); + PASS(); +} + /* cli_scope_install with the activation's stdout captured into out. */ static int cli_scope_install_captured(cli_scope_fixture_t *fixture, const char *home, const char *cache, const char *bin_dir, bool skip_binary, @@ -17940,6 +18064,8 @@ SUITE(cli) { RUN_TEST(cli_install_into_host_namespace_still_drains_host_cohort); RUN_TEST(cli_install_skip_binary_unchanged_in_host_namespace_quiesces_nothing); RUN_TEST(cli_scope_fixture_names_its_failing_setup_step); + RUN_TEST(cli_scope_fixture_finish_keeps_env_after_an_early_failure); + RUN_TEST(cli_scope_retries_a_mute_endpoint_until_it_answers); RUN_TEST(cli_install_foreign_home_unreadable_cohort_cache_keeps_host_daemon); RUN_TEST(cli_install_foreign_home_busy_cohort_keeps_host_daemon); #endif