Skip to content

Add integration tests for memory safety and concurrency - #33

Open
jeeyo wants to merge 4 commits into
mainfrom
integration-tests
Open

Add integration tests for memory safety and concurrency#33
jeeyo wants to merge 4 commits into
mainfrom
integration-tests

Conversation

@jeeyo

@jeeyojeeyo commented Sep 4, 2026

Copy link
Copy Markdown
Owner

Groundwork for the memory-safety work: make the server's behaviour observable before changing any of it.

Unblocking the build

3rdparty/quickjs pointed at github.com/jeeyo/quickjs, which returns 404, so the default build couldn't be checked out. Switched to quickjs-ng v0.16.2 and adapted the port: JSMallocFunctions gained js_calloc and passes void *opaque instead of JSMallocState *, libbf.c/cutils.c are gone in favour of dtoa.c, JS_IsError() is single-argument, and JS_BOOL is now bool.

quickjs-ng also keeps its internal assertions on, which is how the first bug below became visible as a clean abort rather than silent corruption.

The tests

Black-box: they run the real isere binary and speak HTTP to it.

FileCovers
test_smoke.pyStartup, routing, malformed input, abrupt disconnects
test_concurrency.pyLatency distribution, concurrent clients, fd leaks, event-loop starvation
test_memory.pyRSS stability across requests, connect/disconnect, partial requests, timers, large bodies
test_risky_js.pyHostile handler code — throwing, embedded NULs, non-string headers, oversized headers, pipelining
test_leaks.pyvalgrind (marked slow)

Two things worth calling out about the design:

  • The harness compiles each test's .js into handler.so with the same xxd/sed recipe as the root CMakeLists.txt, so what's tested matches a normal build.
  • Leak detection compares memory retained at two different request counts rather than reading one run. Most of what isère leaks stays reachable — a leaked FreeRTOS timer block is still linked into a kernel list — so it never shows as "definitely lost", and RSS is too coarse to see it. Growth against request count is the signal that actually works.

Failing tests — these are findings, not broken tests

1. Pipelined requests abort the process. Two requests in one TCP segment:

isere: quickjs.c:2704: JS_FreeRuntime: Assertion `list_empty(&rt->gc_obj_list)' failed.

__on_message_complete (src/httpd.c:196) calls __httpd_cleanup_conn, which vPortFrees the connection — from inside an llhttp callback, while llhttp_execute still holds &conn->llhttp. On the run before this one the same input left the process alive but no longer accepting connections. Timing-dependent behaviour from one input is the usual signature of memory corruption.

2. Response bodies can contain adjacent heap. A handler returning a body with an embedded NUL gets body_len from the JS string (which counts the NUL) but a buffer from strdup() (which stops at it). Writeback sends body_len bytes from the shorter allocation. The test captured what look like live 64-bit heap pointers on the wire — remotely observable, and enough to defeat ASLR.

3. One slow handler blocks every other client. A handler awaiting a timer stalls an unrelated request for ~6s. ISERE_HTTPD_HANDLER_TIMEOUT_MS is declared at include/httpd.h:26 and never read.

4. The drain loop busy-waits without yielding. Under valgrind's thread serialisation, a handler waiting on a FreeRTOS timer starves the timer task and the server stops accepting entirely. It passes without valgrind, so this is about the loop never yielding rather than valgrind itself.

What passes

33/35 fast tests, 6/8 slow. RSS is flat across 1000 requests, connect/disconnect cycles, partial requests, 64 KiB bodies and allocation-heavy handlers; no fd leaks. Latency: p50 0.4 ms serial, 2.4 ms at 8 concurrent clients; at 40 clients (past the 12-connection cap) p95 goes to 1.0 s and throughput drops to ~96 req/s.

CI

Split into build / integration / leaks. The previous workflow installed CppUTest and ran ./unittests — a target that no longer exists in CMakeLists.txt, so that step could never pass. Those steps are removed here; tests/loader_test.cpp is still stale and unreferenced, and is left for a follow-up.

Fixes are deliberately not in this PR.

🤖 Generated with Claude Code

jeeyoand others added 4 commits September 4, 2026 21:31
The 3rdparty/quickjs submodule pointed at github.com/jeeyo/quickjs, which no
longer resolves, so the default build could not be checked out at all. Move to
quickjs-ng v0.16.2, the maintained fork.
API changes this required:
- JSMallocFunctions gained js_calloc and now passes an opaque void* rather
than a JSMallocState*. The runtime does its own allocation accounting and
limit enforcement, so the shim no longer duplicates it -- it just routes
allocation through the FreeRTOS heap.
- libbf.c and cutils.c are gone; dtoa.c replaces them in the source list.
- CONFIG_BIGNUM and EMSCRIPTEN are no longer meaningful defines here.
- JS_IsError() takes a single argument, and JS_BOOL is now plain bool.
js_def_malloc_usable_size() still returns a hardcoded 0, which is why
JS_SetMemoryLimit() cannot currently bound a handler. Left as-is and marked
with a TODO rather than changed here, to keep this commit to the migration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Runs the real isere binary and talks to it over HTTP, so behaviour that only
shows up in a running server is observable before we start changing code.
tests/integration/
harness.py server lifecycle, handler compilation, RSS/fd sampling
handlers/*.js the JavaScript each test deploys
test_smoke.py startup, routing, malformed input, abrupt disconnects
test_concurrency.py latency distribution, concurrent clients, fd leaks
test_memory.py RSS stability across requests and connect/disconnect
test_risky_js.py hostile handler code
test_leaks.py valgrind, marked slow
The harness compiles each test's .js into handler.so using the same xxd/sed
recipe as the root CMakeLists, so what is tested matches a normal build. Each
server gets its own scratch directory and LD_LIBRARY_PATH, because loader.c
dlopen()s handler.so by relative name.
Leak detection compares memory retained at two different request counts rather
than looking at a single run. Most of what isere leaks stays *reachable* -- a
leaked FreeRTOS timer block is still on a kernel list -- so it never appears as
"definitely lost", and RSS is too coarse to see it. Growth against request
count is the signal that works.
Four tests fail against this branch. They are findings, not broken tests:
- test_pipelined_requests_in_one_segment: two requests in one TCP segment
abort the process in JS_FreeRuntime with `list_empty(&rt->gc_obj_list)'.
__on_message_complete frees the connection from inside an llhttp callback
while llhttp still holds a pointer to it.
- test_embedded_nul_in_body_does_not_leak_memory: a body containing a NUL is
sent with a length taken from the JS string but a buffer copied by strdup(),
so the response contains adjacent heap -- observed heap pointers on the wire.
- test_slow_handler_blocks_the_event_loop: one handler awaiting a timer blocks
an unrelated client for ~6s.
- test_no_leak_with_timers_that_fire: under valgrind's thread serialisation the
drain loop busy-waits without yielding and starves the timer task, so the
server stops accepting entirely.
CI is split into build / integration / leaks. The previous workflow installed
CppUTest and ran ./unittests, a target that no longer exists in CMakeLists, so
that step could never pass; those steps are removed here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pass/fail list alone did not explain what any test was for. Every case now
carries metadata via a `case` marker -- stable id, title, area, severity, what a
failure would mean, and the source lines it exercises -- with the docstring as
the description.
That metadata goes to two places:
- JUnit XML, as <properties> on each <testcase>, so the standard artifact is
self-describing for anything that reads it directly. This needs
junit_family=xunit1; xunit2 drops properties.
- catalog.json, written at collection time before -m/-k deselection, so it
covers the whole suite rather than just what a given run executed.
report.py merges the two into a standalone HTML file: summary counts, failures
listed up front ordered by severity, then every test grouped by area with its
description, rationale, refs and captured output. Tests the run did not execute
appear as "not run" with their documentation intact, so the report describes
the suite whether or not it ran in full. --junit is repeatable, so the fast and
slow runs merge into one report.
Captured output is included (junit_logging=all) since the latency and soak
numbers the tests print are the interesting part of a passing result, not just
the green tick. pytest's empty capture banners are stripped out.
CI gains a `report` job that runs on always(), merges both runs and uploads the
HTML as the test-report-html artifact.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Byte-compiled files were picked up when tests/integration was first added.
Removed from the index and ignored, along with .pytest_cache.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jeeyo

