diff --git a/.claude/rules/c-runtime-memory.md b/.claude/rules/c-runtime-memory.md index 68dde28..6c8f0f7 100644 --- a/.claude/rules/c-runtime-memory.md +++ b/.claude/rules/c-runtime-memory.md @@ -52,6 +52,14 @@ iteration, or a collector that quietly stops working). gate.** LeakSanitizer runs atexit, and the server is torn down with `kill` against no SIGTERM handler, so LSan never runs in the server process — `make asan-http` catches UAF/overflow/UB (those report at the moment of the - bug) but not leaks. Verify a request path with RSS growth instead: drive N - requests and sample `VmRSS` from `/proc//status` in batches. Steady-state - growth per request is the signal (#731 measures at 160 B/request this way). + bug) but not leaks. The gate for this class is + `tests/test_http_rss_growth.sh` (suite [45c]): RSS growth between two + **steady-state** checkpoints, never baseline-to-end (the first requests carry + ~1.4 MB of one-time arena warmup, ~18x the real rate). Add a check there when + you add a request path that allocates. It **skips on sanitizer builds** by + design — ASan's redzones/quarantine grow RSS 567 B/req on a leak-free binary. +- **`eigs_json_encode` borrows its argument and `eigs_json_parse_value` returns + an owned ref.** `eigs_json_encode(make_num(x))` leaks the `make_num`; a parsed + Value must be decref'd on *every* path including early returns, and the value + read out of it before the decref. Three of the five call sites in + `ext_http.c` had this wrong (#731) — if you touch one, re-audit the rest. diff --git a/CHANGELOG.md b/CHANGELOG.md index 31bf14d..0dec142 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -123,6 +123,28 @@ All notable changes to EigenScript are documented here. ### Fixed +- **`ext_http.c` leaked a `Value` on three request paths (#731).** Every + `eigs_json_parse_value` call site in the file was audited after #731 reported + the first; three of five leaked. `shared_incr` dropped two per call — the + parsed counter on *both* the success and the type-mismatch early return, plus + the `make_num` handed to `eigs_json_encode`, which borrows rather than takes + ownership. `builtin_http_post` never released the parsed headers object, and + `http_route_authed`'s shared-store auth branch never released the parsed + `require_auth` string. The last two were found by auditing the class rather + than the instance, and are per-request leaks on the same footing as the + reported one. Measured on a release build against a live server: `shared_incr` + 159 B/req → **0**, an authenticated route 188 B/req → **0**. + + Gated by `tests/test_http_rss_growth.sh` (suite [45c]), which measures RSS + growth between two steady-state checkpoints. This shape is deliberate: no + sanitizer can catch this class here, because LeakSanitizer reports from an + `atexit` handler and the test server is torn down with `kill` against no + SIGTERM handler — the ASan suite reported 32/32 green over the leak for as + long as it existed. The gate skips itself on a sanitizer build, where ASan's + own redzones and quarantine produce 567 B/req of growth on a *fixed* binary + and would swamp the signal. Threshold validated by planting the fault back: + both checks fail without the fix and pass with it (#752). + - **`continue` never ended its loop iteration's env, silently truncating a module's exports (#722).** `AST_BREAK` emits `OP_LOOP_ENV_END` before its jump when the loop allocated a per-iteration env — the pairing #335's diff --git a/src/ext_http.c b/src/ext_http.c index b0bd13a..969685e 100644 --- a/src/ext_http.c +++ b/src/ext_http.c @@ -407,6 +407,10 @@ Value* builtin_http_post(Value *arg) { hdr_count++; } } + /* 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 + * not used past this point. */ + val_decref(hdr_obj); char data_arg[512]; snprintf(data_arg, sizeof(data_arg), "@%s", req_path); @@ -560,14 +564,23 @@ Value* builtin_shared_incr(Value *arg) { if (idx >= 0) { int pos = 0; Value *parsed = eigs_json_parse_value(s->shared[idx].json, &pos); - if (!parsed || parsed->type != VAL_NUM) { + /* Read the number out BEFORE dropping the ref — decref may free it — + * and drop it on the mismatch path too, which is the one an early + * return makes easy to miss. */ + int bad = (!parsed || parsed->type != VAL_NUM); + if (!bad) cur = parsed->data.num; + val_decref(parsed); + if (bad) { pthread_mutex_unlock(&s->shared_mu); return make_null(); } - cur = parsed->data.num; } double new_val = cur + delta; - char *new_json = eigs_json_encode(make_num(new_val)); + /* eigs_json_encode borrows its argument, so the Value handed to it is + * ours to release. */ + Value *new_v = make_num(new_val); + char *new_json = eigs_json_encode(new_v); + val_decref(new_v); long new_json_len = (long)strlen(new_json); long key_len = (long)strlen(key_v->data.str); long old_bytes = (idx >= 0) ? (long)strlen(s->shared[idx].json) : 0; @@ -1171,6 +1184,7 @@ static void handle_request(int fd) { if (parsed && parsed->type == VAL_STR) { auth_src = xstrdup(parsed->data.str); } + val_decref(parsed); } pthread_mutex_unlock(&srv->shared_mu); } diff --git a/tests/run_all_tests.sh b/tests/run_all_tests.sh index 07d640b..66bce52 100755 --- a/tests/run_all_tests.sh +++ b/tests/run_all_tests.sh @@ -1902,6 +1902,29 @@ if ! echo "$HTTP_PROBE_OUT" | grep -q "undefined variable"; then echo " PASS: all $SL_PASS HTTP slow-loris checks" fi echo "" + + # [45c] Per-request leak gate by RSS growth (#731, #752). Not covered by the + # ASan job: LSan runs atexit and the test server is killed, so a per-request + # leak in ext_http.c is invisible to every sanitizer build. + echo "[45c/47] HTTP per-request leak gate (RSS growth, 2 checks)" + RSS_OUTPUT=$(bash "$TESTS_DIR/test_http_rss_growth.sh" 2>&1) + RSS_PASS=$(echo "$RSS_OUTPUT" | grep -c "PASS:" || true) + RSS_FAIL=$(echo "$RSS_OUTPUT" | grep -c "FAIL:" || true) + TOTAL=$((TOTAL + RSS_PASS + RSS_FAIL)) + PASS=$((PASS + RSS_PASS)) + FAIL=$((FAIL + RSS_FAIL)) + if [ "$RSS_FAIL" -gt 0 ]; then + echo " FAIL: $RSS_FAIL HTTP RSS-growth check(s) failed" + echo "$RSS_OUTPUT" | grep "FAIL:" | head -5 + elif [ "$RSS_PASS" -eq 0 ]; then + # Print WHY nothing ran. "PASS: all 0 checks" reads as a pass while + # meaning the gate never executed — the skip is legitimate (sanitizer + # build, no curl, no procfs) but it must not look like coverage. + echo "$RSS_OUTPUT" | grep "SKIP:" | head -1 + else + echo " PASS: all $RSS_PASS HTTP RSS-growth checks" + fi + echo "" else echo "[44-45/47] HTTP tests SKIPPED (binary built without EIGENSCRIPT_EXT_HTTP)" echo "" diff --git a/tests/test_http_rss_growth.sh b/tests/test_http_rss_growth.sh new file mode 100755 index 0000000..622757b --- /dev/null +++ b/tests/test_http_rss_growth.sh @@ -0,0 +1,161 @@ +#!/bin/bash +# Per-request leak gate for ext_http.c, by RSS growth (EigenScript #731, #752). +# +# WHY NOT A SANITIZER: LeakSanitizer reports from an atexit handler, and this +# server is torn down with `kill` against no SIGTERM handler, so LSan never runs +# in the server process. `make asan-http` compiles ext_http.c under ASan and +# catches use-after-free / overflow / UB — those report at the moment of the bug +# — but a per-request LEAK is invisible to it. #731 sat in the request path of +# `shared_incr` while the ASan suite reported 32/32 green. RSS growth is the +# instrument that works on a process that dies by signal. +# +# RSS1 shared_incr: the counter path the builtin exists for. Leaked two +# Values per call (a parsed JSON Value on both the success and the +# type-mismatch path, plus the make_num handed to eigs_json_encode). +# RSS2 http_route_authed with a shared-store auth source: leaked the parsed +# `require_auth` Value on every authenticated request. Same class, found +# while auditing the other eigs_json_parse_value call sites for #731. +# +# METHOD: measure between two STEADY-STATE checkpoints, never baseline-to-end. +# The first requests against a fresh server also carry one-time arena/heap +# warmup (+1.4 MB when #731 was measured), which is ~18x the real leak rate and +# would make any baseline-to-end threshold meaningless. So: warm up, sample A, +# drive a measured batch, sample B, and assert B-A. +# +# Requests are driven with curl's [1-N] globbing — one process, one connection, +# ~1.1s per 500 requests. The query string differs per request but the router +# matches on path, so every one hits the route under test. +set -u +TESTS_DIR="$(cd "$(dirname "$0")" && pwd)" +SRC_DIR="$(cd "$TESTS_DIR/.." && pwd)/src" +EIGS="$SRC_DIR/eigenscript" + +PASS=0 +FAIL=0 +ok() { echo " PASS: $1"; PASS=$((PASS+1)); } +fail() { echo " FAIL: $1${2:+ ($2)}"; FAIL=$((FAIL+1)); } + +if ! command -v curl >/dev/null 2>&1; then + echo " SKIP: curl not available" + echo "HTTP_RSS: 0 passed, 0 failed (skipped)" + exit 0 +fi + +# VmRSS comes from procfs — Linux only. macOS CI legs skip rather than fail. +if [ ! -r /proc/self/status ]; then + echo " SKIP: /proc not available (VmRSS unreadable on this platform)" + echo "HTTP_RSS: 0 passed, 0 failed (skipped)" + exit 0 +fi + +# MUST NOT run against a sanitizer build. ASan's redzones, quarantine and +# allocator metadata make RSS climb on their own: the FIXED binary measures +# 0 kB growth built with `make http` and 1108 kB (567 B/req) built with +# `make asan-http`. That is instrument overhead, not a leak, and it would +# false-fail this gate in CI's asan-http step. RSS growth is only meaningful +# on a release build. +if grep -qa "__asan_init" "$EIGS" 2>/dev/null; then + echo " SKIP: sanitizer build — RSS growth is dominated by ASan overhead, not leaks" + echo "HTTP_RSS: 0 passed, 0 failed (skipped)" + exit 0 +fi + +WARMUP=1000 # discarded: absorbs arena/heap warmup +MEASURE=2000 # the batch B-A is measured over +# Pre-fix rates were ~160 B/req (RSS1) and ~136 B/req (RSS2), i.e. ~320 kB and +# ~270 kB over MEASURE. Post-fix both measure exactly 0 kB. 64 kB sits ~5x under +# the fault and well above page-granularity noise. +THRESHOLD_KB=64 + +rss_of() { awk '/^VmRSS/{print $2}' "/proc/$1/status" 2>/dev/null; } + +# $1 = label, $2 = route path, $3 = server script body +run_growth_check() { + local label="$1" route="$2" body="$3" + local port srv_file srv_pid a b growth + port=$(( (RANDOM % 10000) + 51000 )) + srv_file=$(mktemp /tmp/eigs_rss_srv_XXXXXX.eigs) + printf '%s\n' "$body" | sed "s/__PORT__/$port/" > "$srv_file" + + "$EIGS" "$srv_file" > "/tmp/eigs_rss_srv_$$.log" 2>&1 & + srv_pid=$! + + # Wait for the listener rather than sleeping a fixed interval. + local tries=0 + until curl -s -o /dev/null "http://127.0.0.1:$port$route" 2>/dev/null; do + tries=$((tries + 1)) + if [ "$tries" -gt 100 ] || ! kill -0 "$srv_pid" 2>/dev/null; then + fail "$label server never came up" "port $port" + kill "$srv_pid" 2>/dev/null || true + rm -f "$srv_file" + return + fi + sleep 0.1 + done + + curl -s -o /dev/null "http://127.0.0.1:$port$route?[1-$WARMUP]" + a=$(rss_of "$srv_pid") + curl -s -o /dev/null "http://127.0.0.1:$port$route?[1-$MEASURE]" + b=$(rss_of "$srv_pid") + + kill "$srv_pid" 2>/dev/null || true + wait "$srv_pid" 2>/dev/null || true + rm -f "$srv_file" "/tmp/eigs_rss_srv_$$.log" + + if [ -z "$a" ] || [ -z "$b" ]; then + fail "$label could not read VmRSS" "a='$a' b='$b'" + return + fi + growth=$((b - a)) + if [ "$growth" -le "$THRESHOLD_KB" ]; then + ok "$label steady-state growth ${growth} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + else + fail "$label leaked ${growth} kB over $MEASURE reqs" \ + "$(( growth * 1024 / MEASURE )) B/req; threshold ${THRESHOLD_KB} kB" + fi +} + +run_growth_check "RSS1 shared_incr" "/sinc" \ +'r is http_route of ["GET", "/sinc", "code", "shared_incr of [\"counter\", 1]"] +s is http_serve of [__PORT__]' + +# require_auth is seeded as a source string that evaluates to "" (= allow), so +# every /secret request takes the shared-store auth branch at ext_http.c:~1170. +PORT_A=$(( (RANDOM % 10000) + 52000 )) +AUTH_SRV=$(mktemp /tmp/eigs_rss_auth_XXXXXX.eigs) +cat > "$AUTH_SRV" < "/tmp/eigs_rss_auth_$$.log" 2>&1 & +AUTH_PID=$! +tries=0 +until curl -s -o /dev/null "http://127.0.0.1:$PORT_A/asetup" 2>/dev/null; do + tries=$((tries + 1)) + if [ "$tries" -gt 100 ] || ! kill -0 "$AUTH_PID" 2>/dev/null; then break; fi + sleep 0.1 +done + +if curl -s "http://127.0.0.1:$PORT_A/asetup" | grep -q "ok" \ + && curl -s "http://127.0.0.1:$PORT_A/secret" | grep -q "top secret"; then + curl -s -o /dev/null "http://127.0.0.1:$PORT_A/secret?[1-$WARMUP]" + A=$(rss_of "$AUTH_PID") + curl -s -o /dev/null "http://127.0.0.1:$PORT_A/secret?[1-$MEASURE]" + B=$(rss_of "$AUTH_PID") + GROWTH=$((B - A)) + if [ "$GROWTH" -le "$THRESHOLD_KB" ]; then + ok "RSS2 authed route steady-state growth ${GROWTH} kB over $MEASURE reqs (<= ${THRESHOLD_KB} kB)" + else + fail "RSS2 authed route leaked ${GROWTH} kB over $MEASURE reqs" \ + "$(( GROWTH * 1024 / MEASURE )) B/req" + fi +else + fail "RSS2 authed route did not come up" "port $PORT_A" +fi +kill "$AUTH_PID" 2>/dev/null || true +wait "$AUTH_PID" 2>/dev/null || true +rm -f "$AUTH_SRV" "/tmp/eigs_rss_auth_$$.log" + +echo "HTTP_RSS: $PASS passed, $FAIL failed" +[ "$FAIL" -eq 0 ]