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

feat(xce-harness-component): relocate xce_test into standalone harness component

Browse source 
Move xce_test's 15 production sources + headers from src/controller/testsuite/
into a new src/harness/ component with its own meson.build, and strip every
now-dangling controller-side reference (11 controller_unit_test_src entries,
4 standalone test-binary blocks, 11 controller_test.c extern/RUN_TEST lines)
so the controller build is never left broken.

Fixes a broken relative-symlink bridge (xce_test_controller_link) added to
keep two controller-embedded tests (test_test_cli.c, test_xcetest_process_entry.c)
finding the relocated xce_test binary until task 2.1 moves them too.

Discovered and documented in design.md: four controller-domain regression
tests call functions directly from four of xce_test's own files, requiring
controller_unit_test to permanently recompile them from src/harness/ — a
real dependency neither the original discovery nor design audit caught.

controller_unit_test: 1228 Tests 0 Failures 0 Ignored (verified via the
Unity binary's own printed summary line, not just meson's aggregate count).

Task 1.1 of xce-harness-component.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01RLSNJhNoULvVcUhXEFafQC
This commit is contained in:
Rongsong Shen 2026-09-14 05:08:15 +00:00
parent b8ef82c1b2
commit cf55d691de
No known key found for this signature in database
GPG key ID: 5BCDAAE0123E9FE2
36 changed files with 550 additions and 180 deletions
Download patch file Download diff file Expand all files Collapse all files

326
src/controller/testsuite/interruptcall.h
View file

