lei.chen/myxce
1
0
Fork 1
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

fix(harness): reset getopt_long via optind=0 to stop cross-command corruption

Browse source 
Every cmd_* REPL handler in test.c/join_worker_args.c reset flag parsing
with `optind = 1;` only. glibc's getopt_long only fully reinitializes its
internal scan state (nextchar/first_nonopt/last_nonopt) when optind==0, or
on the process's very first getopt_long() call ever -- optind=1 alone
leaves that state stale for every call after the first. Since xce_test's
REPL dispatches many independent cmd_*() calls per process, a command
whose parse ends on a short no-argument flag (`qctrl -a`, `hostctrl -o`)
left state that corrupted the NEXT unrelated command's parse badly enough
to misread its own name as an unrecognized flag. This was the actual
cause of harness_unit_test's 4 deterministic (not flaky -- 20/20 identical
repeat runs) test_test_cli.c failures, each burning its full 5s
confirmation-prompt timeout. Fixed by using optind=0 at all 21 reset
sites; confirmed via 3 isolated pty repros plus 10 consecutive clean
harness_unit_test runs (132/132, was 4 failures/run every time).

Also fixes a resource leak found while investigating: test_test_cli.c's
spawned xce_test sessions were only torn down by inline
clitest_kill_and_reap()/clitest_close() calls at the bottom of each test
function, which Unity's longjmp-based TEST_ASSERT abort skips entirely on
failure, leaking the child process and its pty/pipe fds. Added a static
session mirror (safe across the longjmp, unlike a stack pointer) reaped
from harness_test.c's tearDown(), which Unity guarantees runs after every
test regardless of outcome.

Logged as bug-1807/bug-1808 in .wolf/buglog.json, with the underlying
getopt/longjmp gotchas recorded in .wolf/cerebrum.md. Full ASAN
/xce-build harness also passes 5/5 with no sanitizer findings.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-14 23:16:11 +00:00
parent cbf822fc53
commit 868c6427c8
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
9 changed files with 672 additions and 74 deletions
Download patch file Download diff file Expand all files Collapse all files

2
src/harness/join_worker_args.c
View file

