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

feat(xce-test-cli-ux): wrap every blocking RPC with the interrupt adapter

Browse source 
Routes all 26 blocking xcecli_* call sites across ~24 commands through
xcetest_run_interruptible(), wires xcetest_set_active_connection() at
execute_line()'s dispatch point (active_ctx->ctx, read fresh per
command), and fixes main()'s REPL loop to distinguish a real EOF from
an EINTR-interrupted readline() so Ctrl-C at an empty prompt discards
the line instead of exiting the session.

Live-discovered and fixed bug-1794: fill_job_alloc() never initializes
xce_job_alloc_t's nres_shared/allocs_shared fields -- harmless as stack
garbage before this task, genuine heap garbage once moved into a
heap-allocated ctx block (a necessary consequence of the adapter's
heap-only precondition). Fixed by switching all ~26 call-context
allocations from malloc to calloc uniformly. Also fixed an argv-aliasing
hazard: cmd_hello/cmd_jobckpt's -m values aliased optarg, which
execute_line() frees immediately after return including on the
abandoned path -- now strdup'd into the heap block.

Also closed a safety-critical gap this task's own design introduced
(bug-1795): before this task the REPL was fully synchronous, so
leave_ctx() (called by up/quit) could never run while an RPC was
outstanding. An abandoned worker can now persist past command
completion, so leave_ctx() tearing down the connection could
use-after-free it. Fixed via xcetest_wait_connection_idle(), which
blocks on the same per-connection semaphore the adapter already
tracks, then releases it, proving no worker is active before teardown.
Proven with a deterministic white-box RED->GREEN test, since the race
itself is too timing-dependent for a black-box test to reliably force.

1361/1361 across the full controller suite (+2), 0 regressions.

Task 3.2 of .kiro/specs/xce-test-cli-ux/tasks.md.
Requirements: 1.1, 1.2, 1.3, 1.4, 1.5

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f
This commit is contained in:
Rongsong Shen 2026-09-09 20:58:18 +00:00
parent f152c59e75
commit 45120af2cc
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
7 changed files with 1887 additions and 70 deletions
Download patch file Download diff file Expand all files Collapse all files

107
src/controller/testsuite/test_interruptcall.c
View file

