Skip to content

fix(builtins): release boxed string elements consumed by implode over a mixed array#613

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/601-implode-mixed-release
Open

fix(builtins): release boxed string elements consumed by implode over a mixed array#613
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/601-implode-mixed-release

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Summary

implode() over an indexed array held in a boxed mixed cell leaked one heap
block per string element (issue #601). Stdout was correct but the heap grew with
the string-element count.

Root cause

In __rt_implode's Mixed-element loop each boxed element is stringified through
__rt_mixed_cast_string. Its string-tag path calls __rt_str_persist, which
allocates a fresh owned heap block (kind 1) detached from the source mixed owner.
implode copied those bytes into the shared concat buffer but never released the
persisted copy — one leaked 48-byte block per string element. Integer, float, and
bool elements stringify into shared scratch storage (__rt_itoa/__rt_ftoa) with
no allocation, so they never leaked.

Fix

The __rt_implode Mixed-element loop now records the cast string pointer in a
dedicated frame slot and releases it with __rt_heap_free once its bytes have been
copied, on both the ARM64 and x86_64 paths:

  • Borrowed typed-array string slots clear the slot each iteration, so nothing is
    freed (they are owned by the array).
  • Integer/float/bool casts return shared scratch storage; __rt_heap_free's
    foreign-pointer guard (live-heap range check) makes the release a safe no-op for
    them.
  • The release runs strictly after the copy, so there is no use-after-free; the
    joined output lives in the concat buffer and is never freed.

The x86_64 frame grows from 64 to 80 bytes for the extra spill slot (balanced in
prologue/epilogue); ARM64 preserves its loop registers around the release call.

Tests

heap-debug regression tests in tests/codegen/runtime_gc/regressions.rs: the exact
repro, a multi-string mixed literal, an int-in-mixed no-op-release case, a
20-iteration loop proving the leak no longer scales, an empty-separator join, and a
typed-array control that must stay untouched. Plus emitter unit tests asserting the
release path and balanced frame on both targets. Focused slices (runtime_gc 225,
implode/join 14, strings 212, mixed 285, arrays 386) pass with no
regressions. Linux x86_64 end-to-end coverage relies on CI (no local
cross-assembler).

Fixes #601

https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L

@github-actions github-actions Bot added area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:s Small pull request. type:fix Corrects broken or incompatible behavior. labels Jul 23, 2026
@mirchaemanuel

Copy link
Copy Markdown
Contributor Author

Adjacent pre-existing bug found while validating this fix (reverified on the pre-fix build — independent of this change): #614, implode() over a mixed array corrupts the output when a float element follows a string element (["x", 1.5]1.1.1 instead of x,1.5); suspected __rt_ftoa clobbering the shared concat buffer mid-join.

… a mixed array

implode() over an indexed array held in a boxed `mixed` cell leaked one heap
block per string element (issue illegalstudio#601). Each boxed element is stringified through
`__rt_mixed_cast_string`, whose string-tag path calls `__rt_str_persist` to
allocate an owned copy detached from the source mixed owner. implode copied the
bytes into the concat buffer but never released that copy.

The `__rt_implode` mixed-element loop now records the cast string pointer in a
dedicated frame slot and releases it with `__rt_heap_free` after its bytes are
copied, on both the ARM64 and x86_64 paths. Borrowed typed-array string slots
clear the slot so nothing is freed, and integer/float/bool elements (whose casts
return shared scratch storage) pass through `__rt_heap_free`'s foreign-pointer
guard as a safe no-op. The joined output lives in the concat buffer and is never
freed, so it stays valid.

Adds heap-debug regression tests for the exact repro, a multi-string mixed
literal, an int-in-mixed no-op-release case, a 20-iteration loop proving the
leak no longer scales, an empty-separator join, and a typed-array control that
must stay untouched, plus emitter unit tests asserting the release path and
balanced frame on both targets.

Claude-Session: https://claude.ai/code/session_01HRvbcjPHa3kAojdCjNZL5L
@mirchaemanuel
mirchaemanuel force-pushed the fix/601-implode-mixed-release branch from 61de0b9 to fe1145e Compare July 24, 2026 22:30
@mirchaemanuel

Copy link
Copy Markdown
Contributor Author

Rebased onto current main (0fd01c162) so this merges without a merge commit, per your standing preference on contributor branches that go stale.

The only conflict was CHANGELOG.md, and it was purely additive: main had added the two CLI entries (--timings/--quiet spinner work and the -h/--help overhaul) while this branch added the implode() leak entry. All three are kept, in that order, at the end of [Unreleased]. No code conflicted — the implode.rs and regression-test changes replayed untouched.

Verified on the rebased head fe1145e5c (macOS aarch64): cargo build clean, and the implode filter is 13 passed / 0 failed, which covers the 6 new #601 regressions plus the pre-existing implode coverage including the #540 property-argument release test.

Nothing else changed. The branch is now a single commit on top of main.

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

Labels

area:runtime Touches runtime helpers, GC, ownership, or bridge runtimes. size:s Small pull request. type:fix Corrects broken or incompatible behavior.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

implode() over a boxed-mixed indexed array leaks one block per element appended through the Mixed path

1 participant