From 45120af2cc06460b91bb8ca1c0167546d92592f6 Mon Sep 17 00:00:00 2001 From: Rongsong Shen Date: Wed, 9 Sep 2026 20:58:18 +0000 Subject: [PATCH] feat(xce-test-cli-ux): wrap every blocking RPC with the interrupt adapter 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 Claude-Session: https://claude.ai/code/session_019jaMf9QEwpugH2W9KAuc7f --- .kiro/specs/xce-test-cli-ux/tasks.md | 3 +- .wolf/buglog.json | 55 + src/controller/testsuite/interruptcall.c | 38 + src/controller/testsuite/interruptcall.h | 58 + src/controller/testsuite/test.c | 1230 ++++++++++++++++- src/controller/testsuite/test_interruptcall.c | 107 ++ src/controller/testsuite/test_test_cli.c | 466 +++++++ 7 files changed, 1887 insertions(+), 70 deletions(-) diff --git a/.kiro/specs/xce-test-cli-ux/tasks.md b/.kiro/specs/xce-test-cli-ux/tasks.md index 73e14ebd..e390311d 100644 --- a/.kiro/specs/xce-test-cli-ux/tasks.md +++ b/.kiro/specs/xce-test-cli-ux/tasks.md @@ -106,7 +106,7 @@ - _Boundary: Flag Parsing Layer_ - _Depends: 1.1, 1.2_ -- [ ] 3.2 Wrap every blocking request with the interruptible-call adapter +- [x] 3.2 Wrap every blocking request with the interruptible-call adapter - Route every command's blocking request through the interruptible-call adapter built in 1.4/1.5, so an interrupt during any of them abandons the wait immediately, reports the command as failed, and leaves the active connection and context intact - Ensure the session never exits as a side effect of a single interrupt keypress, and that an interrupt at an empty prompt simply discards the current input line - Observable completion: interrupting a command that is blocked waiting on a request returns control to the prompt immediately (well before the request's own timeout) and the same connection accepts a subsequent command normally @@ -178,6 +178,7 @@ ## Implementation Notes - `src/tools/xce-build.sh controller asan xce.build --incremental` writes its build dir at `src/xce.build/` (relative to the script's own location), not a repo-root `xce.build/` — meson logs are at `src/xce.build/meson-logs/testlog.txt`. (task 1.1) - New Unity test files need two wiring points, not just meson.build's source list: also register an `extern void test_(void);` + `TEST(test_);` in `src/controller/testsuite/controller_test.c`, mirroring the existing `test_rcfile` registration, or the suite silently never runs. (task 1.1) +- **Safety-critical finding**: wrapping RPC calls in an abandonable worker thread means `leave_ctx()` (called by `up`/`quit`) can now run while a previously-interrupted command's worker is still using the connection — before this task the REPL was fully synchronous so this UAF couldn't exist. Fixed via `xcetest_wait_connection_idle(conn_key)` (interruptcall.c), which blocks on the SAME per-connection semaphore the adapter already uses, then immediately releases it, proving no worker is active before teardown proceeds. Call it before any future connection-teardown path, not just `leave_ctx()`, if one is added later. A timing-dependent black-box test alone could NOT reliably prove this race (ran clean 20/20 even with the fix disabled) — the reliable proof came from a white-box test directly exercising the wait function against a real abandoned worker. When a fix's failure mode is a sub-millisecond race, don't trust a black-box test's clean run as RED-phase evidence; get a deterministic white-box reproduction instead. (task 3.2, bug-1795) - All 17 flag-accepting commands migrated to `flagspec_t`+`getopt_long` (review verified essentially every command line-by-line via git diff context, not just a sample — zero regressions to any existing short flag's letter/value-format/default). `cmd_walcompare`'s migration needs a leading `+` in its optstring (`getopt_long`'s "stop at first non-option") to preserve its pre-migration "flags only before positionals" behavior — without it, argv permuting would change which tokens are treated as flags vs. member paths. Only 7 of the 17 migrated commands got dedicated black-box tests (`qctrl`, `jobinfo`, `jobkill`, `hostctrl`, `setpend`, `resupdate`, `walcompare`); the other 10 (`hello`, `jobrun`, `jobsub`, `resval`, `jobckpt`, `runtasks`, `jobstart`, `jobfinish`, `jobfail`, `join_worker_parse_attrs`) have no valid-flag regression test at all beyond this review's manual line-by-line diff check — worth a follow-up smoke test per command if this area gets touched again. (task 3.1) - `cmd_join_worker`'s sibling fields `maxswap`/`maxtmp` have the exact same stale-MB-scale bug as bug-1557's `maxmem` (1024/2048 bytes instead of KB/MB-appropriate values) but were deliberately left unfixed here: `USAGE_JOIN_WORKER` currently documents these exact stale values accurately, so fixing only the code would create a new code/docs mismatch outside this task's mandate. Worth a coordinated follow-up bug fixing both fields' defaults and their help text together. A new `test_join_worker_sibling_defaults_unchanged` test locks in today's values so that follow-up isn't confused with this task's maxmem-specific assertions. (task 2.6) - When a fix touches a pure function stuck inside `test.c` (no networking/REPL/global-state dependency), extract it into its own module with a standalone meson `test()` target rather than settling for a throwaway/manual verification — this codebase already has two precedents for exactly this (`resval_typeresolve.c`, `misc.c`'s runusage helpers; now also `joblist_display.c` for `worker_status_str`/`job_state_str`/`format_job_time`). `test.c` itself is never linked into `controller_unit_test`, so this extract-to-standalone-module pattern is the only way to get permanent automated coverage for such functions — a scratch/deleted verification harness is not sufficient and will be sent back on review. `cmd_reslist`/`joblist_query`/`cmd_getconf`'s table-printing bodies remain genuinely untestable without a live cluster (fetch+print are inlined together, confirmed on review) and stay uncovered by permanent tests for now. (task 2.5) diff --git a/.wolf/buglog.json b/.wolf/buglog.json index 779fe569..209983cb 100644 --- a/.wolf/buglog.json +++ b/.wolf/buglog.json @@ -21536,6 +21536,61 @@ ], "occurrences": 1, "last_seen": "2026-09-09T16:24:19Z" + }, + { + "id": "bug-1794", + "timestamp": "2026-09-09T19:03:51Z", + "error_message": "xce_test cmd_jobrun crashes (ASAN allocation-size-too-big abort in alloc_packet, requested size ~0xffffffffc3c3c4b0) when run live against a real controller, after xce-test-cli-ux task 3.2 wrapped its xcecli_jobdispatch() call through the interruptible-call adapter (heap-allocated ctx block instead of a stack-local xce_job_alloc_t).", + "file": "src/controller/testsuite/test.c (cmd_jobrun, fill_job_alloc, jobrun_call_t/jobrun_thunk)", + "root_cause": "PRE-EXISTING, latent: fill_job_alloc() (test.c) only ever sets xce_job_alloc_t.nhosts/.allocs -- it never initializes the struct's other two fields, nres_shared and allocs_shared (xce/types.h). Before task 3.2, xce_job_alloc_t jalloc was a plain stack-local in cmd_jobrun, so those two never-set fields held whatever incidental value that stack frame happened to have -- harmless in practice, apparently usually small/zero. Task 3.2 moved this struct into a heap-allocated jobrun_call_t block (required by the interruptible-call adapter's contract: fn must only touch heap-owned memory, never the caller's stack, since an abandoned call's worker thread can outlive the caller's own stack frame) allocated via plain malloc(), which leaves ALL bytes as genuine heap garbage -- confirmed live (real single-node deploy, jobrun -j -m worker-1:1): the garbage nres_shared/allocs_shared corrupted xce_jobdispatch_req's wire-size computation in lib/request.c, producing a multi-exabyte allocation request that ASAN correctly aborted on.", + "fix": "FIXED (2026-09-09, xce-test-cli-ux task 3.2). Every per-call-site heap block (all ~26 _call_t typedefs test.c's interrupt-wrapping introduces) now allocates via calloc(1, sizeof(*call)) instead of malloc(sizeof(*call)), zero-initializing every field uniformly rather than auditing each wrapped request struct for a similarly-never-fully-set field one at a time -- the same defensive default an accidentally-zero stack frame was already silently relied on for. Verified by re-running the exact live repro (jobrun against a real single-node deploy) after the fix: completes normally with no ASAN error.", + "tags": [ + "controller", + "testsuite", + "xce_test", + "cmd_jobrun", + "fill_job_alloc", + "uninitialized-memory", + "heap-allocation", + "asan", + "interruptcall", + "xce-test-cli-ux", + "task-3.2" + ], + "related_bugs": [ + "bug-1791" + ], + "occurrences": 1, + "last_seen": "2026-09-09T19:03:51Z" + }, + { + "id": "bug-1795", + "timestamp": "2026-09-09T20:10:00Z", + "error_message": "leave_ctx() (test.c, called by cmd_up/cmd_quit) could free a connection's api_ctx_t (xcecli_stop()+xcecli_ctx_fini(), src/lib/client.c) while an abandoned worker thread from a prior Ctrl-C-interrupted RPC on that SAME connection was still running and still reading/writing it -- a genuine use-after-free, found by kiro-review during xce-test-cli-ux task 3.2's own round-1 review (not reproduced live/under ASAN on that run; identified structurally by tracing the code paths).", + "file": "src/controller/testsuite/test.c (leave_ctx), src/controller/testsuite/interruptcall.c/.h (xcetest_wait_connection_idle, new), src/lib/client.c (xcecli_wakeup, xcecli_stop, xcecli_ctx_fini, read-only for this fix)", + "root_cause": "Task 3.2's own interruptible-call adapter (tasks 1.4/1.5, interruptcall.c) runs every blocking xce_* RPC on a detached worker thread; when the operator interrupts (Ctrl-C) a blocked call, xcetest_run_interruptible() returns -EINTR to the REPL immediately, but the worker thread it spawned keeps running -- possibly for up to its own ~10s cbuf_timedget deadline -- reading/writing whatever connection state its ctx closes over. Before this task, the REPL was fully synchronous, so leave_ctx() (test.c, shared by cmd_up's '..' and cmd_quit) could never run while an RPC was outstanding on that connection; task 3.2's adapter broke that invariant by letting control return to the prompt WHILE a worker is still abandoned-but-running. leave_ctx() already called xcecli_wakeup(active_ctx->ctx, -EINTR) before tearing the connection down, but that is only a best-effort nudge (it wakes the underlying condvar wait sooner than the worker's own deadline) -- nothing forced leave_ctx() to actually WAIT for that woken worker to finish before proceeding to xcecli_stop() (tears down the live socket + failover lock) and xcecli_ctx_fini() (unconditionally free()s the whole api_ctx_t). If the operator's very next action after abandoning a hung command is 'up' or 'quit' -- an entirely ordinary thing to do -- the still-running worker can be mid-read/write on memory that just got freed out from under it. This directly contradicts the Interruptible Call Adapter's own documented invariant ('at most one worker thread -- running or abandoned-but-still-running -- is ever active per connection at a time', interruptcall.h/design.md) -- that invariant was enforced only BETWEEN xcetest_run_interruptible() calls (via the per-connection semaphore registry, tasks 1.4/1.5), never against connection TEARDOWN, since leave_ctx() never itself goes through xcetest_run_interruptible().", + "fix": "FIXED (2026-09-09, xce-test-cli-ux task 3.2 remediation round 1). Added xcetest_wait_connection_idle(void* conn_key) to interruptcall.c/.h, reusing the EXACT SAME per-connection semaphore registry lookup (xcetest_conn_lock_for()) that xcetest_run_interruptible() already builds and maintains -- not a new/parallel mechanism. It does a plain BLOCKING sem_wait() (deliberately not the interruptible sem_trywait()-poll variant used elsewhere -- this is a narrow, safety-critical wait where correctness outranks Ctrl-C responsiveness, and it resolves quickly in practice since xcecli_wakeup() already nudges the worker) then immediately sem_post()s the semaphore back rather than holding it -- the semantic is 'prove no worker is active on this connection right now', not 'hold the connection locked'. leave_ctx() now calls xcetest_wait_connection_idle(ctx->ctx) right after the existing xcecli_wakeup() call and right before xcecli_stop()/xcecli_ctx_fini() -- since both cmd_up and cmd_quit call this SAME shared leave_ctx(), both paths are covered by a single call site. New black-box regression test test_interrupt_then_immediate_teardown_no_uaf (test_test_cli.c, reuses the existing clitest_start_blackhole_listener() harness helper) interrupts a blocked hello RPC against a black-hole TCP peer and sends 'up' IMMEDIATELY afterward -- with no settling wait for the interrupted prompt to redisplay first, unlike the sibling interrupt test -- to race leave_ctx()'s teardown against the abandoned worker on purpose; asserts the session exits only via a normal (non-signal) status and that stderr never contains an 'AddressSanitizer' diagnostic anywhere across the whole session. Verified under the asan build profile.", + "tags": [ + "controller", + "testsuite", + "xce_test", + "leave_ctx", + "cmd_up", + "cmd_quit", + "interruptcall", + "use-after-free", + "asan", + "concurrency", + "semaphore", + "xce-test-cli-ux", + "task-3.2", + "kiro-review-finding" + ], + "related_bugs": [ + "bug-1794" + ], + "occurrences": 1, + "last_seen": "2026-09-09T20:10:00Z" } ] } diff --git a/src/controller/testsuite/interruptcall.c b/src/controller/testsuite/interruptcall.c index 6953053a..ed999ff4 100644 --- a/src/controller/testsuite/interruptcall.c +++ b/src/controller/testsuite/interruptcall.c @@ -314,3 +314,41 @@ int xcetest_run_interruptible(xcetest_blocking_fn fn, void* ctx) xcetest_sleep_poll_interval(); } } + +/* See interruptcall.h for the full contract. Reuses xcetest_conn_lock_for() + * -- the SAME lazy-lookup/create used by xcetest_run_interruptible() above + * -- rather than a parallel registry, so this genuinely observes the same + * per-connection semaphore any in-flight or abandoned worker on `conn_key` + * is holding. */ +void xcetest_wait_connection_idle(void* conn_key) +{ + sem_t* lock; + int rc; + + lock = xcetest_conn_lock_for(conn_key); + if (!lock) { + /* Allocation failure in the lazy-create path: this key has never + * been registered, so by definition no worker could be holding a + * lock for it via this registry. Nothing to wait for. */ + return; + } + + /* Plain BLOCKING wait, deliberately not the interruptible + * sem_trywait()-poll loop used above -- see interruptcall.h's + * "CONTRACT" section on this function. A signal delivered while + * blocked here can legitimately interrupt sem_wait() with EINTR even + * though nothing else ever posts spuriously; retry rather than treat + * that as "acquired" or bail out early, since either would defeat + * this function's entire purpose (proving the wait genuinely + * completed before the caller proceeds to tear the connection down). */ + do { + rc = sem_wait(lock); + } while ((rc != 0) && (errno == EINTR)); + assert(rc == 0); /* NDEBUG caveat: see xcetest_worker_main()'s comment. */ + + /* Immediately release: the semantic is "prove idle right now," not + * "hold the connection locked." */ + rc = sem_post(lock); + (void)rc; + assert(rc == 0); +} diff --git a/src/controller/testsuite/interruptcall.h b/src/controller/testsuite/interruptcall.h index eee063fc..5bbc5d23 100644 --- a/src/controller/testsuite/interruptcall.h +++ b/src/controller/testsuite/interruptcall.h @@ -249,6 +249,64 @@ void xcetest_set_active_connection(void* conn_key); */ int xcetest_run_interruptible(xcetest_blocking_fn fn, void* ctx); +/* + * xcetest_wait_connection_idle (xce-test-cli-ux task 3.2 remediation round + * 1, bug-1795) -- blocks the calling thread until no worker thread + * (running or abandoned-but-still-running) is active on `conn_key`, per + * this module's own "at most one worker thread per connection" invariant, + * then returns immediately. + * + * WHY THIS EXISTS: xcetest_run_interruptible()'s per-connection lock + * registry above enforces that invariant BETWEEN calls on the same + * connection -- a second call on a connection an abandoned worker still + * holds waits (interruptibly) for that worker to finish. It does NOT by + * itself protect connection TEARDOWN: test.c's leave_ctx() (called by + * `up`/`quit`) tears down the connection object out from under an + * abandoned-but-still-running worker with no wait/join of its own, + * because leave_ctx() never calls xcetest_run_interruptible() -- it is + * connection teardown, not a new RPC. xcecli_wakeup() (called by + * leave_ctx() just before this function, best-effort) nudges that worker + * to unwind faster than its own ~10s deadline, but nothing forced + * leave_ctx() to actually wait for it -- a genuine, exploitable + * use-after-free window (any abandoned worker still reading/writing the + * connection object while leave_ctx()'s xcecli_stop()/xcecli_ctx_fini() + * frees it). This function closes that gap by reusing the SAME + * per-connection semaphore registry xcetest_run_interruptible() already + * builds and maintains, rather than inventing a second mechanism. + * + * CONTRACT: looks up (or lazily creates, via the exact same internal + * lookup xcetest_run_interruptible() itself uses) the sem_t for + * `conn_key`, does a PLAIN BLOCKING sem_wait() on it (deliberately NOT + * the interruptible sem_trywait()-poll variant used inside + * xcetest_run_interruptible() -- this is a narrow, safety-critical wait + * where correctness matters more than Ctrl-C responsiveness; it should + * resolve quickly in the ordinary case since the caller's own + * xcecli_wakeup() call already nudges the worker to finish sooner), then + * immediately sem_post()s it back before returning. It does NOT hold the + * semaphore locked on return -- the semantic this function provides is + * "prove that, at this instant, no worker is active on this connection," + * not "hold this connection locked for some ongoing operation of the + * caller's own." A caller that needs the latter must acquire and hold the + * lock itself (which only xcetest_run_interruptible() currently does). + * + * A transient EINTR from sem_wait() itself (a signal delivered while + * blocked, distinct from `errno`-based break_out signal state) is + * retried internally rather than surfaced -- this function's whole + * purpose is to make the wait unconditional, not skippable. + * + * Safe to call with any `conn_key` previously passed to + * xcetest_set_active_connection() (or, for that matter, any key never + * seen before -- the lazy-create path simply creates a fresh, immediately + * uncontended semaphore for it and returns right away). Must be called + * from the same single REPL thread this module's other entry points are + * documented to run on (see this header's "CONCURRENCY OF THE REGISTRY + * ITSELF" note) -- it is not itself safe to call concurrently from + * multiple threads, though the semaphore it waits on is of course safe to + * be concurrently sem_post()'d by a worker thread while this function + * waits on it. + */ +void xcetest_wait_connection_idle(void* conn_key); + #ifdef XCE_TESTING /* * Test-only instrumentation, NOT part of the production API and never diff --git a/src/controller/testsuite/test.c b/src/controller/testsuite/test.c index 67d1a369..4e96ca3e 100644 --- a/src/controller/testsuite/test.c +++ b/src/controller/testsuite/test.c @@ -31,6 +31,31 @@ * flag-accepting command below migrates onto this * (task 3.1) */ #include "idgen.h" +#include "interruptcall.h" /* xcetest_run_interruptible, xcetest_set_active_connection + * (xce-test-cli-ux tasks 1.4/1.5); every blocking xcecli_* + * call site below is wrapped through this (task 3.2). + * + * Every per-call-site heap block below (typedef'd as + * _call_t, one per wrapped call site) is allocated + * via calloc(1, sizeof(*call)), never plain malloc() -- + * found live, not just by inspection: cmd_jobrun's + * xce_job_alloc_t has two fields (nres_shared, + * allocs_shared) that fill_job_alloc() never sets, a + * PRE-EXISTING gap that was harmless on the old + * stack-allocated xce_job_alloc_t (whatever incidental + * value the stack frame happened to hold there rarely + * mattered) but corrupts the wire encoding when the SAME + * uninitialized fields instead come from freshly + * malloc()'d heap memory (confirmed: an ASAN + * allocation-size-too-big abort in alloc_packet() from a + * garbage nres_shared/allocs_shared, immediately fixed by + * switching this block's allocation to calloc()). Rather + * than auditing every wrapped request struct for a + * similarly-never-fully-initialized field one at a time, + * every call site's heap block is zero-initialized + * uniformly -- the same defensive default an + * accidentally-zero stack frame was already silently + * relied on for. */ #include "preason.h" #include "walcompare.h" /* cross-member committed-prefix divergence (cmd_walcompare) */ #include "waldump.h" /* offline WAL inspector (cmd_waldump) */ @@ -860,6 +885,20 @@ int leave_ctx(command_ctx_t* ctx) xcecli_wakeup(active_ctx->ctx, (int)-EINTR); + /* xce-test-cli-ux task 3.2 remediation round 1 (bug-1795): the + * xcecli_wakeup() call just above is a best-effort nudge only -- it + * does not itself guarantee that a worker thread abandoned by a prior + * Ctrl-C on THIS connection (interruptcall.c's per-connection + * registry, tasks 1.4/1.5) has actually finished touching `ctx->ctx` + * before xcecli_stop()/xcecli_ctx_fini() below free it out from under + * that worker -- a genuine use-after-free if the operator's very next + * action after abandoning a hung command is `up`/`quit` (an entirely + * ordinary thing to do). xcetest_wait_connection_idle() blocks (a + * narrow, deliberately UNINTERRUPTIBLE wait -- see interruptcall.h) + * until that same registry proves no worker is active on this + * connection, then returns immediately, before teardown proceeds. */ + xcetest_wait_connection_idle(ctx->ctx); + xcecli_stop(ctx->ctx); xcecli_ctx_fini(ctx->ctx); @@ -1013,6 +1052,36 @@ int execute_line(char* line) return (-1); } + /* xce-test-cli-ux task 3.2, Requirement 1.1/1.5: this is the SINGLE call + * site for xcetest_set_active_connection() -- interruptcall.h's own + * "SEAM FOR A FUTURE CALLER (task 3.2)" comment names this exact + * dispatch chokepoint ("wherever main()/the dispatcher sets up each + * command's active connection context"). active_ctx->ctx (xcecli_ctx_t + * is `void*`, see client.h) is read fresh here, on every command + * dispatch, so the per-connection lock registry always keys off + * whichever connection is actually active for the command about to run + * -- correct across `worker`/`scheduler`/`client`/`..` context switches + * without needing a separate call in cmd_worker/cmd_scheduler/cmd_client/ + * cmd_up. In topctx (no live connection; enter_ctx() is never called for + * it), active_ctx->ctx is the zero-initialized NULL it starts as -- a + * harmless, consistent key, since no topctx command makes a blocking RPC + * call. */ + xcetest_set_active_connection(active_ctx->ctx); + + /* xce-test-cli-ux task 3.2, Requirement 1.1/1.3/1.4: reset break_out + * immediately before dispatching THIS command, so its own blocking-RPC + * call sites (wrapped via xcetest_run_interruptible() below) observe a + * freshly-clear interrupt flag rather than a stale one left set by an + * EARLIER command that already reported itself as interrupted/failed + * (xcetest_interrupted() is a pure, non-consuming read -- see + * xcetest_interrupt.h -- so nothing else ever clears it on this + * command's behalf). Without this, a command run immediately after an + * interrupted one would itself be instantly, incorrectly reported as + * interrupted before ever running -- breaking exactly the "same + * connection accepts a subsequent command normally" guarantee this + * task exists to provide. */ + break_out = 0; + rc = (command->func)(argc, argv); free_argv(argv, argc); @@ -1711,6 +1780,45 @@ static const flagspec_t hello_flags[] = { }; #define HELLO_NFLAGS (sizeof(hello_flags) / sizeof(hello_flags[0])) +/* xce-test-cli-ux task 3.2: per-call-site heap block wrapping the blocking + * xcecli_hello() call for xcetest_run_interruptible() (interruptcall.h, + * tasks 1.4/1.5). `msg` is copied (strdup) rather than referenced, because + * `-m` can set it to `optarg`, which points into `argv[]` -- storage + * execute_line() frees (free_argv()) immediately after this command + * returns, including on the -EINTR/abandoned path, when a detached worker + * thread may still be reading it. See this file's other xcetest_run_ + * interruptible() call sites for the same pattern applied to every other + * blocking RPC call below. + * + * Ownership contract (interruptcall.h): on a NON-EINTR return, fn() has + * already finished, so the call site below frees this block itself right + * after reading `rc`. On -EINTR, ownership has passed to the still-running + * worker thread -- the call site must not touch it again, so `hello_thunk` + * frees both `msg` and the block itself, unconditionally, once fn() has + * actually finished (this is safe on EITHER path, since exactly one of + * {call site, thunk} ever touches this block after the adapter call + * returns -- see xcetest_run_interruptible()'s own doc comment). A call + * abandoned via Ctrl-C therefore leaks nothing beyond the adapter's own + * documented worst case (an abandoned worker outliving the command by up + * to ~10s) -- deliberately simpler than reproducing interruptcall.c's own + * CAS-based ownership protocol per call site, which would be substantial + * duplicated complexity for a test client's ~19-odd RPC call sites. */ +typedef struct { + xcecli_ctx_t conn; + char* msg; +} hello_call_t; + +static int hello_thunk(void* ctxp) +{ + hello_call_t* call = (hello_call_t*)ctxp; + int rc = xcecli_hello(call->conn, call->msg); + + free(call->msg); + free(call); + + return rc; +} + int cmd_hello(int argc, char* argv[]) { int rc = 0; @@ -1747,7 +1855,27 @@ int cmd_hello(int argc, char* argv[]) break_out = 0; while ((!break_out) && (count > 0)) { - rc = xcecli_hello(active_ctx->ctx, msg); + hello_call_t* call = (hello_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + rc = -ENOMEM; + break; + } + + call->conn = active_ctx->ctx; + call->msg = strdup(msg); + + rc = xcetest_run_interruptible(hello_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` (and its owned msg copy) has passed to + * the still-running worker thread -- must not free or touch it + * again from here (interruptcall.h). Report the command as + * failed via the same errno/perror mechanism this function's + * other failure path would use, per Requirement 1.3. */ + errno = EINTR; + perror("hello"); + return rc; + } count--; } @@ -1755,10 +1883,38 @@ int cmd_hello(int argc, char* argv[]) return rc; } +typedef struct { + xcecli_ctx_t conn; +} shutdown_call_t; + +static int shutdown_thunk(void* ctxp) +{ + shutdown_call_t* call = (shutdown_call_t*)ctxp; + int rc = xcecli_shutdown(call->conn); + + free(call); + + return rc; +} + int cmd_shutdown(int argc, char* argv[]) { - int rc; - rc = xcecli_shutdown(active_ctx->ctx); + int rc; + shutdown_call_t* call = (shutdown_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(shutdown_thunk, call); + if (rc == -EINTR) { + errno = EINTR; + perror("shutdown"); + return rc; + } + /* leave client context */ if (rc >= 0) { execute_line(".."); @@ -1767,12 +1923,52 @@ int cmd_shutdown(int argc, char* argv[]) return rc; } +/* xce-test-cli-ux task 3.2: Case-B (output-bearing) call site. The block + * survives past the wrapped call (unlike the Case-A blocks above) because + * the caller needs to read `res`/`rc` back out of it -- but ONLY on a + * non-EINTR return, which interruptcall.h guarantees means fn() (the + * thunk) has already fully finished writing them. On -EINTR, the call + * site below never touches or frees this block again; the underlying + * xcecli_reslist() reply (and this block itself) is deliberately leaked + * in that case, matching every other call site's documented trade-off. */ +typedef struct { + xcecli_ctx_t conn; + xce_resource_t** res; + int rc; +} reslist_call_t; + +static int reslist_thunk(void* ctxp) +{ + reslist_call_t* call = (reslist_call_t*)ctxp; + + call->rc = xcecli_reslist(call->conn, &call->res); + + return call->rc; +} + int cmd_reslist(int argc, char* argv[]) { int rc, i; xce_resource_t** res; + reslist_call_t* call = (reslist_call_t*)calloc(1, sizeof(*call)); - rc = xcecli_reslist(active_ctx->ctx, &res); + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(reslist_thunk, call); + if (rc == -EINTR) { + /* Same errno/perror mechanism, and the same "xce_reslist" label, + * as this function's own non-interrupt failure path just below. */ + SET_ERROR(EINTR); + perror("xce_reslist"); + return rc; + } + + res = call->res; + free(call); if (rc < 0) { SET_ERROR(-rc); @@ -1815,12 +2011,43 @@ int cmd_reslist(int argc, char* argv[]) /* forward decl: defined near cmd_resval, below, which also uses it */ static void format_bytes_human(double bytes, char* buf, size_t bufsize); +typedef struct { + xcecli_ctx_t conn; + xce_worker_t** workers; + int rc; +} workerlist_call_t; + +static int workerlist_thunk(void* ctxp) +{ + workerlist_call_t* call = (workerlist_call_t*)ctxp; + + call->rc = xcecli_workerlist(call->conn, &call->workers); + + return call->rc; +} + int cmd_workerlist(int argc, char* argv[]) { - int rc, i; - xce_worker_t** workers; + int rc, i; + xce_worker_t** workers; + workerlist_call_t* call = (workerlist_call_t*)calloc(1, sizeof(*call)); - rc = xcecli_workerlist(active_ctx->ctx, &workers); + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(workerlist_thunk, call); + if (rc == -EINTR) { + /* Same mechanism/label as this function's own failure path below. */ + SET_ERROR(EINTR); + perror("xce_workerlist"); + return rc; + } + + workers = call->workers; + free(call); if (rc < 0) { SET_ERROR(-rc); @@ -1881,17 +2108,50 @@ int cmd_workerlist(int argc, char* argv[]) * * On error this returns the negative rc WITHOUT setting any auto-variable, so a * failed joblist never advertises a fake `$jobcount` (Req 3.6). */ +typedef struct { + xcecli_ctx_t conn; + xce_job_t** jobs; + int rc; +} joblist_call_t; + +static int joblist_thunk(void* ctxp) +{ + joblist_call_t* call = (joblist_call_t*)ctxp; + + call->rc = xcecli_joblist(call->conn, &call->jobs); + + return call->rc; +} + static int joblist_query(const char* find_jobid, int* contains, int print_table, int quiet) { - int rc, i; - xce_job_t** jobs; + int rc, i; + xce_job_t** jobs; + joblist_call_t* call = (joblist_call_t*)calloc(1, sizeof(*call)); if (contains != NULL) { *contains = 0; } - rc = xcecli_joblist(active_ctx->ctx, &jobs); + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(joblist_thunk, call); + if (rc == -EINTR) { + /* Same mechanism/label as this function's own failure path below. */ + if (!quiet) { + SET_ERROR(EINTR); + perror("xce_joblist"); + } + return rc; + } + + jobs = call->jobs; + free(call); if (rc < 0) { SET_ERROR(-rc); @@ -2113,6 +2373,22 @@ static long monotonic_ms(void) return ((long)ts.tv_sec * MS_PER_SEC) + (ts.tv_nsec / NS_PER_MS); } +typedef struct { + xcecli_ctx_t conn; + xce_ctrlstatus_t* status; + int rc; +} ctrlstatus_primary_call_t; + +static int ctrlstatus_primary_thunk(void* ctxp) +{ + ctrlstatus_primary_call_t* call = (ctrlstatus_primary_call_t*)ctxp; + + call->status = NULL; + call->rc = xcecli_ctrlstatus_primary(call->conn, &call->status); + + return call->rc; +} + int cmd_wait_primary(int argc, char* argv[]) { long timeout_ms = WAIT_PRIMARY_DEFAULT_MS; @@ -2152,9 +2428,34 @@ int cmd_wait_primary(int argc, char* argv[]) * internal retry budget); we re-read the clock after every probe returns so * the overall loop honors the deadline and returns failure in bounded time. */ for (;;) { - xce_ctrlstatus_t* status = NULL; - int rc = xcecli_ctrlstatus_primary(active_ctx->ctx, - &status); + xce_ctrlstatus_t* status; + int rc; + ctrlstatus_primary_call_t* call + = (ctrlstatus_primary_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + mylog_setlevel(g_active_loglevel); + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(ctrlstatus_primary_thunk, call); + if (rc == -EINTR) { + /* Abandon the whole wait-primary poll immediately (Requirement + * 1.1) rather than retrying -- ownership of `call` has passed + * to the still-running worker thread (interruptcall.h). */ + mylog_setlevel(g_active_loglevel); + /* Same printf-based reporting style as this command's own + * success/timeout messages just below, rather than a one-off + * perror() this loop otherwise never uses. */ + printf("wait-primary: interrupted after %ld ms\n", + monotonic_ms() - start_ms); + return rc; + } + + status = call->status; + free(call); /* Success ONLY on an authoritative reply from the serving primary * (responderRole == PRIMARY, guaranteed by xcecli_ctrlstatus_primary). @@ -2232,11 +2533,28 @@ static const char* ctrlstatus_role_str(uint8_t role) * On failure (no reachable controller etc.) it prints a typed error and returns * the negative rc so the session records the command as failed and exits * non-zero (Req 8.3). On success it prints the fields and returns 0 (Req 8.2). */ +typedef struct { + xcecli_ctx_t conn; + xce_ctrlstatus_t* status; + int rc; +} ctrlstatus_call_t; + +static int ctrlstatus_thunk(void* ctxp) +{ + ctrlstatus_call_t* call = (ctrlstatus_call_t*)ctxp; + + call->status = NULL; + call->rc = xcecli_ctrlstatus(call->conn, &call->status); + + return call->rc; +} + int cmd_ctrlstatus(int argc, char* argv[]) { - xce_ctrlstatus_t* status = NULL; - int rc; - int i; + xce_ctrlstatus_t* status = NULL; + int rc; + int i; + ctrlstatus_call_t* call; (void)argv; @@ -2245,7 +2563,26 @@ int cmd_ctrlstatus(int argc, char* argv[]) return -EINVAL; } - rc = xcecli_ctrlstatus(active_ctx->ctx, &status); + call = (ctrlstatus_call_t*)calloc(1, sizeof(*call)); + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(ctrlstatus_thunk, call); + if (rc != -EINTR) { + status = call->status; + free(call); + } + /* else: ownership of `call` has passed to the still-running worker + * thread (interruptcall.h) -- leave it untouched, `status` stays NULL, + * and rc (-EINTR) falls straight into the SAME generic error-reporting + * switch below that every other ctrlstatus failure already uses + * (Requirement 1.3: "consistent with how any other failing command is + * reported", not a special one-off). xce_strerror_r(EINTR, ...) below + * renders the standard "Interrupted system call" text via its errno + * fallback. */ if (rc < 0) { char ebuf[128]; @@ -2397,6 +2734,23 @@ static const flagspec_t jobinfo_flags[] = { }; #define JOBINFO_NFLAGS (sizeof(jobinfo_flags) / sizeof(jobinfo_flags[0])) +typedef struct { + xcecli_ctx_t conn; + uint64_t jobid; + xce_job_t* job; + int rc; +} jobinfo_call_t; + +static int jobinfo_thunk(void* ctxp) +{ + jobinfo_call_t* call = (jobinfo_call_t*)ctxp; + + call->job = NULL; + call->rc = xcecli_jobinfo(call->conn, call->jobid, &call->job); + + return call->rc; +} + int cmd_jobinfo(int argc, char* argv[]) { int opt; @@ -2425,7 +2779,32 @@ int cmd_jobinfo(int argc, char* argv[]) parse_jobid(&jobid, optarg); if (jobid > 0) { - rc = xcecli_jobinfo(active_ctx->ctx, jobid, &job); + jobinfo_call_t* call + = (jobinfo_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->jobid = jobid; + + rc = xcetest_run_interruptible(jobinfo_thunk, call); + if (rc == -EINTR) { + /* Requirement 1.3: abandon and fail the WHOLE command + * immediately, rather than continuing on to any + * further -j flags on this same invocation. Ownership + * of `call` has passed to the still-running worker + * thread (interruptcall.h) -- must not touch it + * again. */ + errno = EINTR; + perror("xcecli_jobinfo"); + return rc; + } + + job = call->job; + free(call); + if (rc < 0) { perror("xcecli_jobinfo"); continue; @@ -2493,15 +2872,29 @@ static const flagspec_t jobrun_flags[] = { }; #define JOBRUN_NFLAGS (sizeof(jobrun_flags) / sizeof(jobrun_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_job_dispatch_t jdispatch; + xce_job_alloc_t jalloc; + xce_job_alloc_t* allocs_arr[1]; + int rc; +} jobrun_call_t; + +static int jobrun_thunk(void* ctxp) +{ + jobrun_call_t* call = (jobrun_call_t*)ctxp; + + call->rc = xcecli_jobdispatch(call->conn, &call->jdispatch); + + return call->rc; +} + int cmd_jobrun(int argc, char* argv[]) { - int rc; - int opt; - uint64_t jobid = 0; - host_task_t hosts[MAX_TASK_HOSTS]; - xce_job_alloc_t jalloc; - xce_job_dispatch_t jdispatch - = { .njobs = 1, .allocs = (xce_job_alloc_t*[]) { &jalloc } }; + int rc; + int opt; + uint64_t jobid = 0; + host_task_t hosts[MAX_TASK_HOSTS]; int nhosts = 0; char* host; int count; @@ -2568,11 +2961,33 @@ int cmd_jobrun(int argc, char* argv[]) return -1; } - fill_job_alloc(&jalloc, nhosts, &hosts[0]); + { + jobrun_call_t* call = (jobrun_call_t*)calloc(1, sizeof(*call)); - jalloc.jobid = jobid; + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + fill_job_alloc(&call->jalloc, nhosts, &hosts[0]); + call->jalloc.jobid = jobid; + call->allocs_arr[0] = &call->jalloc; + call->jdispatch.njobs = 1; + call->jdispatch.allocs = call->allocs_arr; + + rc = xcetest_run_interruptible(jobrun_thunk, call); + if (rc == -EINTR) { + /* Same mechanism/label as this function's own failure path + * below. */ + SET_ERROR(EINTR); + perror("xce_jobdispatch"); + return rc; + } + + free(call); + } - rc = xcecli_jobdispatch(active_ctx->ctx, &jdispatch); if (rc < 0) { SET_ERROR(-rc); perror("xce_jobdispatch"); @@ -2622,6 +3037,23 @@ static const flagspec_t jobsub_flags[] = { }; #define JOBSUB_NFLAGS (sizeof(jobsub_flags) / sizeof(jobsub_flags[0])) +typedef struct { + xcecli_ctx_t conn; + job_submit_data_t submit; + xce_job_submit_t jobsubmit; + uint64_t jobid; + int rc; +} jobsub_call_t; + +static int jobsub_thunk(void* ctxp) +{ + jobsub_call_t* call = (jobsub_call_t*)ctxp; + + call->rc = xcecli_jobsubmit(call->conn, &call->jobsubmit, &call->jobid); + + return call->rc; +} + int cmd_jobsub(int argc, char* argv[]) { int rc = 0; @@ -2770,7 +3202,43 @@ int cmd_jobsub(int argc, char* argv[]) build_job_exdata(&jobsubmit); jobsubmit.array_size = arsize; - rc = xcecli_jobsubmit(active_ctx->ctx, &jobsubmit, &jobid); + + { + jobsub_call_t* call = (jobsub_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + rc = -ENOMEM; + goto cleanup; + } + + call->conn = active_ctx->ctx; + call->submit = submit; + call->jobsubmit = jobsubmit; + call->jobsubmit.submit = &call->submit; + call->jobid = 0; + + rc = xcetest_run_interruptible(jobsub_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` -- including the copy of `submit` (and + * the SAME heap-owned user/queue/resreq/start_info->* strings + * `submit` itself still references, by pointer value) embedded + * in it -- has passed to the still-running worker thread + * (interruptcall.h). Returning directly here, rather than via + * `goto cleanup`, is deliberate: the `cleanup:` label below + * frees those exact strings through the ORIGINAL `submit` + * local, which would race the worker's still-in-flight read + * of the SAME allocations through `call->submit`. Leaking + * `call` and every string it points to on this path is the + * same documented trade-off every other call site in this + * file makes on an abandoned call. */ + errno = EINTR; + perror("jobsub"); + return rc; + } + + jobid = call->jobid; + free(call); + } cleanup: FREEUP(submit.user); @@ -2922,6 +3390,24 @@ static const flagspec_t resval_flags[] = { }; #define RESVAL_NFLAGS (sizeof(resval_flags) / sizeof(resval_flags[0])) +typedef struct { + xcecli_ctx_t conn; + char* host; + char* res; + xce_resval_t** vals; + int rc; +} resval_call_t; + +static int resval_thunk(void* ctxp) +{ + resval_call_t* call = (resval_call_t*)ctxp; + + call->vals = NULL; + call->rc = xcecli_resval(call->conn, call->host, call->res, &call->vals); + + return call->rc; +} + int cmd_resval(int argc, char* argv[]) { char* host = NULL; @@ -2962,7 +3448,38 @@ int cmd_resval(int argc, char* argv[]) } } - rc = xcecli_resval(active_ctx->ctx, host, res, &vals); + { + resval_call_t* call = (resval_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + FREEUP(host); + FREEUP(res); + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->host = host; + call->res = res; + + rc = xcetest_run_interruptible(resval_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` -- including `host`/`res`, still + * embedded in it by pointer -- has passed to the still- + * running worker thread (interruptcall.h). Do NOT also + * FREEUP(host)/FREEUP(res) here: the SAME heap allocations + * are still referenced by call->host/call->res while that + * worker may still be reading them. Unlike this function's + * usual "always return 0" convention, an interrupted wait is + * reported as a genuine command failure (Requirement 1.3). */ + errno = EINTR; + perror("resval"); + return rc; + } + + vals = call->vals; + free(call); + } + if (rc > 0) { int i; @@ -3015,6 +3532,22 @@ static const flagspec_t jobkill_flags[] = { }; #define JOBKILL_NFLAGS (sizeof(jobkill_flags) / sizeof(jobkill_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_jobctrl_t jobctrl; + xce_signal_t siginfo; +} jobkill_call_t; + +static int jobkill_thunk(void* ctxp) +{ + jobkill_call_t* call = (jobkill_call_t*)ctxp; + int rc = xcecli_jobctrl(call->conn, &call->jobctrl); + + free(call); + + return rc; +} + int cmd_jobkill(int argc, char* argv[]) { int signo; @@ -3066,7 +3599,26 @@ int cmd_jobkill(int argc, char* argv[]) siginfo.signo = signo; siginfo.force = 1; - rc = xcecli_jobctrl(active_ctx->ctx, &jobctrl); + { + jobkill_call_t* call = (jobkill_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->jobctrl = jobctrl; + call->siginfo = siginfo; + call->jobctrl.u.siginfo = &call->siginfo; + + rc = xcetest_run_interruptible(jobkill_thunk, call); + if (rc == -EINTR) { + errno = EINTR; + perror("jobkill"); + return rc; + } + } + if (rc < 0) { perror("xcecli_jobctrl"); return rc; @@ -3088,6 +3640,28 @@ static const flagspec_t jobckpt_flags[] = { }; #define JOBCKPT_NFLAGS (sizeof(jobckpt_flags) / sizeof(jobckpt_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_jobctrl_t jobctrl; + xce_ckpt_options_t ckptoption; + char* method; /* owned copy: -m's ckptoption.method may + * alias optarg (argv[]), which execute_line() + * frees right after this command returns -- + * see hello_call_t's comment above for the + * same hazard/fix applied here. */ +} jobckpt_call_t; + +static int jobckpt_thunk(void* ctxp) +{ + jobckpt_call_t* call = (jobckpt_call_t*)ctxp; + int rc = xcecli_jobctrl(call->conn, &call->jobctrl); + + free(call->method); + free(call); + + return rc; +} + int cmd_jobckpt(int argc, char* argv[]) { uint64_t jobid = 0; @@ -3142,7 +3716,28 @@ int cmd_jobckpt(int argc, char* argv[]) jobctrl.jobid = jobid; jobctrl.u.ckptoption = &ckptoption; - rc = xcecli_jobctrl(active_ctx->ctx, &jobctrl); + { + jobckpt_call_t* call = (jobckpt_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->jobctrl = jobctrl; + call->ckptoption = ckptoption; + call->method = strdup(ckptoption.method); + call->ckptoption.method = call->method; + call->jobctrl.u.ckptoption = &call->ckptoption; + + rc = xcetest_run_interruptible(jobckpt_thunk, call); + if (rc == -EINTR) { + errno = EINTR; + perror("jobckpt"); + return rc; + } + } + if (rc < 0) { perror("xcecli_jobctrl"); return rc; @@ -3163,6 +3758,21 @@ static const flagspec_t qctrl_flags[] = { }; #define QCTRL_NFLAGS (sizeof(qctrl_flags) / sizeof(qctrl_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_queuectrl_t queuectrl; +} qctrl_call_t; + +static int qctrl_thunk(void* ctxp) +{ + qctrl_call_t* call = (qctrl_call_t*)ctxp; + int rc = xcecli_queuectrl(call->conn, &call->queuectrl); + + free(call); + + return rc; +} + int cmd_qctrl(int argc, char* argv[]) { int opt; @@ -3234,7 +3844,29 @@ int cmd_qctrl(int argc, char* argv[]) queuectrl.action = action; queuectrl.queue = queue; - rc = xcecli_queuectrl(active_ctx->ctx, &queuectrl); + { + qctrl_call_t* call = (qctrl_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->queuectrl = queuectrl; + + rc = xcetest_run_interruptible(qctrl_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` -- including its embedded copy of + * `queue`'s pointer value -- has passed to the still-running + * worker thread. Do NOT also free(queue) here: the SAME + * allocation is still referenced by call->queuectrl.queue + * while that worker may still be reading it. */ + errno = EINTR; + perror("qctrl"); + return rc; + } + } + if (rc < 0) { perror("xcecli_queuectrl"); return rc; @@ -3261,6 +3893,21 @@ static const flagspec_t hostctrl_flags[] = { }; #define HOSTCTRL_NFLAGS (sizeof(hostctrl_flags) / sizeof(hostctrl_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_hostctrl_t hostctrl; +} hostctrl_call_t; + +static int hostctrl_thunk(void* ctxp) +{ + hostctrl_call_t* call = (hostctrl_call_t*)ctxp; + int rc = xcecli_hostctrl(call->conn, &call->hostctrl); + + free(call); + + return rc; +} + int cmd_hostctrl(int argc, char* argv[]) { int opt; @@ -3316,7 +3963,28 @@ int cmd_hostctrl(int argc, char* argv[]) hostctrl.action = action; hostctrl.hostname = hostname; - rc = xcecli_hostctrl(active_ctx->ctx, &hostctrl); + { + hostctrl_call_t* call = (hostctrl_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + free(hostname); + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->hostctrl = hostctrl; + + rc = xcetest_run_interruptible(hostctrl_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` -- including its embedded copy of + * `hostname`'s pointer value -- has passed to the still- + * running worker thread. Do NOT also free(hostname) here. */ + errno = EINTR; + perror("hostctrl"); + return rc; + } + } + if (rc < 0) { perror("xcecli_hostctrl"); free(hostname); @@ -3338,6 +4006,29 @@ int cmd_hostctrl(int argc, char* argv[]) * verification), and drives it through xcecli_upgradeconfig -> * xce_upgradeconfig_req_raw (OP_upgradeConfig). Mirrors cmd_snapdiff's * bounds-checked file-reading style. */ +typedef struct { + xcecli_ctx_t conn; + uint8_t sha256[SHA256_DIGEST_LENGTH]; + uint8_t* bytes; /* ownership transferred here from cmd_upgradeconfig's + * local `bytes` -- the thunk frees it unconditionally, + * since the caller only needs `rc` back, not `bytes` + * itself, after the call (Case-A pattern). */ + size_t len; +} upgradeconfig_call_t; + +static int upgradeconfig_thunk(void* ctxp) +{ + upgradeconfig_call_t* call = (upgradeconfig_call_t*)ctxp; + int rc = xcecli_upgradeconfig( + call->conn, call->sha256, sizeof(call->sha256), call->bytes, + call->len); + + free(call->bytes); + free(call); + + return rc; +} + int cmd_upgradeconfig(int argc, char* argv[]) { if (argc != 2) { @@ -3387,10 +4078,26 @@ int cmd_upgradeconfig(int argc, char* argv[]) uint8_t sha256[SHA256_DIGEST_LENGTH]; SHA256(bytes, (size_t)fsz, sha256); - int rc = xcecli_upgradeconfig(active_ctx->ctx, sha256, - sizeof(sha256), bytes, (size_t)fsz); - free(bytes); + upgradeconfig_call_t* call + = (upgradeconfig_call_t*)calloc(1, sizeof(*call)); + int rc; + if (call == NULL) { + free(bytes); + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + memcpy(call->sha256, sha256, sizeof(sha256)); + call->bytes = bytes; /* ownership transferred; do not free() locally */ + call->len = (size_t)fsz; + + rc = xcetest_run_interruptible(upgradeconfig_thunk, call); + /* -EINTR deliberately falls straight into the SAME generic error + * report below that every other upgradeconfig failure already uses + * (Requirement 1.3: "consistent with how any other failing command is + * reported") -- strerror(-rc) with rc == -EINTR renders the standard + * "Interrupted system call" text, no special-case branch needed. */ if (rc < 0) { fprintf(stderr, "upgradeconfig: failed (rc=%d): %s\n", rc, strerror(-rc)); @@ -3415,6 +4122,23 @@ static const flagspec_t runtasks_flags[] = { }; #define RUNTASKS_NFLAGS (sizeof(runtasks_flags) / sizeof(runtasks_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_taskrun_t taskrun; + xce_taskhost_t hosts[MAX_TASK_HOSTS]; + xce_taskhost_t* taskhosts_arr[MAX_TASK_HOSTS]; +} runtasks_call_t; + +static int runtasks_thunk(void* ctxp) +{ + runtasks_call_t* call = (runtasks_call_t*)ctxp; + int rc = xcecli_runtask(call->conn, &call->taskrun); + + free(call); + + return rc; +} + int cmd_runtasks(int argc, char* argv[]) { xce_taskhost_t hosts[MAX_TASK_HOSTS]; @@ -3531,20 +4255,32 @@ int cmd_runtasks(int argc, char* argv[]) } } - taskrun.taskhosts - = (xce_taskhost_t**)calloc(nhosts, sizeof(xce_taskhost_t*)); - taskrun.clientidx = -1; + { + runtasks_call_t* call = (runtasks_call_t*)calloc(1, sizeof(*call)); - if (taskrun.taskhosts != NULL) { - for (i = 0; i < nhosts; i++) { - taskrun.taskhosts[i] = &hosts[i]; + if (call == NULL) { + rc = -ENOMEM; + } else { + call->conn = active_ctx->ctx; + memcpy(call->hosts, hosts, sizeof(xce_taskhost_t) * nhosts); + + for (i = 0; i < nhosts; i++) { + call->taskhosts_arr[i] = &call->hosts[i]; + } + + call->taskrun.taskhosts = call->taskhosts_arr; + call->taskrun.clientidx = -1; + call->taskrun.nhosts = nhosts; + call->taskrun.jobid = jobid; + + /* On -EINTR, ownership of `call` (and its embedded copies of + * hosts[]'s dir/user/exdata/cmdline/launch_type pointer + * values) has passed to the still-running worker thread -- + * runtasks_thunk() always frees `call` itself once fn() + * actually finishes, on either path, so there is nothing + * further to free here. */ + rc = xcetest_run_interruptible(runtasks_thunk, call); } - - taskrun.nhosts = nhosts; - taskrun.jobid = jobid; - rc = xcecli_runtask(active_ctx->ctx, &taskrun); - } else { - rc = -ENOMEM; } if (rc < 0) { @@ -3598,6 +4334,24 @@ static const flagspec_t setpend_flags[] = { }; #define SETPEND_NFLAGS (sizeof(setpend_flags) / sizeof(setpend_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_job_preason_t jreasons; + xce_pendreason_t preason; + xce_pendreason_t* preasons_arr[1]; + xce_preason_set_t reasons; +} setpend_call_t; + +static int setpend_thunk(void* ctxp) +{ + setpend_call_t* call = (setpend_call_t*)ctxp; + int rc = xcecli_set_pend_reason(call->conn, &call->reasons); + + free(call); + + return rc; +} + int cmd_setpend(int argc, char* argv[]) { int opt; @@ -3652,7 +4406,37 @@ int cmd_setpend(int argc, char* argv[]) return -1; } - rc = xcecli_set_pend_reason(active_ctx->ctx, &reasons); + { + setpend_call_t* call = (setpend_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + erase_job_preason(&jreasons); + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->jreasons = jreasons; + call->preason = preason; + call->preason.reasons = &call->jreasons; + call->preasons_arr[0] = &call->preason; + call->reasons = reasons; + call->reasons.reasons = call->preasons_arr; + + rc = xcetest_run_interruptible(setpend_thunk, call); + if (rc == -EINTR) { + /* Ownership of `call` -- including the copy of `jreasons` (and + * its heap-owned host_based_reasons[]/res_based_reasons[] + * entries, still referenced by the SAME pointer values) -- has + * passed to the still-running worker thread. Do NOT also + * erase_job_preason(&jreasons) here: that would free those + * same allocations while the worker may still be reading + * them. */ + errno = EINTR; + perror("setpend"); + return rc; + } + } + if (rc < 0) { perror("xcecli_set_pend_reason"); erase_job_preason(&jreasons); @@ -3664,6 +4448,22 @@ int cmd_setpend(int argc, char* argv[]) return 0; } +typedef struct { + xcecli_ctx_t conn; + int role; + hostattrs_t attrs; +} join_worker_call_t; + +static int join_worker_thunk(void* ctxp) +{ + join_worker_call_t* call = (join_worker_call_t*)ctxp; + int rc = xcecli_join(call->conn, call->role, &call->attrs); + + free(call); + + return rc; +} + int cmd_join_worker(int argc, char* argv[]) { int rc; @@ -3683,7 +4483,25 @@ int cmd_join_worker(int argc, char* argv[]) return rc; } - rc = xcecli_join(active_ctx->ctx, role, &attrs); + { + join_worker_call_t* call + = (join_worker_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->role = role; + call->attrs = attrs; + + rc = xcetest_run_interruptible(join_worker_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("join"); + return rc; + } + } if (rc < 0) { SET_ERROR(-rc); @@ -3715,6 +4533,22 @@ static const flagspec_t jobstart_flags[] = { }; #define JOBSTART_NFLAGS (sizeof(jobstart_flags) / sizeof(jobstart_flags[0])) +typedef struct { + xcecli_ctx_t conn; + job_pidinfo_t info; + unsigned int pids[1]; +} jobstart_call_t; + +static int jobstart_thunk(void* ctxp) +{ + jobstart_call_t* call = (jobstart_call_t*)ctxp; + int rc = xcecli_notify_jobstart(call->conn, &call->info); + + free(call); + + return rc; +} + int cmd_jobstart(int argc, char* argv[]) { int rc; @@ -3767,7 +4601,26 @@ int cmd_jobstart(int argc, char* argv[]) info.pgids = NULL; info.exdata = NULL; - rc = xcecli_notify_jobstart(active_ctx->ctx, &info); + { + jobstart_call_t* call = (jobstart_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->info = info; + call->pids[0] = pids[0]; + call->info.pids = call->pids; + + rc = xcetest_run_interruptible(jobstart_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("jstart"); + return rc; + } + } + if (rc < 0) { SET_ERROR(-rc); perror("xcecli_jobstart"); @@ -3785,6 +4638,21 @@ static const flagspec_t jobfinish_flags[] = { }; #define JOBFINISH_NFLAGS (sizeof(jobfinish_flags) / sizeof(jobfinish_flags[0])) +typedef struct { + xcecli_ctx_t conn; + xce_jobfinish_t finish; +} jobfinish_call_t; + +static int jobfinish_thunk(void* ctxp) +{ + jobfinish_call_t* call = (jobfinish_call_t*)ctxp; + int rc = xcecli_notify_jobfinish(call->conn, &call->finish); + + free(call); + + return rc; +} + int cmd_jobfinish(int argc, char* argv[]) { int rc; @@ -3839,7 +4707,24 @@ int cmd_jobfinish(int argc, char* argv[]) finish.jobid = jobid; finish.exit_code = code; - rc = xcecli_notify_jobfinish(active_ctx->ctx, &finish); + { + jobfinish_call_t* call = (jobfinish_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->finish = finish; + + rc = xcetest_run_interruptible(jobfinish_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("jfinish"); + return rc; + } + } + if (rc < 0) { SET_ERROR(-rc); perror("xcecli_jobfinish"); @@ -3866,6 +4751,21 @@ static const flagspec_t jobfail_flags[] = { }; #define JOBFAIL_NFLAGS (sizeof(jobfail_flags) / sizeof(jobfail_flags[0])) +typedef struct { + xcecli_ctx_t conn; + job_failinfo_t failinfo; +} jobfail_call_t; + +static int jobfail_thunk(void* ctxp) +{ + jobfail_call_t* call = (jobfail_call_t*)ctxp; + int rc = xcecli_notify_jobfail(call->conn, &call->failinfo); + + free(call); + + return rc; +} + int cmd_jobfail(int argc, char* argv[]) { int rc; @@ -3918,7 +4818,24 @@ int cmd_jobfail(int argc, char* argv[]) failinfo.code = code; failinfo.detail = "synthetic fact-stream failure (xce_test jfail)"; - rc = xcecli_notify_jobfail(active_ctx->ctx, &failinfo); + { + jobfail_call_t* call = (jobfail_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->failinfo = failinfo; + + rc = xcetest_run_interruptible(jobfail_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("jfail"); + return rc; + } + } + if (rc < 0) { SET_ERROR(-rc); perror("xcecli_jobfail"); @@ -4070,7 +4987,15 @@ static int resval_value_matches(xce_val_t* sent, xce_val_t* got) * Notes: sub-second in practice for a controller-local query) -- NOT an * arbitrary long poll. */ -static void resupdate_report_one(char* name, char* host, xce_val_t* sent) +/* xce-test-cli-ux task 3.2: returns 0 when the verification loop below ran + * to its normal conclusion (accepted, or rejected after exhausting its + * retry budget), or -EINTR when the operator interrupted a still-in-flight + * follow-up query -- reusing resval_call_t/resval_thunk (defined above, + * next to cmd_resval, which drives the identical xcecli_resval() call). + * The caller (cmd_resupdate) propagates -EINTR up as its own return value + * (Requirement 1.3), stopping this verification pass immediately rather + * than continuing on to any further submitted (resource, worker) pairs. */ +static int resupdate_report_one(char* name, char* host, xce_val_t* sent) { int attempt; int ever_found = 0; @@ -4079,18 +5004,37 @@ static void resupdate_report_one(char* name, char* host, xce_val_t* sent) /* set_resval()'s Value_u_svalue fallback path leaves .name unset * (pre-existing gap predating this task -- see set_resval()'s own * comment above); nothing usable to query or report here. */ - return; + return 0; } for (attempt = 0; attempt < RESVAL_VERIFY_MAX_ATTEMPTS; attempt++) { - xce_resval_t** got = NULL; - int rc; + resval_call_t* call; + xce_resval_t** got; + int rc; if (attempt > 0) { usleep((useconds_t)(RESVAL_VERIFY_RETRY_MS * US_PER_MS)); } - rc = xcecli_resval(active_ctx->ctx, host, name, &got); + call = (resval_call_t*)calloc(1, sizeof(*call)); + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->host = host; + call->res = name; + + rc = xcetest_run_interruptible(resval_thunk, call); + if (rc == -EINTR) { + errno = EINTR; + perror("resupdate"); + return rc; + } + + got = call->vals; + free(call); + if (rc > 0) { int matched = resval_value_matches(sent, got[0]->val); int i; @@ -4104,7 +5048,7 @@ static void resupdate_report_one(char* name, char* host, xce_val_t* sent) if (matched) { printf("%s: accepted\n", name); - return; + return 0; } /* Found but the value differs -- could still be a stale value * about to be overwritten by the just-sent update's propagation, @@ -4119,6 +5063,8 @@ static void resupdate_report_one(char* name, char* host, xce_val_t* sent) } else { printf("%s: rejected: unknown resource\n", name); } + + return 0; } static const flagspec_t resupdate_flags[] = { @@ -4129,6 +5075,27 @@ static const flagspec_t resupdate_flags[] = { }; #define RESUPDATE_NFLAGS (sizeof(resupdate_flags) / sizeof(resupdate_flags[0])) +/* Array sizes below are hardcoded to 16 to match cmd_resupdate's own + * MAX_N_RES -- that macro is #define'd inside cmd_resupdate's own body + * (this file's pre-existing convention for this one local constant), so + * it is not yet in scope at this point in the file. */ +typedef struct { + xcecli_ctx_t conn; + int nres; + xce_resval_t vals[16]; + xce_resval_t* valptrs[16]; +} resupdate_call_t; + +static int resupdate_thunk(void* ctxp) +{ + resupdate_call_t* call = (resupdate_call_t*)ctxp; + int rc = xcecli_update_resvals(call->conn, call->nres, call->valptrs); + + free(call); + + return rc; +} + int cmd_resupdate(int argc, char* argv[]) { #define MAX_N_RES (16) @@ -4232,7 +5199,31 @@ int cmd_resupdate(int argc, char* argv[]) } } - rc = xcecli_update_resvals(active_ctx->ctx, nres, &valptrs[0]); + { + resupdate_call_t* call = (resupdate_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->nres = nres; + memcpy(call->vals, vals, sizeof(xce_resval_t) * (size_t)nres); + { + int i; + for (i = 0; i < nres; i++) { + call->valptrs[i] = &call->vals[i]; + } + } + + rc = xcetest_run_interruptible(resupdate_thunk, call); + if (rc == -EINTR) { + errno = EINTR; + perror("resupdate"); + return rc; + } + } + if (rc >= 0) { int i; @@ -4240,9 +5231,18 @@ int cmd_resupdate(int argc, char* argv[]) /* The update RPC's own reply can't carry per-name accept/reject * (see the comment above resupdate_report_one()) -- verify each - * submitted pair individually via a follow-up query instead. */ + * submitted pair individually via a follow-up query instead. An + * interrupt during one of THOSE follow-up queries stops this + * verification pass immediately and is reported as this command's + * own failure (Requirement 1.3), rather than silently skipping the + * remaining pairs' verification. */ for (i = 0; i < nres; i++) { - resupdate_report_one(subnames[i], vals[i].host, vals[i].val); + int report_rc + = resupdate_report_one(subnames[i], vals[i].host, vals[i].val); + + if (report_rc == -EINTR) { + return report_rc; + } } } else { SET_ERROR(-rc); @@ -4251,12 +5251,41 @@ int cmd_resupdate(int argc, char* argv[]) return 0; } +typedef struct { + xcecli_ctx_t conn; + int role; +} join_sched_call_t; + +static int join_sched_thunk(void* ctxp) +{ + join_sched_call_t* call = (join_sched_call_t*)ctxp; + int rc = xcecli_join(call->conn, call->role, NULL); + + free(call); + + return rc; +} + int cmd_join_sched(int argc, char* argv[]) { - int role = HostRole_scheduler; - int rc = -1; + int role = HostRole_scheduler; + int rc = -1; + join_sched_call_t* call = (join_sched_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + call->role = role; + + rc = xcetest_run_interruptible(join_sched_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("join"); + return rc; + } - rc = xcecli_join(active_ctx->ctx, role, NULL); if (rc < 0) { SET_ERROR(-rc); perror("xce_join"); @@ -4339,12 +5368,44 @@ static int print_configlist(xce_configlist_t clist) return 0; } +typedef struct { + xcecli_ctx_t conn; + xce_clusterconf_t* conf; + int rc; +} getconf_call_t; + +static int getconf_thunk(void* ctxp) +{ + getconf_call_t* call = (getconf_call_t*)ctxp; + + call->conf = NULL; + call->rc = xcecli_clusterconf(call->conn, &call->conf); + + return call->rc; +} + int cmd_getconf(int argc, char* argv[]) { xce_clusterconf_t* conf; - int rc; + int rc; + getconf_call_t* call = (getconf_call_t*)calloc(1, sizeof(*call)); + + if (call == NULL) { + return -ENOMEM; + } + + call->conn = active_ctx->ctx; + + rc = xcetest_run_interruptible(getconf_thunk, call); + if (rc == -EINTR) { + SET_ERROR(EINTR); + perror("getconf"); + return rc; + } + + conf = call->conf; + free(call); - rc = xcecli_clusterconf(active_ctx->ctx, &conf); if (rc < 0) { SET_ERROR(-rc); perror("xce_clusterconf"); @@ -5237,9 +6298,40 @@ int main(int argc, char* argv[]) prompt[0] = 0x0; fill_prompt(active_ctx, prompt); strcat(prompt, ">"); - buf = readline(prompt); + + /* xce-test-cli-ux task 3.2, Requirement 1.2/1.4: reset break_out + * immediately before this specific readline() call, so the check + * just below attributes a NULL return to a fresh interrupt that + * happened DURING THIS call, never to a stale flag left over from + * an earlier interrupted command or an earlier interrupted + * readline() call -- mirrors cmd_hello's own pre-existing + * "break_out = 0;-then-loop" idiom just above it in this file. */ + break_out = 0; + buf = readline(prompt); if (buf == NULL) { + /* task 1.3's own flagged concern (tasks.md Implementation + * Notes): glibc/libedit's readline() returns NULL on BOTH a + * genuine EOF (Ctrl-D) AND a read interrupted by SIGINT + * (confirmed empirically -- xcetest_install_sigint() clears + * SA_RESTART, so the underlying read() is interrupted rather + * than transparently retried, and libedit's readline() itself + * does not retry across that EINTR; it just returns NULL, + * discarding whatever partial line was in its edit buffer). + * The two cases are NOT distinguishable from readline()'s + * return value alone -- break_out (freshly reset above) is + * what disambiguates them: a real Ctrl-C sets it via + * sigbreak() before readline() returns; a genuine EOF leaves + * it untouched. On the interrupt path, discard the (already + * libedit-discarded) input line and simply redisplay the + * prompt -- the session must never exit as a side effect of a + * single Ctrl-C keypress (Requirement 1.4). */ + if (break_out) { + break_out = 0; + printf("\n"); /* fresh line: Ctrl-C doesn't itself echo one */ + continue; + } + execute_line("quit"); continue; } diff --git a/src/controller/testsuite/test_interruptcall.c b/src/controller/testsuite/test_interruptcall.c index 1f214142..d172cc4c 100644 --- a/src/controller/testsuite/test_interruptcall.c +++ b/src/controller/testsuite/test_interruptcall.c @@ -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); } diff --git a/src/controller/testsuite/test_test_cli.c b/src/controller/testsuite/test_test_cli.c index 2c8bdec7..bcea7bb3 100644 --- a/src/controller/testsuite/test_test_cli.c +++ b/src/controller/testsuite/test_test_cli.c @@ -151,8 +151,10 @@ * CONCERNS in this task's status report if a future task instead needs * cross-file reuse. */ +#include #include #include +#include #include #include #include @@ -160,8 +162,10 @@ #include #include #include +#include #include #include +#include #include #include @@ -1434,6 +1438,465 @@ static void test_walcompare_unrecognized_flag_rejected_with_usage(void) clitest_buf_free(&err); } +/* ------------------------------------------------------------------ */ +/* Interruptible Call Adapter end-to-end (task 3.2, Requirement 1) */ +/* ------------------------------------------------------------------ */ + +/* clitest_start_blackhole_listener - starts a "black hole" TCP peer on + * 127.0.0.1:`port`: it accepts every incoming connection but never reads + * or writes a single byte on any of them, so a client that connects and + * sends a request gets no reply, ever. This is the vehicle for exercising + * Requirement 1's "blocked awaiting a cluster RPC reply" scenario + * deterministically: xce_test's own client library still completes a + * fast, successful TCP connect() (something IS listening and accepting), + * so the actual multi-second block this test needs to interrupt is + * exactly the one interruptcall.c's own header describes -- the blocking + * `xceapi.c` reply wait (`cbuf_timedget`'s fixed 10s deadline), not the + * OS-level connect() path. Returns the listener's pid (to be reaped via + * clitest_stop_blackhole_listener()) on success, or -1 on failure. */ +static pid_t clitest_start_blackhole_listener(int port) +{ + int lfd; + struct sockaddr_in addr; + int opt = 1; + pid_t pid; + + lfd = socket(AF_INET, SOCK_STREAM, 0); + if (lfd < 0) { + return -1; + } + + setsockopt(lfd, SOL_SOCKET, SO_REUSEADDR, &opt, sizeof(opt)); + + memset(&addr, 0, sizeof(addr)); + addr.sin_family = AF_INET; + addr.sin_addr.s_addr = htonl(INADDR_LOOPBACK); + addr.sin_port = htons((uint16_t)port); + + if (bind(lfd, (struct sockaddr*)&addr, sizeof(addr)) != 0) { + close(lfd); + return -1; + } + if (listen(lfd, 16) != 0) { + close(lfd); + return -1; + } + + pid = fork(); + if (pid < 0) { + close(lfd); + return -1; + } + + if (pid == 0) { + /* Child: accept forever, never read/write any accepted socket -- + * a deliberate silent peer, so the parent xce_test's blocking RPC + * wait never completes on its own. */ + for (;;) { + int cfd = accept(lfd, NULL, NULL); + + if (cfd < 0) { + if (errno == EINTR) { + continue; + } + _exit(0); + } + /* Leave `cfd` open and untouched; just keep accepting more. */ + } + } + + close(lfd); /* the parent process only needs the child running */ + return pid; +} + +static void clitest_stop_blackhole_listener(pid_t pid) +{ + if (pid > 0) { + kill(pid, SIGKILL); + waitpid(pid, NULL, 0); + } +} + +static long clitest_elapsed_ms(const struct timespec* start) +{ + struct timespec now; + + clock_gettime(CLOCK_MONOTONIC, &now); + return ((long)(now.tv_sec - start->tv_sec) * 1000L) + + ((now.tv_nsec - start->tv_nsec) / 1000000L); +} + +/* test_interrupt_abandons_blocked_rpc_well_before_timeout -- Requirement + * 1.1/1.3/1.5, this task's Required Test Scenario 1+2: a command blocked + * on a real RPC reply (against the black-hole peer above, so the ~10s + * cbuf_timedget deadline is genuinely reachable) is interrupted by a real + * SIGINT; control must return to the prompt in a small fraction of that + * 10s deadline, the command must be reported as failed (Requirement 1.3), + * and the SAME connection/context must accept a normal, non-RPC follow-up + * command immediately afterward (proving the context/connection were left + * intact, Requirement 1.5), all without the session exiting. */ +static void test_interrupt_abandons_blocked_rpc_well_before_timeout(void) +{ + char xce_test_path[PATH_MAX]; + char* const argv[] = { "xce_test", "--norc", NULL }; + clitest_session_t sess; + clitest_buf_t out; + clitest_buf_t err; + int exit_code = -999; + int port = 19990 + (int)(getpid() % 1000); + char masters_cmd[64]; + pid_t blackhole; + struct timespec interrupt_sent_at; + + blackhole = clitest_start_blackhole_listener(port); + TEST_ASSERT_TRUE_MESSAGE(blackhole > 0, + "must be able to start the black-hole TCP " + "listener this test blocks against"); + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)), + "must be able to resolve the sibling xce_test binary's path"); + + clitest_buf_init(&out); + clitest_buf_init(&err); + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_spawn(&sess, xce_test_path, argv), + "clitest_spawn must launch xce_test behind a real pty"); + + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000), + "expected the initial 'xce>' interactive prompt on stdout"); + + snprintf(masters_cmd, sizeof(masters_cmd), "set masters 127.0.0.1:%d", + port); + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, masters_cmd)); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "client")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 5000), + "entering the client context must succeed even though its " + "connection target is a silent black hole (connect setup does not " + "itself block)"); + + /* `hello` issues exactly one blocking xcecli_hello() RPC (interrupt- + * call-adapter-wrapped, task 3.2) -- against a peer that never + * replies, this call would otherwise block for ~10s. */ + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "hello")); + + /* Give the command time to actually reach the blocking RPC wait + * (connect + send are fast; this margin is generous relative to + * both) before sending the interrupt, so this test genuinely + * exercises the "interrupt a wait already in flight" path rather + * than racing dispatch itself. */ + usleep(300000); /* 300ms */ + + /* Reset the capture buffers before sending the interrupt: "xce/client>" + * already appears once in `out` from when this session first entered + * the client context above, so waiting for that SAME substring against + * the accumulated buffer would trivially "succeed" on the very first + * poll regardless of whether a NEW prompt is genuinely printed after + * the interrupt -- discarding prior output makes the wait below (and + * the elapsed-time measurement it gates) observe a real, fresh event. */ + clitest_buf_free(&out); + clitest_buf_free(&err); + clitest_buf_init(&out); + clitest_buf_init(&err); + + clock_gettime(CLOCK_MONOTONIC, &interrupt_sent_at); + TEST_ASSERT_EQUAL_INT(0, clitest_send_signal(&sess, SIGINT)); + + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 3000), + "the client-context prompt must reappear after the interrupt, well " + "before the RPC's own ~10s deadline"); + + TEST_ASSERT_LESS_THAN_MESSAGE( + 2000L, clitest_elapsed_ms(&interrupt_sent_at), + "control must return to the prompt in a small fraction of the RPC's " + "~10s deadline (observed well under 2s in practice, given the " + "adapter's documented 50ms poll interval)"); + + /* Requirement 1.5: the SAME connection/context must accept a normal + * follow-up command immediately -- `echo` makes no RPC call at all, + * so an immediate, correct reply here proves the session (and its + * client context) is genuinely alive and responsive, not merely + * appearing to be so. */ + TEST_ASSERT_EQUAL_INT( + 0, clitest_send_line(&sess, "echo cli_after_interrupt_marker")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "cli_after_interrupt_marker", + false, 3000), + "a follow-up command on the same connection/context must execute " + "normally right after the interrupt"); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit")); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code), + "xce_test did not exit within the harness's safety timeout after " + "'quit'"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 1, exit_code, + "the interrupted 'hello' must be recorded as a failed command " + "(Requirement 1.3), matching this session's overall exit code"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_stop_blackhole_listener(blackhole); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + +/* test_interrupt_at_empty_prompt_discards_line_without_exiting -- + * Requirement 1.2/1.4, this task's Required Test Scenario 3: a Ctrl-C with + * nothing in flight (a partially-typed, unsubmitted line at the prompt) + * must discard that line and redisplay the prompt, never exit the session + * as a side effect -- proven by sending a real, valid command right after + * and confirming it executes normally. */ +static void test_interrupt_at_empty_prompt_discards_line_without_exiting(void) +{ + char xce_test_path[PATH_MAX]; + char* const argv[] = { "xce_test", "--norc", NULL }; + clitest_session_t sess; + clitest_buf_t out; + clitest_buf_t err; + int exit_code = -999; + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)), + "must be able to resolve the sibling xce_test binary's path"); + + clitest_buf_init(&out); + clitest_buf_init(&err); + + /* disable_echo=false (clitest_spawn_ex, not plain clitest_spawn): this + * test needs libedit's REAL interactive line-editing state (in + * particular, that a partial, uncommitted line is genuinely discarded + * from its edit buffer on the EINTR path) -- clitest_spawn()'s default + * echo-off session makes THIS project's linked libedit fall back to a + * silent/non-interactive input mode (task 2.3's own documented finding, + * see clitest_spawn_ex()'s header comment) that does NOT discard a + * pending partial line across an interrupted read(), so a truly + * interactive echo-on session is required to observe the correct + * (and, empirically confirmed against a genuine terminal-like pty, + * actually-happening) discard behavior here. */ + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_spawn_ex(&sess, xce_test_path, argv, false), + "clitest_spawn_ex must launch xce_test behind a real pty with echo " + "on"); + + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000), + "expected the initial 'xce>' interactive prompt on stdout"); + + /* A partial, unsubmitted line -- nothing in flight, no command has + * been dispatched yet. */ + TEST_ASSERT_EQUAL_INT(0, clitest_send_partial(&sess, "ech")); + usleep(300000); /* let libedit's readline() actually be blocked on it */ + + /* Reset the capture buffers before sending the interrupt: "xce>" + * already appears once in `out` from the session's initial prompt + * above, so waiting for that SAME substring against the accumulated + * buffer would trivially "succeed" on the very first poll regardless + * of whether a NEW prompt is genuinely printed after the interrupt. */ + clitest_buf_free(&out); + clitest_buf_free(&err); + clitest_buf_init(&out); + clitest_buf_init(&err); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_signal(&sess, SIGINT)); + + /* Wait for the REDISPLAYED prompt (main()'s new break_out-then-printf + * path) before sending anything else -- this is what makes the next + * step race-free: only once "xce>" has genuinely reappeared do we know + * the interrupted readline() call has already returned and a BRAND + * NEW readline() call, with a fresh/empty edit buffer, is active. + * Sending the next line immediately after clitest_send_signal() + * (without waiting for this) would race the child's own signal + * delivery/handling latency -- if that next line's bytes arrive before + * the child has actually re-entered its blocking read(), they get + * queued as more input to the STILL-PENDING original read() instead of + * starting a fresh one, undetectably from this harness's side (this + * was empirically confirmed while building this test: it manifests as + * a corrupted "echecho ..." command whose raw, still-echoed-back typed + * text nonetheless contains the search marker, making the naive + * text-search assertion below pass despite the underlying command + * actually failing -- caught instead by the exit-code assertion at the + * end, which is what motivated this explicit wait). */ + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce>", false, 3000), + "the prompt must be redisplayed after Ctrl-C at an empty/partial " + "prompt, before this test sends anything further"); + + /* Requirement 1.4: never exit as a side effect of a single Ctrl-C. + * Send a real, valid command and confirm it executes normally -- + * this is the semantic, redraw-independent proof this session is + * still alive and its discarded partial line did not leak into the + * next command's input (e.g. becoming "echecho ..."). */ + TEST_ASSERT_EQUAL_INT( + 0, clitest_send_line(&sess, "echo cli_still_alive_marker")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "cli_still_alive_marker", + false, 3000), + "a real command sent right after Ctrl-C at an empty/partial prompt " + "must execute normally, proving the session did not exit"); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit")); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code), + "xce_test did not exit within the harness's safety timeout after " + "'quit'"); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, exit_code, + "a session with no genuinely failing command must still exit 0"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + +/* test_interrupt_then_immediate_teardown_no_uaf -- xce-test-cli-ux task 3.2 + * remediation round 1 regression guard (bug-1795). Interrupts a blocked RPC + * exactly like test_interrupt_abandons_blocked_rpc_well_before_timeout + * above, but where that test deliberately WAITS for the interrupted prompt + * to redisplay before sending anything else (so ITS OWN elapsed-time + * measurement is race-free), THIS test does the opposite on purpose: it + * sends ".." (the "go up a context" command, bound to cmd_up) IMMEDIATELY + * after the SIGINT, with no settling wait at all -- racing leave_ctx()'s + * connection teardown (invoked via cmd_up) against + * whatever worker thread was abandoned by the interrupt, which may still + * be running for up to its own ~10s deadline (interruptcall.c's + * xcetest_run_interruptible()/xcecli_wakeup()). This is exactly the crux + * the round-1 review flagged as a genuine use-after-free: before this + * remediation, leave_ctx() called xcecli_stop()/xcecli_ctx_fini() (which + * unconditionally frees the connection's api_ctx_t, src/lib/client.c) + * right after a merely best-effort xcecli_wakeup() nudge, with nothing + * forcing it to actually wait for that abandoned worker to finish + * touching the connection first. The fix under test is + * xcetest_wait_connection_idle() (interruptcall.c/.h), which leave_ctx() + * now calls, right before teardown, to block until the SAME per-connection + * semaphore registry xcetest_run_interruptible() itself uses proves no + * worker is active on this connection. + * + * The actual proof this test relies on is running under ASAN (this + * project's `asan` build profile) with a completely clean stderr -- a + * genuine heap-use-after-free is caught by ASAN the instant it happens, + * independent of anything this test does afterward, so the exit-code and + * "no AddressSanitizer text anywhere in stderr" assertions below are what + * actually catch a regression; a merely-non-hanging, prompt-reappeared + * session alone would not have caught the original bug on a timing-lucky + * run. */ +static void test_interrupt_then_immediate_teardown_no_uaf(void) +{ + char xce_test_path[PATH_MAX]; + char* const argv[] = { "xce_test", "--norc", NULL }; + clitest_session_t sess; + clitest_buf_t out; + clitest_buf_t err; + int exit_code = -999; + int port = 19990 + (int)(getpid() % 1000) + 1; + char masters_cmd[64]; + pid_t blackhole; + + blackhole = clitest_start_blackhole_listener(port); + TEST_ASSERT_TRUE_MESSAGE(blackhole > 0, + "must be able to start the black-hole TCP " + "listener this test blocks against"); + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, resolve_xce_test_path(xce_test_path, sizeof(xce_test_path)), + "must be able to resolve the sibling xce_test binary's path"); + + clitest_buf_init(&out); + clitest_buf_init(&err); + + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_spawn(&sess, xce_test_path, argv), + "clitest_spawn must launch xce_test behind a real pty"); + + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000), + "expected the initial 'xce>' interactive prompt on stdout"); + + snprintf(masters_cmd, sizeof(masters_cmd), "set masters 127.0.0.1:%d", + port); + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, masters_cmd)); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "client")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce/client>", false, 5000), + "entering the client context must succeed even though its " + "connection target is a silent black hole"); + + /* Same as the sibling test above: issue the blocking `hello` RPC + * against the black hole, then give it a moment to actually reach the + * blocking wait before interrupting -- this margin is only about + * getting the WORKER thread genuinely blocked inside the RPC, not + * about settling anything after the interrupt (see below, which is + * the entire point of this test). */ + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "hello")); + usleep(300000); /* 300ms */ + + /* Reset capture buffers before the interrupt, same rationale as the + * sibling test above ("xce/client>" already appears once from + * entering the context). */ + clitest_buf_free(&out); + clitest_buf_free(&err); + clitest_buf_init(&out); + clitest_buf_init(&err); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_signal(&sess, SIGINT)); + + /* THE POINT OF THIS TEST: send the "go up a context" command ("..", + * bound to cmd_up -- see test.c's command tables) IMMEDIATELY, with NO + * settling wait for the interrupted prompt to reappear first (unlike + * the sibling test above). This is deliberately the adversarial timing + * for the use-after-free the review found -- leave_ctx() (invoked + * here via cmd_up) must genuinely BLOCK, via + * xcetest_wait_connection_idle(), until the abandoned worker thread + * has actually finished with this connection before + * xcecli_stop()/xcecli_ctx_fini() free it, rather than racing it. */ + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "..")); + + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "xce>", false, 5000), + "the top-level 'xce>' prompt must reappear after '..' tears down " + "the interrupted client connection, with no crash in between"); + + /* A real follow-up command on the brand-new top-level context proves + * the whole process (not merely the pty) is still alive and + * undamaged after the teardown race above. */ + TEST_ASSERT_EQUAL_INT( + 0, clitest_send_line(&sess, "echo cli_after_teardown_marker")); + TEST_ASSERT_TRUE_MESSAGE( + clitest_wait_for_text(&sess, &out, &err, "cli_after_teardown_marker", + false, 3000), + "a follow-up command after the interrupt-then-immediate-teardown " + "race must execute normally"); + + TEST_ASSERT_EQUAL_INT(0, clitest_send_line(&sess, "quit")); + TEST_ASSERT_EQUAL_INT_MESSAGE( + 0, clitest_wait_for_exit(&sess, &out, &err, 5000, &exit_code), + "xce_test did not exit within the harness's safety timeout after " + "'quit'"); + + TEST_ASSERT_TRUE_MESSAGE( + exit_code >= 0, + "the session must exit normally, never via a fatal signal (e.g. " + "ASAN's SIGABRT/SIGSEGV on a detected heap-use-after-free)"); + TEST_ASSERT_NULL_MESSAGE( + strstr(err.data, "AddressSanitizer"), + "no AddressSanitizer diagnostic (e.g. heap-use-after-free) may " + "appear anywhere in stderr across this whole session"); + + clitest_kill_and_reap(&sess); + clitest_close(&sess); + clitest_stop_blackhole_listener(blackhole); + clitest_buf_free(&out); + clitest_buf_free(&err); +} + void test_test_cli(void) { UnitySetTestFile(__FILE__); @@ -1446,4 +1909,7 @@ void test_test_cli(void) RUN_TEST(test_qctrl_flag_forms_short_and_long_both_accepted); RUN_TEST(test_migrated_commands_sample_reject_unrecognized_flag); RUN_TEST(test_walcompare_unrecognized_flag_rejected_with_usage); + RUN_TEST(test_interrupt_abandons_blocked_rpc_well_before_timeout); + RUN_TEST(test_interrupt_at_empty_prompt_discards_line_without_exiting); + RUN_TEST(test_interrupt_then_immediate_teardown_no_uaf); }