Skip to content

vm: stop leaking heap VAL_NULL from json_decode'd dicts - #213

Merged
InauguralPhysicist merged 1 commit into
mainfrom
fix/heap-null-leak
Jun 17, 2026
Merged

vm: stop leaking heap VAL_NULL from json_decode'd dicts#213
InauguralPhysicist merged 1 commit into
mainfrom
fix/heap-null-leak

Conversation

@InauguralPhysicist

Copy link
Copy Markdown
Collaborator

Summary

Two interacting bugs leaked a 128-byte heap VAL_NULL per source dict the first time any null-valued field was accessed:

  1. promote_if_arena's VAL_NULL case xcalloc'd a new heap VAL_NULL for the global singleton — ignoring that the singleton (`arena=1`, `refcount=1000000`) is already safe to store anywhere.
  2. `slot_bridge_wrap` converted VAL_NULL pointers to an immediate-null slot without decref'ing the owned ref the caller passed in. `DOT_GET` incref'd the dict's heap_null and pushed it; the push silently dropped the +1 ref.

Either fix alone closes the leak; both ship for defense in depth. `slot_from_value` already enforces the same ownership contract — `slot_bridge_wrap` was just missing it.

Minimal repro

```eigs
bools is json_decode of "{"n": null}"
x is bools.n # this access triggers the leak
```

Without the access: no leak (refcounts balance at exit teardown). With any access via DOT_GET, the heap_null gets +1 from `val_incref` and the +1 is stranded by `slot_bridge_wrap`'s pointer drop.

Test plan

  • `make asan && cd tests && ASAN_OPTIONS=detect_leaks=1 bash run_all_tests.sh` — 1882/1882, leak tally 13 -> 11
  • `make && cd tests && bash run_all_tests.sh` — 1882/1882
  • Standalone leakers: 17 -> 15 (retires `test_json_hard.eigs`, `test_ui.eigs`; `test_coverage_v2.eigs` and `test_store.eigs` each lose one heap_null too)
  • Minimal repro verified leak-clean

🤖 Generated with Claude Code

…pped null refs

Two interacting bugs leaked a 128-byte heap VAL_NULL per source dict
the first time any null-valued field was accessed:

1. promote_if_arena's VAL_NULL case xcalloc'd a new heap VAL_NULL
   for the global singleton, ignoring that the singleton (arena=1,
   refcount=1000000) is already safe to store anywhere.
2. slot_bridge_wrap converted VAL_NULL pointers to an immediate-null
   slot without decref'ing the owned ref the caller passed in. DOT_GET
   incref'd the dict's heap_null and pushed it; the push silently
   dropped the +1 ref to /dev/null.

Fix both: promote_if_arena now returns the singleton unchanged, and
slot_bridge_wrap val_decref's the dropped null pointer (no-op on the
singleton, releases any heap VAL_NULL that slipped past the source
fix). Defense in depth — either fix alone closes the test_json_hard
leak, but the slot_bridge_wrap one is the real ownership contract
slot_from_value already enforces.

Retires test_json_hard.eigs and test_ui.eigs from the standalone
leakers list (17 -> 15); harness tally 13 -> 11. test_coverage_v2
and test_store each lose one heap_null too. Suite: 1882/1882 release
+ ASan.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
@InauguralPhysicist
InauguralPhysicist merged commit ec9dee6 into main Jun 17, 2026
11 checks passed
@InauguralPhysicist
InauguralPhysicist deleted the fix/heap-null-leak branch June 17, 2026 07:26
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