Skip to content

vm: heap-force builtin-arg wrapper to plug arena-window ref leak - #212

Merged
InauguralPhysicist merged 1 commit into
mainfrom
fix/arena-builtin-arg-leak
Jun 17, 2026
Merged

vm: heap-force builtin-arg wrapper to plug arena-window ref leak#212
InauguralPhysicist merged 1 commit into
mainfrom
fix/arena-builtin-arg-leak

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Summary

  • Multi-arg builtin calls inside an arena_mark/arena_reset window leaked one ref per heap argument.
  • The CALL path packs args into a wrapper make_list(argc); with g_arena.active=1 that wrapper was arena-allocated. list_append incref'd heap items (e.g. weights, grad), but the matching val_decref(arg) is a no-op on arena values — arena reclaim doesn't walk children, so the items' incref was permanently stranded.
  • Fix: new make_list_heap() constructor that forces heap regardless of arena state; used by the two builtin-arg wrapper sites (jit_helper_call, CASE(CALL)). val_decref(arg) now actually releases the items.

Why this matters

Minimal repro (no loop, no allocation pressure):

weights is [1.0, 2.0, 3.0, 4.0]
grad is [0.01, 0.01, 0.01, 0.01]
arena_mark of null
sgd_update of [weights, grad, 1.0]
arena_reset of null

Before: 1408 bytes leaked in 12 allocations (weights + grad lists + 10 inner items + items arrays). After: clean.

The bug generalizes: any multi-arg builtin (str of [a, b], contains of [list, x], etc.) called inside an arena window leaks one ref per heap arg position. SGD training loops accumulate one phantom ref per arg per step, so weights/grad globals never reach refcount 0 at exit.

Test plan

  • make asan && cd tests && ASAN_OPTIONS=detect_leaks=1 bash run_all_tests.sh — 1882/1882, leak tally 13 (unchanged ceiling)
  • make && cd tests && bash run_all_tests.sh — 1882/1882
  • Standalone leak audit: 19 → 17 leakers (retires test_arena_ownership.eigs and test_optimize.eigs)
  • Minimal repro verified leak-clean

🤖 Generated with Claude Code

Multi-arg builtin calls inside an arena_mark/arena_reset window leaked
one ref per heap argument. The CALL path packs args into a wrapper
make_list(argc); with g_arena.active=1 that wrapper was arena-allocated,
list_append incref'd heap items (e.g. weights/grad), but the matching
val_decref(arg) is a no-op on arena values — arena reclaim doesn't walk
children, so the items' incref was permanently stranded.

Add make_list_heap() that forces heap regardless of arena state; use it
for the two builtin-arg wrapper sites (jit_helper_call, CASE(CALL)).
val_decref(arg) now actually releases items as designed.

Retires test_arena_ownership.eigs and test_optimize.eigs from the
standalone-leakers list (19 -> 17). Suite: 1882/1882 release + ASan.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@InauguralPhysicist
InauguralPhysicist merged commit 789d522 into main Jun 17, 2026
11 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the fix/arena-builtin-arg-leak branch June 17, 2026 07:03
InauguralPhysicist added a commit that referenced this pull request Jun 17, 2026
- CHANGELOG [Unreleased]: document #220 behavior change (assert
  failures route through g_has_error and are catchable by try/catch;
  exit code + stderr unchanged for uncaught failures).
- docs/BUILTINS.md: assert row no longer says "Exit with message";
  now describes the catchable runtime error.
- CLAUDE.md: harness leak tally reflects the actual current count
  (10, down from 13 after PRs #212/#213/#214/#220) and the suite
  size is bumped to ~1882 to match.

Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
InauguralPhysicist added a commit that referenced this pull request Jun 18, 2026
These memory-safety fixes (arena-window builtin-arg ref leak, heap-promoted
VAL_NULL, eval partial-AST on parse error) shipped in v0.16.0 but had no
CHANGELOG entries; record them under [0.16.0]. Drops the ASan harness leak
tally 13 -> 10. CHANGELOG-only; no code change.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
InauguralPhysicist added a commit that referenced this pull request Jun 19, 2026
… recovery

Patch release on top of 0.16.0. Highlights:
- Remote HTTP DoS fixed (Content-Length SEGV; missing _GNU_SOURCE → 64-bit
  pointer truncation) and the implicit-declaration class hard-errored in the
  build (#239/#240).
- ASan leak tally driven 10 → 4 (spawn-thread floor) across the runtime,
  HTTP code routes, container builtins, and the store extension
  (#234/#235/#236/#233).
- OSR/JIT coverage for observed-assign loops recovered: #211's conservative
  scanner bail replaced by the real fix — reset the OP_POP last_imm peephole
  at forward-jump targets — so those loops OSR-compile again (~34%, #231).
- Docs: clarified the implicit-`n` parameter shadows an outer `n` (#241,
  not a bug).

Also repairs CHANGELOG drift: commit 8fe0731 had accidentally deleted the
`## [0.16.0] — 2026-06-18` header, folding released 0.16.0 content back into
[Unreleased]. The header is restored (with its summary), the windowed-predicate
+ #212/#213/#214 entries are reattributed to 0.16.0 (their code shipped in the
v0.16.0 tag), and everything post-tag is grouped under the new [0.16.1].

VERSION 0.16.0 → 0.16.1. Release 1943/1943; ASan 1943/1943, leak tally 4.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Sign up for free to 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