@ -1,326 +0,0 @@
#pragma once
/*
* interruptcall.h -- xcetest_run_interruptible(): a detached-worker-thread
* wrapper that lets a command make a blocking xce_* RPC call cancellably
* from the operator's Ctrl-C, without ever touching libxce's own blocking
* primitive (xce-test-cli-ux spec, task 1.4; Requirements 1.1, 1.2, 1.3,
* 1.4).
*
* ROOT CAUSE this module addresses: every blocking xce_* call
* (src/lib/xceapi.c) waits on pthread_cond_timedwait() with a fixed
* 10-second deadline (cbuf_deadline_10s/cbuf_timedget, src/lib/mylock.c) --
* a wait a signal-setting flag alone cannot interrupt (glibc's condvar
* wait retries internally). Modifying that shared libxce primitive is out
* of this spec's boundary (research.md's Architecture Pattern Evaluation:
* "Modify libxce's cbuf_timedget to poll a cancellation token" -- Rejected,
* out of boundary). The fix is entirely client-side: run the existing
* blocking call, UNMODIFIED, on a detached worker thread; have the calling
* thread poll xcetest_interrupted() (xcetest_interrupt.h, task 1.3) on a
* short interval instead of blocking on the call itself; on interrupt,
* abandon the wait and return immediately, leaving the worker thread to
* finish (or hit its own 10s deadline) on its own -- satisfying "leave the
* connection intact" (Requirement 1.5) and the design's "cancelling a
* client-side wait does not need to cancel the server-side request" note.
*
* THE HARD PART this module exists to get right: the worker thread and the
* calling thread both have a legitimate reason to touch the heap-allocated
* call state backing one call (the worker to run fn()/store its result;
* the caller to read that result) -- but at most ONE of them may free it,
* and it must be resolved without a lock the caller would have to block
* on (that would defeat the whole point of "abandon immediately"). See
* design.md's "Heap-Block Ownership Handoff" state diagram and
* research.md's "Decision: Interruptible RPC via detached worker thread
* with CAS-based ownership handoff" for the full rationale, including why
* pthread_cancel() was rejected (cancellation-safety of
* pthread_cond_timedwait()'s internal state is not established).
*
* PER-CONNECTION LOCK REGISTRY (task 1.5, Requirements 1.1, 1.5): an
* abandoned worker thread from task 1.4's mechanism above keeps running
* against whatever connection state `ctx` closes over. If the operator's
* very next command reuses that SAME connection, its own call would race
* the still-running abandoned worker for that connection's single-
* outstanding-reply state -- not a rare edge case, but the direct, likely
* consequence of Ctrl-C's own purpose (cancel, then immediately try
* something else). xcetest_run_interruptible() closes this by acquiring
* a per-connection lock -- looked up or lazily created in a small,
* process-lifetime registry keyed by whatever `xcetest_set_active_
* connection()` was last called with -- BEFORE spawning the worker, and
* holding it for the call's full lifetime, including through an
* abandonment (ownership of "release the lock" passes to the worker in
* that case, exactly mirroring the heap-block CAS handoff above). Lock
* acquisition is itself interruptible (the same break_out poll loop), so
* a second call waiting on a connection a prior abandoned call still
* holds can itself be Ctrl-C'd out of that wait, before ever acquiring
* the lock or spawning a worker of its own. See design.md's "Interruptible
* RPC Call and Ctrl-C Abandonment" sequence diagram and research.md's
* "Decision: Per-connection lock serializes overlapping calls on the
* Interruptible Call Adapter" for the full rationale.
*
* WHY THE LOCK IS A POSIX SEMAPHORE (sem_t), NOT A MUTEX: the handoff
* above is a genuine CROSS-THREAD release -- the REPL/caller thread
* acquires the lock before spawning the worker, but on the abandonment
* path it is the still-running WORKER thread that later releases it,
* once fn() finishes. A pthread_mutex_t (of ANY type, including the
* default PTHREAD_MUTEX_NORMAL) is the wrong primitive for this: POSIX
* defines pthread_mutex_t in terms of a single owning thread, and
* classifies pthread_mutex_unlock() from a non-owning thread as
* undefined behavior for every mutex type -- PTHREAD_MUTEX_ERRORCHECK/
* _RECURSIVE are simply *required* to detect and report that UB (hence
* the EPERM an earlier version of this code hit immediately), while
* NORMAL/DEFAULT are merely *permitted* not to detect it. POSIX does NOT
* separately declare cross-thread unlock well-defined for NORMAL; it
* just does not mandate an error return for it. A pthread_mutex_t-based
* fix that swaps ERRORCHECK for the default type therefore only trades a
* loud crash for a silent reliance on non-portable, POSIX-classified-UB
* behavior -- something that happens to hold on this project's actual
* target (glibc/NPTL's PTHREAD_MUTEX_NORMAL has had no ownership
* tracking for decades, and XCE's Nix-hermetic build is glibc/Linux-only)
* but is the wrong thing to build on when a fully-defined alternative
* exists at the same code complexity.
*
* sem_t has NO ownership concept at all: sem_wait()/sem_trywait()/
* sem_post() may be called from any thread regardless of which thread
* called which, with fully-defined POSIX semantics for exactly this
* cross-thread signal pattern. Each registry entry is therefore a binary
* semaphore (sem_init(&sem, 0, 1)) used as a mutual-exclusion lock:
* sem_trywait() takes the place of pthread_mutex_trylock() in the
* interruptible poll loop below (EAGAIN where trylock would have
* returned EBUSY), and sem_post() takes the place of
* pthread_mutex_unlock() at both release sites -- with no UB caveat on
* either.
*
* WHY THE REGISTRY IS A SINGLY-LINKED LIST, NOT A GROWABLE ARRAY: a
* sem_t* handed out by this registry may be cached inside a call block
* that a still-running abandoned worker thread holds onto indefinitely
* (up to ~10s, per the heap-block ownership handoff above). A growable
* array's realloc() can MOVE every previously-returned element the
* moment a later, unrelated connection is first seen -- silently
* invalidating that worker's pointer into stale/reused memory. A linked
* list's nodes are individually allocated and, per this registry's own
* "entries are never removed" contract, never freed or moved for the
* rest of the process's life, so a pointer into it stays valid forever
* once handed out.
*
* CONCURRENCY OF THE REGISTRY ITSELF: per design.md's Implementation
* Notes, "the registry's lookup/insert step needs no synchronization of
* its own -- a new registry entry is only ever created from the single
* REPL thread that issues commands" (detached worker threads never
* initiate a new top-level call; they only run the `fn` they were given
* and release the lock/free the block they were handed). The SEMAPHORE
* each entry owns, once handed out, is of course safe for concurrent
* wait/post from different threads -- that safety is sem_t's own job,
* not the registry's.
*/
#include <stdatomic.h>
/* A blocking call this module runs on a detached worker thread. `ctx` is
* whatever heap-owned state `fn` needs -- see xcetest_run_interruptible()'s
* preconditions below for the one hard rule about what `fn` may touch. */
typedef int (*xcetest_blocking_fn)(void* ctx);
/* The heap-allocated call block's lifecycle, resolved by exactly one
* atomic compare-and-swap regardless of which side "wins":
* RUNNING -- fn() has not yet returned, and no one has decided
* to abandon the call yet. Initial state.
* DONE_UNCLAIMED -- fn() returned and the worker's CAS from RUNNING to
* this state succeeded (i.e. the caller had not yet
* abandoned it). The CALLER is left owning the
* block: it reads rc and frees it.
* ABANDONED -- the caller's CAS from RUNNING to this state
* succeeded (i.e. it observed an interrupt before
* fn() finished). The WORKER is left owning the
* block: once fn() eventually returns, its own CAS
* attempt (RUNNING -> DONE_UNCLAIMED) fails against
* this state, and it frees the block itself instead.
* See design.md's "Heap-Block Ownership Handoff" stateDiagram-v2 -- this
* enum's three values are exactly its three states. */
typedef enum {
XCT_RUNNING,
XCT_DONE_UNCLAIMED,
XCT_ABANDONED
} xcetest_call_state_t;
/*
* Sets the connection-identity key that xcetest_run_interruptible() below
* reads INTERNALLY when a new call starts, to look up (or lazily create)
* that connection's entry in the per-connection lock registry (task 1.5,
* see the file header above). This is a simple settable-elsewhere,
* read-internally piece of process state -- deliberately mirroring how
* break_out/xcetest_interrupted() already works -- NOT a parameter added
* to xcetest_run_interruptible()'s own signature, which stays exactly
* `(fn, ctx)` per this task's design constraint.
*
* SEAM FOR A FUTURE CALLER (task 3.2): production code (test.c's cmd_*
* functions) does not exist inside this module and has no notion of
* "the currently active command_ctx_t's connection pointer" that this
* header can reach into directly -- test.c itself is never linked into
* this binary's unit tests (see this file's own header and
* xcetest_interrupt.h's). The intended real wiring is: before invoking
* xcetest_run_interruptible() for a blocking RPC, a cmd_* function (or a
* shared dispatch point above all of them) calls
* xcetest_set_active_connection(<that connection's own stable pointer,
* e.g. the command_ctx_t's connection field>). Any stable, comparable-by-
* identity pointer works as `conn_key` -- this module never dereferences
* it, only compares it by pointer equality against the registry's keys.
* Until task 3.2 wires that in, xce_test's real binary calls this
* exactly once, from wherever main()/the dispatcher sets up each
* command's active connection context.
*
* Concurrency: like break_out, callable only from the single REPL thread
* that issues top-level commands -- see the file header's "CONCURRENCY
* OF THE REGISTRY ITSELF" note. Passing the same key across unrelated
* connections would incorrectly serialize them against each other;
* passing a fresh/different key per connection is what keeps unrelated
* connections independent (Requirement 1.5, "leave ... its underlying
* connection intact").
*/
void xcetest_set_active_connection(void* conn_key);
/*
* Runs `fn(ctx)` on a detached worker thread while THIS (the calling)
* thread polls xcetest_interrupted() every ~50ms (design.md's Interruptible
* Call Adapter component), so a Ctrl-C observed during the wait abandons
* it immediately instead of blocking for fn()'s own, possibly much
* longer, duration.
*
* Before spawning that worker, this function first acquires -- also
* interruptibly, via the same ~50ms xcetest_interrupted() poll, but
* using sem_trywait() instead of a blocking sem_wait() -- the
* per-connection lock for whichever connection key was last passed to
* xcetest_set_active_connection() (task 1.5; see the file header's "PER-
* CONNECTION LOCK REGISTRY" section above). This serializes this call
* against any other in-flight or abandoned-but-still-running call on the
* same connection, so that connection's single-outstanding-reply state
* is never touched by two threads at once.
*
* Preconditions:
* - `fn` is non-NULL.
* - `fn` touches only what `ctx` itself heap-owns -- NEVER memory on
* this function's caller's own stack frame. This is the precondition
* that makes the -EINTR postcondition below safe: once ownership of
* `ctx` has passed to the still-running worker, the caller's own
* stack frame may have long since unwound.
*
* Postconditions:
* - Returns fn()'s own return value when fn() finishes before this
* thread's abandon-CAS wins the race (whether or not
* xcetest_interrupted() ever became true during the wait) -- see the
* header comment's "near-simultaneous" case below. On this path, the
* per-connection lock is released by this (the caller's) thread,
* after reading rc and freeing the block.
* - Returns -EINTR when this thread observes an interrupt WHILE
* WAITING FOR THE PER-CONNECTION LOCK -- i.e. before ever acquiring
* it. No worker is spawned for this call at all in this case, and
* nothing needs releasing, since this call never held the lock to
* begin with.
* - Returns -EINTR when this thread observes an interrupt AFTER
* acquiring the lock and spawning the worker, and its own CAS
* (RUNNING -> ABANDONED) wins the race. On this return, the caller
* MUST NOT touch `ctx` again -- ownership of both the call block AND
* the per-connection lock has passed to the still-running worker
* thread, which will finish fn(), free the call block, and release
* the lock on its own, asynchronously, with no further signal back
* to this caller.
* - Returns -EINVAL if `fn` is NULL (fn is never invoked in this case;
* no lock is acquired).
* - Returns a negative errno from the per-connection lock registry
* lookup (e.g. -ENOMEM), from sem_trywait() itself for an
* unexpected error other than EAGAIN, or from pthread_attr_init()/
* pthread_create() if the worker thread could not be set up or
* spawned at all; in every one of these cases fn() is never invoked,
* any call block already allocated is freed before returning, and
* the per-connection lock -- if this call had already acquired it --
* is released before returning.
*
* The near-simultaneous race (this thread notices the interrupt in the
* same window the worker finishes fn()) is resolved by the SAME single
* CAS deciding both sides at once: whichever of {this thread's
* RUNNING->ABANDONED attempt, the worker's RUNNING->DONE_UNCLAIMED
* attempt} executes first wins, and the loser observes that the state is
* no longer RUNNING and falls back to the OTHER side's outcome -- a
* caller who loses the abandon-CAS falls through to the normal
* completion path (reads rc, frees the block, releases the lock, returns
* rc) instead of returning -EINTR; a worker who loses the done-CAS frees
* the block and releases the lock itself instead of leaving them for the
* caller. Exactly one free() of the block, and exactly one release of
* the lock, are therefore reachable on every possible interleaving --
* never zero, never two.
*/
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
* even declared unless XCE_TESTING is defined (i.e. never visible to the
* real xce_test binary): incremented (via atomic_fetch_add) by the worker
* thread immediately after it frees an ABANDONED call block.
*
* Exists solely so test_interruptcall.c's forced-abandonment test can
* confirm the abandoned worker's asynchronous cleanup genuinely completed
* before the test process exits, rather than merely hoping a fixed sleep
* was long enough -- a still-running detached worker racing a leak
* detector's exit-time check could otherwise either mask a real bug or
* (more likely, if the implementation above is correct) flag a false
* leak on an allocation that was always going to be freed, just not yet.
*/
extern _Atomic int xcetest_test_abandoned_free_count;
#endif