@ -464,6 +464,112 @@ static void test_interrupted_while_waiting_for_lock_never_spawns_worker(void)
wait_for_abandoned_free_count(1);
}
/* -- (iv) xcetest_wait_connection_idle() (task 3.2 remediation round 1,
* bug-1795) -- deterministic proof of the exact mechanism leave_ctx() now
* relies on to close the connection-teardown use-after-free.
*
* test_test_cli.c's black-box test_interrupt_then_immediate_teardown_no_uaf
* exercises the SAME fix end-to-end through a real xce_test subprocess, but
* the actual race window there (between an abandoned worker being woken by
* xcecli_wakeup() and leave_ctx()'s teardown proceeding) is sub-millisecond
* and timing-dependent -- it did not reliably reproduce as a failing test
* even with the fix disabled in this environment (matching the original
* round-1 review's own observation that a clean ASAN run is evidence the
* race didn't fire on THAT run's timing, not evidence the gap doesn't
* exist). This test instead proves xcetest_wait_connection_idle() itself,
* in isolation, with a WIDE, deterministic timing margin (a 700ms abandoned
* worker, not a sub-millisecond one) -- a direct, reliable RED (this
* function returning early would fail it) / GREEN (the shipped
* implementation passes it) demonstration of the invariant leave_ctx()
* depends on. */
#define WCI_SLOW_SLEEP_MS 700
#define WCI_MIN_WAIT_MS 400 /* comfortably below the ~700ms - \
* SIGNAL_DELAY_MS remaining sleep once the \
* call is abandoned, comfortably above \
* scheduling noise if \
* xcetest_wait_connection_idle() did NOT \
* actually wait (that failure mode returns \
* in ~0ms). */
static int wci_slow_fn(void* arg)
{
(void)arg;
sleep_ms(WCI_SLOW_SLEEP_MS);
return SLOW_FN_RC;
}
static void test_wait_connection_idle_blocks_until_abandoned_worker_finishes(
void)
{
static int conn_e; /* private fake connection key for this test */
pthread_t signaler;
struct timespec wait_start;
long waited_ms;
int rc2;
g_test_break_out = 0;
atomic_store(&xcetest_test_abandoned_free_count, 0);
xcetest_install_sigint(test_sigint_handler);
xcetest_set_active_connection(&conn_e);
TEST_ASSERT_EQUAL_INT(
0, pthread_create(&signaler, NULL, send_sigint_after_delay, NULL));
int rc = xcetest_run_interruptible(wci_slow_fn, NULL);
TEST_ASSERT_EQUAL_INT(-EINTR, rc);
TEST_ASSERT_EQUAL_INT(0, pthread_join(signaler, NULL));
/* At this instant the abandoned worker is still running wci_slow_fn()
* -- it has NOT yet freed its call block or released conn_e's lock
* (interruptcall.c's abandoned path). This is exactly the production
* race leave_ctx() closes: xcetest_wait_connection_idle() below must
* genuinely block until that worker finishes, not return immediately. */
TEST_ASSERT_EQUAL_INT_MESSAGE(
0, atomic_load(&xcetest_test_abandoned_free_count),
"the abandoned worker must still be running at this point for "
"this test to actually exercise the wait -- if this fires, the "
"worker finished implausibly fast and the timing margin above "
"needs to grow, not shrink");
clock_gettime(CLOCK_MONOTONIC, &wait_start);
xcetest_wait_connection_idle(&conn_e);
waited_ms = elapsed_ms_since(&wait_start);
TEST_ASSERT_TRUE_MESSAGE(
waited_ms >= WCI_MIN_WAIT_MS,
"xcetest_wait_connection_idle() returned too quickly -- it must "
"genuinely block until the abandoned worker finishes, not return "
"immediately (this is precisely the bug-1795 use-after-free gap: "
"a caller proceeding to tear down the connection before this "
"point would race the still-running worker)");
TEST_ASSERT_EQUAL_INT_MESSAGE(
1, atomic_load(&xcetest_test_abandoned_free_count),
"the abandoned worker's cleanup must have genuinely completed by "
"the time xcetest_wait_connection_idle() returns");
/* The semaphore must be left available (posted back), not held --
* proven by a normal subsequent call on the SAME connection key
* completing without waiting at all. Reset the fake interrupt flag
* first -- xcetest_interrupted() is a pure, non-clearing read (see
* xcetest_interrupt.h's CONTRACT), so the flag the earlier SIGINT set
* is still "on" here and would otherwise spuriously abandon this call
* too, same reasoning as every other multi-call scenario in this
* file. */
g_test_break_out = 0;
clock_gettime(CLOCK_MONOTONIC, &wait_start);
rc2 = xcetest_run_interruptible(fast_fn, NULL);
waited_ms = elapsed_ms_since(&wait_start);
TEST_ASSERT_EQUAL_INT(NORMAL_FN_RC, rc2);
TEST_ASSERT_TRUE_MESSAGE(
waited_ms < WCI_MIN_WAIT_MS,
"xcetest_wait_connection_idle() must release the semaphore back "
"immediately after proving idleness, not hold it -- a subsequent "
"call on the same connection should not itself have to wait");
}
void test_interruptcall(void)
{
UnitySetTestFile(__FILE__);
@ -472,4 +578,5 @@ void test_interruptcall(void)
RUN_TEST(test_same_connection_key_serializes_overlapping_calls);
RUN_TEST(test_different_connection_keys_run_independently);
RUN_TEST(test_interrupted_while_waiting_for_lock_never_spawns_worker);
RUN_TEST(test_wait_connection_idle_blocks_until_abandoned_worker_finishes);
}