mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-05 08:23:53 +08:00
fix(subprocess): a child that is exiting at the group check is a finished run
macOS answers ESRCH to setpgid and getpgid for our own unreaped child from
the moment it starts to exit, before waitid can report it. a5c7ae97 accepted
a child that had already exited, probed with waitid(WNOHANG|WNOWAIT); in the
window before the child is reapable that probe said "not exited", and the
spawn was reported as SPAWN_FAILED errno 3. macos-15-intel CI kept hitting it
in a different Git-backed fixture on each run; with the new diagnostics it
named itself: `git config --local core.protectNTFS false`, outcome=5 errno=3.
A child of ours that setpgid/getpgid cannot find is not running: it has
exited or is exiting. The parent now waits until it is reapable (WNOWAIT
leaves it for the supervision, which reaps it with its real exit status);
the wait ends with the child's exit. EACCES/EPERM keep the group check.
A test seam makes the parent observe the exiting state (ESRCH, not yet
reapable): subprocess_run_child_exiting_at_group_check is SPAWN_FAILED
without this change and exit status 3 with it.
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This commit is contained in:
+42
-13
@@ -1124,6 +1124,10 @@ static int g_hold_until_exit = 0;
|
||||
void cbm_subprocess_hold_parent_until_child_exits_for_testing(int spawns) {
|
||||
g_hold_until_exit = spawns > 0 ? spawns : 0;
|
||||
}
|
||||
static int g_observe_exiting = 0;
|
||||
void cbm_subprocess_observe_exiting_child_for_testing(int spawns) {
|
||||
g_observe_exiting = spawns > 0 ? spawns : 0;
|
||||
}
|
||||
int cbm_subprocess_pending_spawn_eagain_for_testing(void) {
|
||||
return g_force_spawn_eagain;
|
||||
}
|
||||
@@ -1400,12 +1404,22 @@ static int cbm_posix_spawn_apple(cbm_subprocess_t *process, int input, int outpu
|
||||
}
|
||||
#endif
|
||||
|
||||
/* The child exited and waits to be reaped (WNOWAIT leaves it for the
|
||||
* supervision to collect with its exit status). */
|
||||
static bool cbm_posix_child_exited(pid_t pid) {
|
||||
siginfo_t info;
|
||||
memset(&info, 0, sizeof(info));
|
||||
return waitid(P_PID, (id_t)pid, &info, WEXITED | WNOHANG | WNOWAIT) == 0 && info.si_pid == pid;
|
||||
/* Our own unreaped child that setpgid/getpgid cannot find (ESRCH) is not
|
||||
* running: it has exited, or is exiting -- macOS stops finding a child as it
|
||||
* starts to exit, before waitid can report it. Wait until it is reapable;
|
||||
* the wait ends with its exit, and WNOWAIT leaves it for the supervision to
|
||||
* collect with its exit status. */
|
||||
static bool cbm_posix_child_reapable(pid_t pid) {
|
||||
for (;;) {
|
||||
siginfo_t info;
|
||||
memset(&info, 0, sizeof(info));
|
||||
if (waitid(P_PID, (id_t)pid, &info, WEXITED | WNOWAIT) == 0) {
|
||||
return info.si_pid == pid;
|
||||
}
|
||||
if (errno != EINTR) {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) {
|
||||
@@ -1543,15 +1557,30 @@ static int cbm_subprocess_spawn_posix(cbm_subprocess_t *process) {
|
||||
#endif
|
||||
/* Parent and child both establish the group, removing scheduler-order races.
|
||||
* If the child won and already execed, EACCES is accepted only after proving
|
||||
* that its process group is the expected isolated one. A child that already
|
||||
* EXITED is a finished run, not a failed spawn: macOS no longer shows an
|
||||
* unreaped child to setpgid/getpgid (ESRCH), and on a loaded machine a short
|
||||
* command (`git write-tree`) finishes before the parent gets here. Its group
|
||||
* was set before exec, by the spawn attributes or by the child itself; the
|
||||
* that its process group is the expected isolated one. A child that has
|
||||
* EXITED, or is exiting, is a finished run, not a failed spawn: macOS no
|
||||
* longer shows such an unreaped child to setpgid/getpgid (ESRCH), and on a
|
||||
* loaded machine a short command (`git config`) gets there before the
|
||||
* parent does. Its group was set before exec, by the spawn attributes or by
|
||||
* the child itself; the parent waits until it is reapable, and the
|
||||
* supervision below reaps it with its real exit status. */
|
||||
bool contained = setpgid(pid, pid) == 0;
|
||||
bool observe_exiting = false;
|
||||
#ifdef CBM_ENABLE_TEST_SEAMS
|
||||
if (g_observe_exiting > 0) {
|
||||
g_observe_exiting--;
|
||||
observe_exiting = true;
|
||||
}
|
||||
#endif
|
||||
bool contained = !observe_exiting && setpgid(pid, pid) == 0;
|
||||
if (observe_exiting) {
|
||||
errno = ESRCH;
|
||||
}
|
||||
if (!contained && (errno == EACCES || errno == EPERM || errno == ESRCH)) {
|
||||
contained = getpgid(pid) == pid || cbm_posix_child_exited(pid);
|
||||
bool missing = errno == ESRCH;
|
||||
contained = !observe_exiting && getpgid(pid) == pid;
|
||||
if (!contained && (missing || errno == ESRCH)) {
|
||||
contained = cbm_posix_child_reapable(pid);
|
||||
}
|
||||
}
|
||||
if (!contained) {
|
||||
(void)kill(pid, SIGKILL);
|
||||
|
||||
@@ -211,6 +211,10 @@ int cbm_subprocess_pending_spawn_eagain_for_testing(void);
|
||||
* machine produces when a short command finishes before the parent runs
|
||||
* again. Test builds only. */
|
||||
void cbm_subprocess_hold_parent_until_child_exits_for_testing(int spawns);
|
||||
/* For the next N POSIX spawns, the parent's group check observes what macOS
|
||||
* 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);
|
||||
#endif
|
||||
|
||||
/* How the POSIX fork+exec child closed its inherited descriptors (#1484). */
|
||||
|
||||
@@ -155,6 +155,23 @@ TEST(subprocess_run_child_exited_before_group_check) {
|
||||
#endif
|
||||
}
|
||||
|
||||
/* The window before that: macOS answers ESRCH for a child that has started
|
||||
* exiting, before waitid can report it. A non-blocking "has it exited?"
|
||||
* probe said no there, and macos-15-intel CI read a short `git config` as
|
||||
* SPAWN_FAILED errno 3. The seam makes the parent observe exactly that. */
|
||||
TEST(subprocess_run_child_exiting_at_group_check) {
|
||||
#ifdef _WIN32
|
||||
SKIP_PLATFORM("POSIX process groups");
|
||||
#else
|
||||
cbm_subprocess_observe_exiting_child_for_testing(1);
|
||||
cbm_proc_result_t r = run_sh("exit 3", 0);
|
||||
cbm_subprocess_observe_exiting_child_for_testing(0);
|
||||
ASSERT_EQ(r.outcome, CBM_PROC_EXIT_NONZERO);
|
||||
ASSERT_EQ(r.exit_code, 3);
|
||||
PASS();
|
||||
#endif
|
||||
}
|
||||
|
||||
/* Daemon background helpers intentionally invoke fixed tool names such as
|
||||
* `curl` and `git`. A shell-free spawn must still perform the normal PATH
|
||||
* lookup for a name without a directory separator; exact binary paths keep
|
||||
@@ -2346,6 +2363,7 @@ SUITE(subprocess) {
|
||||
RUN_TEST(subprocess_run_clean);
|
||||
RUN_TEST(subprocess_run_exit_nonzero);
|
||||
RUN_TEST(subprocess_run_child_exited_before_group_check);
|
||||
RUN_TEST(subprocess_run_child_exiting_at_group_check);
|
||||
RUN_TEST(subprocess_run_resolves_literal_binary_name_from_path);
|
||||
RUN_TEST(subprocess_run_crash_is_crash);
|
||||
RUN_TEST(subprocess_run_hang_is_hang);
|
||||
|
||||
Reference in New Issue
Block a user