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
326 lines
19 KiB
C
326 lines
19 KiB
C
#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
|