@ -74,7 +74,7 @@ int join_worker_parse_attrs(int argc, char* argv[], hostattrs_t* attrs)
attrs->hostmodel = "Intel64";
attrs->hosttype = "LINUX64";
optind = 1;
optind = 0;
opterr = 0;
while ((opt = getopt_long(argc, argv, optstring, long_opts, NULL))

40
src/harness/test.c
View file

@ -1626,7 +1626,7 @@ int cmd_walcompare(int argc, char* argv[])
char optstring[8];
char posix_optstring[sizeof(optstring) + 1];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(walcompare_flags, WALCOMPARE_NFLAGS, long_opts,
@ -2043,7 +2043,7 @@ int cmd_hello(int argc, char* argv[])
struct option long_opts[HELLO_NFLAGS + 1];
char optstring[32];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(hello_flags, HELLO_NFLAGS, long_opts, optstring,
@ -2131,7 +2131,7 @@ int cmd_shutdown(int argc, char* argv[])
char optstring[16];
shutdown_call_t* call;
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(shutdown_flags, SHUTDOWN_NFLAGS, long_opts,
@ -2231,7 +2231,7 @@ int cmd_reslist(int argc, char* argv[])
struct option long_opts[RESLIST_NFLAGS + 1];
char optstring[8];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(reslist_flags, RESLIST_NFLAGS, long_opts,
@ -2374,7 +2374,7 @@ int cmd_workerlist(int argc, char* argv[])
struct option long_opts[WORKERLIST_NFLAGS + 1];
char optstring[8];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(workerlist_flags, WORKERLIST_NFLAGS, long_opts,
@ -2662,7 +2662,7 @@ int cmd_joblist(int argc, char* argv[])
struct option long_opts[JOBLIST_NFLAGS + 1];
char optstring[8];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(joblist_flags, JOBLIST_NFLAGS, long_opts,
@ -3238,7 +3238,7 @@ int cmd_jobinfo(int argc, char* argv[])
struct option long_opts[JOBINFO_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(jobinfo_flags, JOBINFO_NFLAGS, long_opts,
@ -3381,7 +3381,7 @@ int cmd_jobrun(int argc, char* argv[])
struct option long_opts[JOBRUN_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(jobrun_flags, JOBRUN_NFLAGS, long_opts, optstring,
@ -3550,7 +3550,7 @@ int cmd_jobsub(int argc, char* argv[])
struct option long_opts[JOBSUB_NFLAGS + 1];
char optstring[32];
optind = 1;
optind = 0;
opterr = 0;
memset(&submit, 0x0, sizeof(submit));
@ -3898,7 +3898,7 @@ int cmd_resval(int argc, char* argv[])
struct option long_opts[RESVAL_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(resval_flags, RESVAL_NFLAGS, long_opts, optstring,
@ -4046,7 +4046,7 @@ int cmd_jobkill(int argc, char* argv[])
struct option long_opts[JOBKILL_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
signo = SIGTERM;
@ -4304,7 +4304,7 @@ int cmd_qctrl(int argc, char* argv[])
struct option long_opts[QCTRL_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(qctrl_flags, QCTRL_NFLAGS, long_opts, optstring,
@ -4458,7 +4458,7 @@ int cmd_hostctrl(int argc, char* argv[])
struct option long_opts[HOSTCTRL_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(hostctrl_flags, HOSTCTRL_NFLAGS, long_opts,
@ -4607,7 +4607,7 @@ int cmd_upgradeconfig(int argc, char* argv[])
struct option long_opts[UPGRADECONFIG_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(upgradeconfig_flags, UPGRADECONFIG_NFLAGS,
@ -4776,7 +4776,7 @@ int cmd_runtasks(int argc, char* argv[])
struct option long_opts[RUNTASKS_NFLAGS + 1];
char optstring[32];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(runtasks_flags, RUNTASKS_NFLAGS, long_opts,
@ -4979,7 +4979,7 @@ int cmd_setpend(int argc, char* argv[])
struct option long_opts[SETPEND_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(setpend_flags, SETPEND_NFLAGS, long_opts,
@ -5175,7 +5175,7 @@ int cmd_jobstart(int argc, char* argv[])
struct option long_opts[JOBSTART_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
memset(&info, 0x0, sizeof(info));
@ -5286,7 +5286,7 @@ int cmd_jobfinish(int argc, char* argv[])
* and the command errors out) — matches the reset convention most other
* cmd_ handlers in this file already follow (e.g. cmd_join_worker just
* above, cmd_jobsub, cmd_resupdate). */
optind = 1;
optind = 0;
opterr = 0;
if (flagspec_build_opts(jobfinish_flags, JOBFINISH_NFLAGS, long_opts,
@ -5390,7 +5390,7 @@ int cmd_jobfail(int argc, char* argv[])
struct option long_opts[JOBFAIL_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
memset(&failinfo, 0x0, sizeof(failinfo));
@ -5732,7 +5732,7 @@ int cmd_resupdate(int argc, char* argv[])
struct option long_opts[RESUPDATE_NFLAGS + 1];
char optstring[16];
optind = 1;
optind = 0;
opterr = 0;
nres = 0;

9
src/harness/testsuite/harness_test.c
View file

@ -19,10 +19,19 @@ extern void test_listfmt_json(void);
extern void test_xcetest_process_entry(void);
extern void test_test_cli(void);
/* test_test_cli.c's leak-safety net (see its own comment): reaps and
* closes any clitest session left open by a test function that aborted
* via a TEST_ASSERT_* failure partway through, before it reached its own
* clitest_kill_and_reap()+clitest_close() cleanup. A no-op whenever no
* session is currently open (i.e. after every test outside test_test_cli,
* and after every test_test_cli sub-test that ran its own cleanup). */
extern void clitest_reap_leaked_session(void);
void setUp(void) { return; }
void tearDown(void)
{
clitest_reap_leaked_session();
printf("\n-----------------------------------------------------\n");
return;
}

50
src/harness/testsuite/test_test_cli.c
View file

@ -229,6 +229,49 @@ typedef struct {
bool reaped;
} clitest_session_t;
/* Leak-safety net: every test function below is expected to end with its
* own clitest_kill_and_reap()+clitest_close() calls, but a TEST_ASSERT_*
* failure ANYWHERE earlier in the same function longjmp()s straight out
* (Unity's TEST_PROTECT() mechanism) -- the rest of the function,
* including that inline cleanup, never runs. That leaves the spawned
* xce_test child alive (blocked forever on its own prompt, nobody left to
* send it input) plus its pty master/stderr-pipe fds open. A mirror is
* kept here in STATIC storage rather than tracking a pointer into the
* aborted function's own local `sess` -- that stack frame is unwound by
* the longjmp, so a pointer into it would dangle. harness_test.c's
* tearDown() -- which Unity guarantees to call after every RUN_TEST(),
* pass, fail, or abort -- calls clitest_reap_leaked_session() below to
* catch exactly this case. Only one clitest session is ever open at a
* time (every test function here declares a single `clitest_session_t`
* local), so one mirror slot is enough. */
static clitest_session_t g_last_session;
static void clitest_track_session(const clitest_session_t* sess)
{
g_last_session = *sess;
}
/* Called from harness_test.c's tearDown(). A no-op on the normal path,
* where clitest_kill_and_reap()+clitest_close() (below) have already
* synced g_last_session back to all-zero before this runs. */
void clitest_reap_leaked_session(void)
{
if (g_last_session.pid <= 0) {
return;
}
if (!g_last_session.reaped) {
kill(g_last_session.pid, SIGKILL);
waitpid(g_last_session.pid, NULL, 0);
}
if (g_last_session.master_fd >= 0) {
close(g_last_session.master_fd);
}
if (g_last_session.err_fd >= 0) {
close(g_last_session.err_fd);
}
memset(&g_last_session, 0, sizeof(g_last_session));
}
/* clitest_spawn_ex - capability 1 (parameterized): forks and execs `path`
* (argv[0] conventionally the program name, NULL-terminated) with:
* - a genuine pty pair on the child's stdin+stdout (openpty(), <pty.h>),
@ -340,6 +383,7 @@ static int clitest_spawn_ex(clitest_session_t* sess, const char* path,
sess->master_fd = master_fd;
sess->err_fd = errpipe[0];
sess->reaped = false;
clitest_track_session(sess);
return 0;
}
@ -546,6 +590,9 @@ static void clitest_kill_and_reap(clitest_session_t* sess)
kill(sess->pid, SIGKILL);
waitpid(sess->pid, &status, 0);
sess->reaped = true;
if (g_last_session.pid == sess->pid) {
g_last_session.reaped = true;
}
}
static void clitest_close(clitest_session_t* sess)
@ -558,6 +605,9 @@ static void clitest_close(clitest_session_t* sess)
close(sess->err_fd);
sess->err_fd = -1;
}
if (g_last_session.pid == sess->pid) {
memset(&g_last_session, 0, sizeof(g_last_session));
}
}
/* ------------------------------------------------------------------ */