fix(mir): release select-arm owned bindings on scope exit#2668
Open
slepp wants to merge 5 commits into
Open
Conversation
A select arm's value binding (reply from an actor ask, an item from rx.recv(), and the checker-gated stream/task forms) entered binding_locals but never register_owned_local, so it never reached owned_locals and build_lifo_drops released it on no exit edge — one heap value leaked per selected iteration (32 bytes per owned string reply in the #1875 repro). Register the binding at the shared body-block site that already emits Bind + record_binding_scope for every value-bearing arm kind, making the select binding a first-class owner with the same scope-exit drop discipline as a let-bound local. ValueOwnership::classify filters BitCopy types to no-op drops, the move-out analysis suppresses the drop when the arm body moves the value out, and per #1869's contract the reply channel reaps only non-consumed legs, so the scope-exit drop is the sole reaper on the win path. AfterTimer arms bind nothing and stay unregistered. The fix sits upstream of the blocking/ suspending terminator split, so Terminator::Select and SuspendingSelect inherit the drop identically. Fixes the select-arm half of #1875.
Registering the select-arm binding as owned exposed two gaps on the
selected-value-escape shape (let x = select { r from a.m() => r }):
- The arm body's move of the binding into the select result was not
recognised as a consume, so the binding and the destination aliased
one heap pointer as two owned locals and the drop provers failed
closed to a leak on both. lower_select now marks the binding moved
when the arm body's value is the binding's own slot, transferring
the single drop obligation to the destination.
- The destination never earned a drop: the cow fresh-owner derivation
only seeds fresh string producers from runtime-ABI calls, and a
select binding slot is written by the winning edge, not by any MIR
producer instruction. derive_cow_fresh_borrowed_owner now seeds
string-typed select/suspending-select arm binding slots as fresh
+1 owners (the reply channel hands over the consumed leg; try_recv
pops without clone or drop), so the escaped value's let-binding is
admitted for its one balancing scope-exit drop.
Verified: the escape shape drops from 1 leak/iteration to zero and
runs clean under the poisoned allocator; the unused-binding and
timeout-wins shapes are unchanged.
Compiled leak-slope coverage for the select-arm half of #1875, on the shared leak_slope harness (LOW/HIGH node-count delta, macOS leaks(1), graceful skip elsewhere): - headline unused owned ask-reply binding (flat slope; ~1 leak/iter before the fix), with a channel-recv Option<string> mirror; - timeout-wins shape: an after arm beating owned ask/recv losers (losers reaped by the reply-channel destructor, unwritten binding slots not dropped); - selected-value escape as both a slope and a poisoned-allocator pin (the escape move suppresses the arm binding's drop; the value is read back verbatim and released exactly once); - select-loser race and straight-line unused-binding scribble pins (no double-free against the channel destructor's loser-leg reap). The recv mirror sends a static literal deliberately: send_layout deep-copies into the envelope, and a heap send argument would contaminate the slope with the pre-existing send-side temp leak, which reproduces without select and is a separate seam.
The registration comment implied the win-edge-only release follows from the Bind's scope placement. The real invariant is the dataflow's: the binding is Live only in its own body block, the join-entry meet demotes it to absent (Uninit meets anything to Uninit), and the scope-close forward-Goto pass releases it on the winning body-block-to-join edge — the last point it is provably the live sole owner. Loser arms' slots are never Live, so no drop can fire on them. Comment-only; no behaviour change.
The shared registration site covers all four value-bearing select arm kinds, but only ActorAsk and ChannelRecv compile to native for the leak oracle. Pin the rest at the MIR layer: - TaskAwait (t from await actor.method()) passes the checker, lowers to a real SelectArmKind::TaskAwait arm, and registers its binding through the shared site — asserted here, with a discriminator guard so the coverage cannot silently degrade to ActorAsk. Codegen fails closed with E_NOT_YET_IMPLEMENTED on the task-await winner edge, so no compiled oracle can exist yet; the arm-setup's await_ty also falls back to Unit for this form (documented in the test), both to be resolved when the codegen surface lands. - StreamNext is not source-reachable at all (the checker rejects next(<stream>) and stream.recv() arm sources), so it cannot reach MIR from source and stays covered by construction. The task-winner slot-drop vs hew_task_free runtime seam and the stream-loser cancel leg get their compiled oracles when those surfaces land.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Select-arm bindings (
ActorAsk,StreamNext,TaskAwait,ChannelRecv) enteredbinding_localswithout ownership registration, so every heap value selected into an arm binding leaked — 32 bytes per iteration in the reproducer. Bindings are now registered as owned locals at the shared body-block site inlower_select, so the winning arm's value is dropped on scope exit. Escaping bindings (let x = select { r from a.m() => r }) are released exactly once: the pass-through is move-marked and the destination is seeded as a fresh owner, so the value transfers instead of double-freeing. Drops are win-edge-only — loser and timeout legs stay with the reply-channel destructor per the single-reaper contract, proven by poisoned-allocator scribble pins.This fixes the select-arm half of #1875. The match-arm scrutinee half was superseded by #2462, and the per-spawn leak reported there was not reproducible on current main.
Verification
select_arm_owned_binding_leak_oracle: 7/7 pass (4 leaks(1)-slope oracles + 3 malloc-scribble double-free pins), 3 consecutive runs; with the fix reverted, the headline slope oracle fails at exactly 1 leak/iteration, confirming teethselect_arm_owned_registration): 3/3, 3 consecutive runs — including registration pins forTaskAwait(codegen currently fails closed NYI) andStreamNext(checker-rejected today), so the MIR contract holds when those paths open upcargo test -p hew-mir: greencargo clippy -D warnings,cargo fmt --check: cleanmake test-hew-ratchet: 0 regressionsask_reply_owned_leak_oracle2/2,match_call_scrutinee_leak_oracle9/9Out of scope
recv-escape drop admission:msg from rx.recv() => msgbindsOption<string>, not a leafstring, so fresh-owner seeding does not admit the escaped destination — that shape still leaks (fail-closed, never double-free). It needs Option/composite fresh-owner admission, which is a separate piece of work.hew_channel_send_layoutdeep-copies while MIR markssendconsuming, so the original is never freed) reproduces without select and is tracked separately.lower_joinwere deliberately not touched.Fixes #1875