Skip to content

fix(ir): release the producer's reference when boxing an owned value as Mixed (#484)#489

Open
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/484-mixed-box-producer-release
Open

fix(ir): release the producer's reference when boxing an owned value as Mixed (#484)#489
mirchaemanuel wants to merge 1 commit into
illegalstudio:mainfrom
mirchaemanuel:fix/484-mixed-box-producer-release

Conversation

@mirchaemanuel

Copy link
Copy Markdown
Contributor

Summary

Fixes #484. __rt_mixed_from_value retains refcounted payloads (objects, arrays, hashes, callables, nested cells) and persists strings, so a boxed Mixed cell always carries its own reference or copy — but EIR lowering treated Op::MixedBox as consuming its operand and never released the producer's reference. Boxing an owning temporary therefore left the payload one reference high forever:

function g(): ?P { return new P(); }   // leaked one P per call

The boxed cell itself was freed correctly on rebind; only the payload leaked (fresh arrays boxed the same way leaked identically).

Fix

A new LoweringContext::box_value_as_mixed helper emits the MixedBox and then releases the operand when it is an owning temporary — the same ownership rule as the throw-of-borrowed fix in #477. The 11 value-carrying emission sites now route through it; the two remaining direct sites box freshly-created nulls (non-refcounted, commented accordingly).

Deliberately unchanged:

Verification

  • Regression tests (object + array flavours) in tests/codegen/runtime_gc/regressions.rs; broad suites green (1872 codegen across callables/closures/runtime_gc/control/types/exceptions/arrays/oop/generators + 993 error tests).
  • Adversarially reviewed across ~28 fixtures: no double free, no use-after-release (all converted sites' callers shadow the operand — audited one by one), IR shows exactly one release immediately after the box for owned producers and none for borrows. Closure-capture and invoker-hash shapes improve as a side effect.

Found during review (pre-existing, filed separately)

…as Mixed

Fixes illegalstudio#484. __rt_mixed_from_value retains refcounted payloads (objects, arrays,
hashes, callables, nested cells) and persists strings, so the boxed cell always
carries its own reference or copy — but EIR lowering treated Op::MixedBox as
consuming its operand and never released the producer's reference. Boxing an
owning temporary therefore left the payload one reference high forever:
'function g(): ?P { return new P(); }' leaked one P per call (and fresh arrays
boxed on return leaked identically) even though the boxed cell itself was freed
on rebind.

A new LoweringContext::box_value_as_mixed helper emits the MixedBox and then
releases the operand when it is an owning temporary (same ownership pattern as
the throw-of-borrowed fix in illegalstudio#477); the 11 value-carrying emission sites now go
through it. Borrowed operands (a boxed local) are untouched — the box's retain
is their +1 — and concat-scratch strings remain exempt from heap release, so
string boxing behavior is unchanged (its separate persisted-copy leak is issue
illegalstudio#485). The two remaining direct emission sites box freshly-created nulls
(non-refcounted) and need no release.

Regression tests in tests/codegen/runtime_gc/regressions.rs for the object and
array flavours. Broad suites green (1872 codegen across callables/closures/
runtime_gc/control/types/exceptions/arrays/oop/generators + 993 error tests).
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.

mixed_box of an owned object leaks the producer's reference (?Class returns leak one object per call)

1 participant