From b01138c20b687e72ce91d0a45c071080698a3d10 Mon Sep 17 00:00:00 2001 From: InauguralPhysicist Date: Tue, 28 Jul 2026 22:19:57 -0500 Subject: [PATCH] fix(trace): the tape is the process's, not a connection worker's (#739, #755, #760) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit trace_shutdown() is a PROCESS-wide teardown — it closes the one tape, drops an embedder's trace sink, and shuts down the replay reader — and ext_http.c's per-connection worker called it when it finished a request. Each worker owns its own EigsState, but the tape belongs to the process, so the FIRST request served closed it and every later request's records were silently dropped. Measured on the unfixed runtime, 400 requests against a code route that assigns one marker each: records on tape: 1 (fd already closed) and after the fix, mid-run, same test: records on tape: 336 (fd still open) The same teardown freed the temporal prev-table, running slot_decref on entries recorded by other, still-live threads. The teardown is now split. trace_shutdown() keeps the process-wide half. A new trace_thread_release() releases only the calling thread's history, and eigs_thread_detach calls it for every thread — so the HTTP worker no longer touches the trace subsystem at all. Ordering is preserved: the release runs while eigs_current still points at the thread and before the freelists and arena go, and trace_shutdown still releases the process owner's own table before the global env dies (eigs_close). The prev-table moved onto EigsThread behind bridge macros. That is not a new scope — it is the scope the table always effectively had: it is keyed by INTERNED NAME POINTER and the intern table is already per-thread, so two threads' "x" were never the same key. Process-global storage bought nothing and cost ownership, and per-thread needs no lock because only the owning thread touches its table. Corrects one premise in the issue: cross-state history reads were NOT possible. Pointer-keying already separated them. The damage was the teardown, not the lookup. The tape ENCODING is untouched, so no format-version bump (#411) — this was an ownership bug, not a format one. Also in the same file: #755 — http_post silently sent NO headers when `headers` was a JSON object. The code tested VAL_LIST while eigs_json_parse_object returns a VAL_DICT, so the loop never ran and the request went out bare: no error, a normal response body, an Authorization header silently not sent. Both shapes are accepted now, and a headers argument that parses to neither raises type_mismatch instead of dropping them. BUILTINS.md never said which shape was expected; it does now. #760 — HS27/HS28 failed one CI run reporting "aggregate body budget (got 000)", a DoS-gate regression that had not happened: both checks had received EMPTY responses because the server was never reachable. Three measured defects — the listen port was drawn from 50000-59999, entirely inside the kernel's ephemeral range (32768-60999), so the suite's own curl clients could hold the port the next server tried to bind; the readiness loop's real budget was 3.58s, not the ~33s its --max-time implied, because a refused connection returns instantly; and neither failure was detected, so the test ran against a server that was not there while the log naming the cause was written to /tmp and never read. Shared pick_port + wait_ready helpers now, a setup failure reports itself as one and quotes the log, and the main server in the same file had both defects too. Gates: release 3273/3273; asan+ubsan 3277/3277 and asan-http 3373/3373, leak tally 0 in both; observer corpus 14/14; test_replay.sh 24/24; HTTP suite 42/42; freestanding symbol gate stage 1+2 OK; jit-smoke; embed stack soak OK. HS33 validated red against the unfixed runtime (got 1 request marker, expected >= 3). Closes #739 Closes #755 Closes #760 Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 53 ++++++++++++++ docs/BUILTINS.md | 2 +- docs/EMBEDDING.md | 10 +++ docs/TRACE.md | 20 ++++++ src/eigenscript.h | 12 ++++ src/ext_http.c | 41 ++++++++++- src/state.c | 2 + src/trace.c | 84 +++++++++++++++------- src/trace.h | 12 +++- tests/test_http_server.sh | 147 ++++++++++++++++++++++++++++++++++---- 10 files changed, 340 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8038958a..10b0fbfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,42 @@ All notable changes to EigenScript are documented here. ### Fixed +- **Every HTTP connection worker tore down the process trace tape, so the tape + died after the first request (#739).** `trace_shutdown()` is a process-wide + teardown — it closes the one tape, drops an embedder's trace sink, and shuts + down the replay reader — and `ext_http.c`'s per-connection worker called it + when it finished a request. Each worker owns its own `EigsState`, but the + tape belongs to the process, so the *first* request served closed it and + every later request's records were silently dropped. Measured on the + unfixed runtime: **one record for four hundred requests**, with the server's + tape fd already closed; after the fix, 336 records had reached disk mid-run + and the fd was still open. The same teardown freed the temporal prev-table, + running `slot_decref` on entries recorded by other, still-live threads. The + teardown is now split: `trace_shutdown()` keeps the process-wide half, and a + new `trace_thread_release()` releases only the calling thread's history — + called by `eigs_thread_detach` for every thread, so the HTTP worker no longer + touches the trace subsystem at all. The prev-table moved onto `EigsThread` + behind bridge macros, which is the scope it always effectively had: it is + keyed by *interned name pointer* and the intern table is per-thread, so two + threads' `x` were never the same key — process-global storage bought nothing + and cost ownership. It needs no lock, since only the owning thread touches + its table. The tape encoding is untouched, so no format-version bump — this + was an ownership bug, not a format one. (Corrects one premise in the issue: + cross-state history reads were *not* possible, because pointer-keying already + separated them.) + +- **`http_post` silently sent NO headers when `headers` was a JSON object + (#755).** The documented signature is `http_post of [url, headers, body]`, + and an object is the shape a caller reaches for first — but the code tested + `VAL_LIST` while `eigs_json_parse_object` returns a `VAL_DICT`, so the loop + never ran and the request went out bare. No error, no warning, a normal + response body: an `Authorization` header silently not sent. Only the flat + alternating array form worked, and `docs/BUILTINS.md` never said which shape + was expected. Both shapes are accepted now, and a headers argument that + parses to neither raises `type_mismatch` instead of quietly dropping them + (an unparseable argument, including `""`, still means "no headers" — the + documented idiom). BUILTINS.md now states the accepted shapes. + - **`report` labelled a still-moving value "equilibrium" at a full window, violating the agreement guarantee it documents (#735).** `report of x` resolves the six windowed bands in priority order and then, if none is true, @@ -381,6 +417,23 @@ All notable changes to EigenScript are documented here. ### Changed +- **`tests/test_http_server.sh` no longer reports a setup failure as a feature + failure (#760).** `HS27`/`HS28` failed one CI run with "aggregate body budget + (got 000)" — a DoS-gate regression that had not happened. Both checks had + simply received *empty* responses, because the server was never reachable. + Three defects, each measured: the listen port was drawn from `50000-59999`, + entirely inside the kernel's ephemeral range (`32768-60999` on Linux), so the + suite's own `curl` clients could hold the port the next server tried to bind; + the readiness loop's real budget was **3.58s**, not the ~33s its + `--max-time 1` implied, because a refused connection returns instantly; and + neither failure was detected, so the test proceeded against a server that + was not there and the log naming the real cause was written to `/tmp` and + never read. There is now a shared `pick_port` (below the ephemeral floor, + read from `/proc` where available) and a `wait_ready` with a wall-clock + deadline, and a server that never comes up fails as a **setup** failure + quoting its own log. The main server in the same file had both defects too + and now uses the helpers. + - **`make asan-http` puts the HTTP + model extensions under a sanitizer for the first time, and CI runs the suite that way.** `make asan` compiles those extensions out, so `ext_http.c` — a network-facing server and client, the most diff --git a/docs/BUILTINS.md b/docs/BUILTINS.md index 3855fb90..9c94bb45 100644 --- a/docs/BUILTINS.md +++ b/docs/BUILTINS.md @@ -538,7 +538,7 @@ axis. Three controls bound it: | `http_serve` | `http_serve of port` | Start blocking HTTP server | | `http_request_body` | `http_request_body of null` | Get current request body | | `http_session_id` | `http_session_id of null` | Get current session ID | -| `http_post` | `http_post of [url, headers, body]` | HTTP POST via curl (no shell) | +| `http_post` | `http_post of [url, headers, body]` | HTTP POST via curl (no shell). `headers` is a JSON **string**, either an object (`"{\"X-Key\": \"v\"}"`) or a flat alternating array (`"[\"X-Key\", \"v\"]"`); `""` means no headers. Any other parsed shape raises `type_mismatch` rather than silently sending none (#755). Max 32 headers; CR/LF is stripped from both halves | | `http_request_headers` | `http_request_headers of null` | Get current request headers | ### Per-worker code routes diff --git a/docs/EMBEDDING.md b/docs/EMBEDDING.md index 691b4788..d9671370 100644 --- a/docs/EMBEDDING.md +++ b/docs/EMBEDDING.md @@ -254,6 +254,16 @@ act between evals. While a replay tape is set, nondet builtins return the recorded `N` values in order instead of consulting their live sources; when the tape runs out they fall back to live. +**The sink and the tape are per-PROCESS, not per-state** (#739). With +several states co-located, one sink serves them all, and the teardown +that drops it — `trace_shutdown()`, which `eigs_close` calls — belongs +to whoever owns the process. A per-request or per-task state must not +call it: `ext_http`'s connection worker did, so the first request served +closed the tape and unregistered the host's sink. A worker that wants to +release its own temporal history calls `trace_thread_release()`, which +`eigs_thread_detach` already does for every thread. The per-name history +behind `prev of x` / `at` / `state_at` is per-thread and never shared. + Host builtins participate with the take/record pair, the same contract the runtime's own nondet builtins use: diff --git a/docs/TRACE.md b/docs/TRACE.md index b0083095..d22b8e24 100644 --- a/docs/TRACE.md +++ b/docs/TRACE.md @@ -284,6 +284,26 @@ in-run time travel. resolve in O(history/64) instead of O(history). The index adds one `int` per 64 history entries and an O(1) min-update per assign. - Per-assign cost of the history: one cache line + a pointer compare. +- **The history is per-thread; the tape is per-process** (#739). The + history table is keyed by *interned name pointer*, and the intern + table lives on `EigsThread`, so two threads' `x` were never the same + key — per-thread is the only scope on which the table is coherent, + and it needs no lock because only its owning thread touches it. It is + released by `eigs_thread_detach` (and by `trace_shutdown` for the + process owner's own thread, which must run before the global env dies + — the slots it drops can reach the env). +- **`trace_shutdown()` is process-wide and a worker must never call + it.** It closes the one tape, drops the embed sink, and shuts down + the replay reader. Every `ext_http` connection worker used to call it + on finishing a request: the first request served closed the tape, so + every later request's records were silently dropped (measured: one + record for four hundred requests), an embedder's sink was + unregistered by whichever request arrived first, and prev-table slots + recorded by other still-live threads were decref'd. A worker that + wants to clean up after itself wants `trace_thread_release()`, which + touches only its own history. Nothing about the tape *encoding* + changed here, so no format-version bump: this was an ownership bug, + not a format one. Language-level syntax and examples: [SYNTAX.md](SYNTAX.md), [GRAMMAR.md](GRAMMAR.md). diff --git a/src/eigenscript.h b/src/eigenscript.h index 769cde0f..f48eeb87 100644 --- a/src/eigenscript.h +++ b/src/eigenscript.h @@ -586,6 +586,15 @@ struct EigsState { struct EigsThread { EigsState *state; Arena arena; + /* #739: temporal prev-table (`prev of x`, `at `, `state_at`). + * Per-THREAD because it is keyed by interned name pointer and the + * intern table above is per-thread — a shared table could never have + * merged two threads' "x". Opaque here; the layout is trace.c's. + * Released by eigs_thread_detach (and by trace_shutdown, which must + * run before the global env dies — see eigs_close). */ + struct TracePrevEntry *prev_tab; + int prev_cap; /* power of two */ + int prev_count; /* Control-flow propagation (return/break/continue out of nested * blocks). All three are checked + cleared by the interpreter * walk and the VM dispatch loop. */ @@ -773,6 +782,9 @@ extern __thread EigsThread *eigs_current; #define g_env_freelist (eigs_current->env_freelist) #define g_env_freelist_count (eigs_current->env_freelist_count) #define g_env_name_interns (eigs_current->env_name_interns) +#define g_prev_tab (eigs_current->prev_tab) +#define g_prev_cap (eigs_current->prev_cap) +#define g_prev_count (eigs_current->prev_count) #define g_parse_depth (eigs_current->parse_depth) #define g_tokenize_depth (eigs_current->tokenize_depth) #define g_vts_depth (eigs_current->vts_depth) diff --git a/src/ext_http.c b/src/ext_http.c index 78d46bcc..b368b0ea 100644 --- a/src/ext_http.c +++ b/src/ext_http.c @@ -409,7 +409,27 @@ Value* builtin_http_post(Value *arg) { int hdr_count = 0; int jpos = 0; Value *hdr_obj = eigs_json_parse_value(headers_json, &jpos); - if (hdr_obj && hdr_obj->type == VAL_LIST) { + /* #755: a JSON OBJECT is the shape a caller reaches for first, and it used + * to send NO headers at all — this tested only VAL_LIST, while + * eigs_json_parse_object returns a VAL_DICT, so the loop never ran and the + * request went out bare with no error and a normal-looking response. An + * Authorization header silently not sent is the worst version of that. Both + * shapes are accepted now: object (natural) and flat alternating array + * (what already worked). Anything else that PARSED is a caller mistake, not + * a no-header request — it says so instead of dropping the headers. */ + if (hdr_obj && hdr_obj->type == VAL_DICT) { + for (int i = 0; i < hdr_obj->data.dict.count && hdr_count < 32 && argc < 90; i++) { + char *hk = xstrdup(hdr_obj->data.dict.keys[i]); + char *hv = value_to_string(hdr_obj->data.dict.vals[i]); + http_strip_crlf(hk); + http_strip_crlf(hv); + snprintf(header_bufs[hdr_count], sizeof(header_bufs[0]), "%s: %s", hk, hv); + free(hk); free(hv); + argv[argc++] = "-H"; + argv[argc++] = header_bufs[hdr_count]; + hdr_count++; + } + } else if (hdr_obj && hdr_obj->type == VAL_LIST) { for (int i = 0; i + 1 < hdr_obj->data.list.count && hdr_count < 32 && argc < 90; i += 2) { char *hk = value_to_string(hdr_obj->data.list.items[i]); char *hv = value_to_string(hdr_obj->data.list.items[i + 1]); @@ -426,6 +446,18 @@ Value* builtin_http_post(Value *arg) { argv[argc++] = header_bufs[hdr_count]; hdr_count++; } + } else if (hdr_obj && hdr_obj->type != VAL_NULL) { + /* Parsed, but neither shape — e.g. a bare string or number. Silently + * sending nothing is the #755 failure mode; say so. An unparseable + * headers argument (including "") still means "no headers", which is + * the documented idiom. */ + rt_error(EK_TYPE, 0, + "http_post: headers must be a JSON object or a flat " + "[key, value, ...] array (got %s)", + val_type_name(hdr_obj->type)); + val_decref(hdr_obj); + unlink(req_path); + TRACE_NONDET_RECORD("http_post", make_str("")); } /* Released here rather than at the end of the function: the pipe/fork * failure paths below return through TRACE_NONDET_RECORD, and hdr_obj is @@ -1386,7 +1418,12 @@ static void *http_conn_thread(void *arg) { handle_request(fd); fd = -1; /* handle_request closed it */ - trace_shutdown(); + /* #739: NO trace_shutdown() here. This worker owns one connection, not the + * process — calling it closed the process tape after the FIRST request + * (every later request's records silently lost), dropped an embedder's + * trace sink, and decref'd prev-table slots recorded by other still-live + * threads. This thread's own prev-table is released by eigs_thread_detach + * below, beside the other per-thread destructors. */ gc_collect_at_exit(global); env_decref(global); g_global_env = NULL; diff --git a/src/state.c b/src/state.c index 5a3d7be6..34863ddd 100644 --- a/src/state.c +++ b/src/state.c @@ -5,6 +5,7 @@ #include "state.h" #include "vm.h" #include "jit.h" +#include "trace.h" /* #739: trace_thread_release on detach */ #if EIGENSCRIPT_EXT_HTTP /* Forward-declared here to avoid pulling ext_http_internal.h (and its @@ -145,6 +146,7 @@ void eigs_thread_detach(void) { jit_thread_destroy(th); vm_thread_destroy(th); task_sched_thread_free(); /* #408: release the cooperative task scheduler */ + trace_thread_release(); /* #739: this thread's prev-table (NOT the tape) */ /* Phase 8: release freelist + intern memory before the EigsThread * struct itself goes. Must run while eigs_current still points at th diff --git a/src/trace.c b/src/trace.c index 6c52bb05..8100a904 100644 --- a/src/trace.c +++ b/src/trace.c @@ -97,7 +97,17 @@ typedef struct { #define HIST_SEG_SHIFT 6 #define HIST_SEG (1 << HIST_SEG_SHIFT) -typedef struct { +/* #739: the prev-table lives on EigsThread, not in a file static. It is keyed + * by INTERNED NAME POINTER, and the intern table is itself per-thread + * (`g_env_name_interns` -> `eigs_current->env_name_interns`), so two threads' + * "x" were never the same key and a shared table could not have merged them + * anyway — process-global storage bought nothing and cost ownership. The + * entries hold slots allocated by their own thread, so per-thread is also the + * only scope on which "release these" is a well-defined operation, and it needs + * no lock: only the owning thread ever touches its table. The tape itself + * (FILE*, sink, replay reader, enable flags) stays process-wide below — one + * process, one tape — which is the distinction `trace_shutdown` got wrong. */ +typedef struct TracePrevEntry { const char *name; EigsSlot prev; EigsSlot current; @@ -115,9 +125,9 @@ typedef struct { int obs_cap; } PrevEntry; -static PrevEntry *g_prev_tab = NULL; -static int g_prev_cap = 0; /* power of two */ -static int g_prev_count = 0; +/* g_prev_tab / g_prev_cap / g_prev_count are bridge macros onto EigsThread + * (eigenscript.h), reached only with a thread attached. Every read path below + * that can run during teardown or from atexit guards on `eigs_current` first. */ /* g_trace_current_line (exported, see trace.h) replaces the old static * line cache: OP_LINE stores it directly instead of paying a call. */ @@ -161,7 +171,7 @@ static void prev_grow(void) { } static void prev_record_assign(const char *name, EigsSlot value) { - if (!name) return; + if (!eigs_current || !name) return; if (g_prev_count * PREV_LOAD_DEN >= g_prev_cap * PREV_LOAD_NUM) { prev_grow(); if (!g_prev_tab) return; @@ -253,7 +263,7 @@ static void prev_record_assign(const char *name, EigsSlot value) { * Gated by g_trace_obs_hist at the call site. */ void trace_record_obs(const char *name, double entropy, double dH, double last_entropy) { - if (!g_prev_tab || !name) return; + if (!eigs_current || !name || !g_prev_tab) return; PrevEntry *e = prev_lookup_slot(g_prev_tab, g_prev_cap, name); if (!e->name || e->hist_count == 0 || !e->obs) return; int idx = e->hist_count - 1; @@ -267,7 +277,7 @@ void trace_record_obs(const char *name, double entropy, double dH, } int trace_query_prev(const char *interned_name, EigsSlot *out) { - if (!interned_name || !out || !g_prev_tab) return 0; + if (!eigs_current || !interned_name || !out || !g_prev_tab) return 0; PrevEntry *e = prev_lookup_slot(g_prev_tab, g_prev_cap, interned_name); if (!e->name || !e->has_prev) return 0; *out = e->prev; @@ -311,7 +321,7 @@ static int find_hist_idx_at_or_before(PrevEntry *e, int line) { } int trace_query_at(int kind, const char *interned_name, int line, EigsSlot *out) { - if (!interned_name || !out || !g_prev_tab) return 0; + if (!eigs_current || !interned_name || !out || !g_prev_tab) return 0; PrevEntry *e = prev_lookup_slot(g_prev_tab, g_prev_cap, interned_name); if (!e->name) return 0; @@ -1022,6 +1032,46 @@ int trace_replay_take(const char *fn, Value **out) { } } +/* #739: release THIS THREAD's prev-table. Idempotent, and a no-op with no + * thread attached (the atexit path runs detached). Must run while + * `eigs_current` still points at the owning thread — the slots it drops are + * that thread's, and their destructors read the bridge macros — which is why + * eigs_thread_detach calls it beside the other Phase-5 destructors, and why + * eigs_close calls trace_shutdown before the global env dies. + * + * This is the half that used to be fused into trace_shutdown, and the fusion + * was the bug: every ext_http connection worker called trace_shutdown when it + * finished a request, so one process-wide teardown ran per HTTP request — + * closing the tape after the FIRST request (every later request's records + * silently lost), unregistering an embedder's sink, and decref'ing prev-table + * slots recorded by other, still-live threads. */ +void trace_thread_release(void) { + if (!eigs_current || !g_prev_tab) return; + PrevEntry *tab = g_prev_tab; + int cap = g_prev_cap; + /* Clear the thread's view FIRST: a destructor reached from slot_decref + * below must not find a half-freed table through the bridge macros. */ + g_prev_tab = NULL; + g_prev_cap = 0; + g_prev_count = 0; + for (int i = 0; i < cap; i++) { + PrevEntry *e = &tab[i]; + if (!e->name) continue; + if (e->has_prev) slot_decref(e->prev); + if (e->has_current) slot_decref(e->current); + for (int j = 0; j < e->hist_count; j++) + slot_decref(e->history[j].value); + free(e->history); + free(e->seg_min); + free(e->obs); + } + free(tab); +} + +/* Process-wide teardown: the tape, the sink, the replay reader. One process, + * one tape — so this belongs to whoever owns the process (main / eigs_close / + * atexit), NEVER to a per-connection or per-task worker. A worker that wants + * to clean up after itself wants trace_thread_release. */ void trace_shutdown(void) { #if !EIGENSCRIPT_FREESTANDING if (g_trace_fp) { @@ -1035,23 +1085,7 @@ void trace_shutdown(void) { g_trace_sink_ud = NULL; g_trace_enabled = 0; - if (g_prev_tab) { - for (int i = 0; i < g_prev_cap; i++) { - PrevEntry *e = &g_prev_tab[i]; - if (!e->name) continue; - if (e->has_prev) slot_decref(e->prev); - if (e->has_current) slot_decref(e->current); - for (int j = 0; j < e->hist_count; j++) - slot_decref(e->history[j].value); - free(e->history); - free(e->seg_min); - free(e->obs); - } - free(g_prev_tab); - g_prev_tab = NULL; - g_prev_cap = 0; - g_prev_count = 0; - } + trace_thread_release(); replay_shutdown(); } diff --git a/src/trace.h b/src/trace.h index 5002c4cf..9baa4982 100644 --- a/src/trace.h +++ b/src/trace.h @@ -67,9 +67,19 @@ extern int g_trace_obs_hist; * multiple times — second and later calls no-op. */ void trace_init(void); -/* Called at process exit to flush and close the tape. */ +/* Called at process exit to flush and close the tape. PROCESS-WIDE: it closes + * the one tape, drops the embed sink, and shuts down the replay reader. A + * per-connection or per-task worker must NOT call this — see #739, where every + * ext_http worker did and the tape died after the first request. */ void trace_shutdown(void); +/* #739: release the CURRENT thread's prev-table (`prev of x` / `at` / + * `state_at` history) without touching the process tape. Idempotent; a no-op + * with no thread attached. Called by eigs_thread_detach for every thread, and + * by trace_shutdown for the process owner's own thread (which must run before + * the global env dies — the slots it drops can reach the env). */ +void trace_thread_release(void); + /* ---- Embed tape seam (the freestanding tape path; see eigs_embed.h). * trace_set_sink installs a byte sink for tape records and enables * recording — the sink receives complete record lines (newline diff --git a/tests/test_http_server.sh b/tests/test_http_server.sh index 1fa22721..e1db8676 100755 --- a/tests/test_http_server.sh +++ b/tests/test_http_server.sh @@ -19,8 +19,42 @@ if ! command -v curl >/dev/null 2>&1; then exit 0 fi -# Pick a random high port to avoid collisions in CI. -PORT=$(( (RANDOM % 10000) + 40000 )) +# ---- Server-start helpers (#760) ---------------------------------------- +# Both defects below made a SETUP failure look like a feature failure: HS27/HS28 +# reported "aggregate body budget (got 000)" when the truth was that the server +# was never reachable. +# +# 1. Pick a listen port BELOW the kernel's ephemeral range. The old +# `(RANDOM % 10000) + 50000` window sits INSIDE it (32768-60999 on Linux), +# and this suite's own curl clients take ephemeral ports from that window — +# so a server could lose the bind to the test's own traffic. +# 2. Wait on a WALL-CLOCK deadline. `30 x (curl --max-time 1; sleep 0.1)` reads +# like ~33s but is ~3.6s measured: a refused connection returns instantly, so +# --max-time only ever bounded the case where something WAS listening. +pick_port() { + local lo hi + lo=$(awk '{print $1}' /proc/sys/net/ipv4/ip_local_port_range 2>/dev/null) + case "$lo" in ''|*[!0-9]*) lo=32768 ;; esac + hi=$((lo - 1)) + lo=$((hi - 11999)) + [ "$lo" -lt 1024 ] && lo=1024 + echo $(( (RANDOM % (hi - lo + 1)) + lo )) +} + +# wait_ready [deadline_seconds] -> 0 ready, 1 timed out +wait_ready() { + local url=$1 limit=${2:-15} start=$SECONDS + while [ $((SECONDS - start)) -lt "$limit" ]; do + curl -s --max-time 1 "$url" >/dev/null 2>&1 && return 0 + sleep 0.1 + done + return 1 +} + +# Pick a listen port below the kernel's ephemeral range (#760) — the old +# 40000-49999 window sits inside it, so this suite's own curl clients could +# hold the port the server then failed to bind. +PORT=$(pick_port) # Static directory for static-file serving. STATIC_DIR=$(mktemp -d /tmp/eigs_http_static_XXXXXX) @@ -95,13 +129,9 @@ cleanup() { } trap cleanup EXIT -# Wait for server to accept connections (up to ~3 seconds). -for _ in $(seq 1 30); do - if curl -s --max-time 1 "http://127.0.0.1:$PORT/ping" > /dev/null 2>&1; then - break - fi - sleep 0.1 -done +# Wait for the server on a wall-clock deadline (#760): the old 30-iteration +# loop was ~3.6s, not the ~33s its --max-time suggested. +wait_ready "http://127.0.0.1:$PORT/ping" 15 || true if ! curl -s --max-time 1 "http://127.0.0.1:$PORT/ping" > /dev/null 2>&1; then echo " FAIL: server never came up on port $PORT" @@ -612,7 +642,7 @@ echo "" # tests (HS12's 17 MiB header probe needs the default 128 MiB budget). A single # upload larger than the budget must be shed with 503 DURING the read (before # routing), and the server must stay alive for the next request. -PORT2=$(( (RANDOM % 10000) + 50000 )) +PORT2=$(pick_port) SRV2=$(mktemp /tmp/eigs_http_srv2_XXXXXX.eigs) cat > "$SRV2" < /tmp/eigs_http_srv2_$$.log 2>&1 & SRV2_PID=$! -for _ in $(seq 1 30); do - curl -s --max-time 1 "http://127.0.0.1:$PORT2/ping" > /dev/null 2>&1 && break - sleep 0.1 -done +if ! wait_ready "http://127.0.0.1:$PORT2/ping" 15; then + # #760: say what actually happened. Reporting the budget check as failed + # here sent a reader hunting a DoS-gate regression that did not exist. + fail "HS27/HS28 setup: server never came up on port $PORT2" \ + "$(tail -3 /tmp/eigs_http_srv2_$$.log 2>/dev/null | tr '\n' ' ')" +fi BIGBODY=$(mktemp /tmp/eigs_http_big_XXXXXX) head -c 2097152 /dev/zero | tr '\0' 'a' > "$BIGBODY" # 2 MiB > 1 MiB budget STATUS=$(curl -s --max-time 5 -o /dev/null -w "%{http_code}" \ @@ -645,6 +677,93 @@ kill "$SRV2_PID" 2>/dev/null || true wait "$SRV2_PID" 2>/dev/null || true rm -f "$SRV2" /tmp/eigs_http_srv2_$$.log +# ---- HS34: http_post header shapes (#755) -------------------------------- +# The signature is `http_post of [url, headers, body]` and a JSON OBJECT is what +# a caller reaches for first. It used to send NO headers: the code tested +# VAL_LIST while eigs_json_parse_object returns VAL_DICT, so the loop never ran +# and the request went out bare — no error, normal-looking response. An +# Authorization header silently not sent is the worst version of that. Both +# shapes must work, and a shape that is neither must say so rather than +# quietly dropping the headers. /hdrs echoes the raw request header block. +CLI=$(mktemp /tmp/eigs_http_cli_XXXXXX.eigs) +cat > "$CLI" </dev/null | tr -d '\r') +rm -f "$CLI" +if echo "$SHAPES" | grep -q "^OBJ:1$"; then + ok "HS34 http_post sends headers given as a JSON object" +else + fail "HS34 http_post object headers" "header did not reach the wire ($(echo "$SHAPES" | tr '\n' ' '))" +fi +if echo "$SHAPES" | grep -q "^ARR:1$"; then + ok "HS34 http_post still sends headers given as a flat array" +else + fail "HS34 http_post array headers" "regressed ($(echo "$SHAPES" | tr '\n' ' '))" +fi +CLI=$(mktemp /tmp/eigs_http_cli_XXXXXX.eigs) +cat > "$CLI" <&1 | tr -d '\r') +rm -f "$CLI" +if echo "$BAD" | grep -q "headers must be a JSON object"; then + ok "HS34 a headers argument of the wrong shape raises instead of dropping" +else + fail "HS34 wrong-shape headers" "expected a type error, got: $(echo "$BAD" | head -1)" +fi + +# ---- HS33: the tape survives more than one request (#739) ---------------- +# Every connection worker owns its own EigsState and used to call +# trace_shutdown() when it finished — a PROCESS-wide teardown run per request. +# The first request served closed the tape, so every later request's records +# were silently dropped (measured: 1 record for 400 requests). The tape is the +# process's execution record; a worker never owns it. +# +# Observation is content-based on purpose: no /proc (macOS runs this suite too) +# and no assumption about stdio buffer size. Each request stamps a distinct +# marker from the shared store FIRST, then writes ~400 padding assignments that +# push the marker out of the buffer — so a marker on disk means that request's +# records really reached the tape. +PORT3=$(pick_port) +SRV3=$(mktemp /tmp/eigs_http_srv3_XXXXXX.eigs) +TAPE3=$(mktemp /tmp/eigs_http_tape3_XXXXXX.tape) +cat > "$SRV3" < /tmp/eigs_http_srv3_$$.log 2>&1 & +SRV3_PID=$! +if ! wait_ready "http://127.0.0.1:$PORT3/hit" 15; then + fail "HS33 setup: trace server never came up on port $PORT3" \ + "$(tail -3 /tmp/eigs_http_srv3_$$.log 2>/dev/null | tr '\n' ' ')" +else + for _ in 1 2 3; do + curl -s --max-time 5 "http://127.0.0.1:$PORT3/hit" > /dev/null 2>&1 + done + sleep 0.5 + MARKERS=$(grep -c '^A zz=' "$TAPE3" 2>/dev/null || echo 0) + # The readiness probe is request 1, so requests 2+ are what the bug ate. + if [ "$MARKERS" -ge 3 ]; then + ok "HS33 tape keeps recording after the first request ($MARKERS markers)" + else + fail "HS33 tape stopped after the first request" \ + "got $MARKERS request markers (expected >= 3)" + fi + if grep -q '^A zz=3' "$TAPE3" 2>/dev/null; then + ok "HS33 a later request's records reach the tape" + else + fail "HS33 later request missing from tape" "no 'A zz=3' record" + fi +fi +kill "$SRV3_PID" 2>/dev/null || true +wait "$SRV3_PID" 2>/dev/null || true +rm -f "$SRV3" "$TAPE3" /tmp/eigs_http_srv3_$$.log + echo "HTTP_SERVER: $PASS passed, $FAIL failed" if [ "$FAIL" -gt 0 ]; then exit 1; fi exit 0