jeeyo commented Sep 4, 2026

Copy link
Copy Markdown
OwnerAuthor

Reporting added

The pass/fail list didn't explain what any test was for, so every case now carries structured metadata and there's an HTML report built on top.

Metadata. Each test has a case marker — stable id (RISK-11, MEM-04…), title, area, severity, what a failure would mean, and the source lines it exercises — with the docstring as the description.

JUnit XML stays the standard artifact, and each <testcase> now carries that metadata as <properties>, so it's self-describing for anything reading it directly. This needs junit_family=xunit1; xunit2 silently drops properties.

HTML report (tests/integration/report.py) merges JUnit results with a catalogue written at collection time — before-m/-k deselection. So it documents all 44 tests whether or not a given run executed them; skipped ones show as "not run" with their documentation intact. Failures are listed up front ordered by severity, then everything is grouped by area.

Captured output is included, because for these tests the numbers are the result, not the green tick:

CONC-01 Serial latency baseline PASSED 0.10s
One client, no concurrency -- the floor for per-request latency.
If this fails: Per-request latency regressed badly, or the server is
doing something blocking on the happy path.
Exercises: src/httpd.c:423
[serial] ok=50 fail=0 wall=0.02s throughput=2622.1 req/s
p50=0.4ms p95=0.5ms max=0.8ms

--junit is repeatable, so the fast and slow runs merge into a single report. A new report CI job runs on always() and uploads it as the test-report-html artifact.

Full run

44 tests — 40 passed, 4 failed. Failures by severity:

CaseWhat it means
criticalRISK-02Response body with an embedded NUL discloses adjacent heap — live pointers observed on the wire
criticalRISK-11Two pipelined requests abort the process in JS_FreeRuntime
highCONC-06One slow handler blocks unrelated clients for ~6s
mediumLEAK-04Fails, but not for the leak it targets — under valgrind's thread serialisation the drain loop busy-waits without yielding and starves the timer task. Passes without valgrind, so the finding is the missing yield.

Areas: Smoke 11 · Concurrency 6 · Memory 8 · Hostile handler code 12 · Leaks 7.

Also dropped a stray __pycache__ that got committed with the first push; the net branch diff is clean.

🤖 Generated with Claude Code

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@jeeyo