diff --git a/changelog.d/6979-temp-root-cut-invariant.md b/changelog.d/6979-temp-root-cut-invariant.md new file mode 100644 index 0000000000..d916e31472 --- /dev/null +++ b/changelog.d/6979-temp-root-cut-invariant.md @@ -0,0 +1,5 @@ +Documented why the deferred `this`-patch closure roots in object-literal lowering +need no explicit release: `temp_root` truncation is a stack cut, so the enclosing +scope's single truncate already releases every root pushed after it. A #6972 +review finding read the missing per-root release as a leak; it is not, and the +absence of a comment saying so is what made it look like one. diff --git a/crates/perry-codegen/src/expr/object_literal.rs b/crates/perry-codegen/src/expr/object_literal.rs index 29ea88afb4..5aaf35d26a 100644 --- a/crates/perry-codegen/src/expr/object_literal.rs +++ b/crates/perry-codegen/src/expr/object_literal.rs @@ -496,6 +496,14 @@ pub(crate) fn lower_object_literal( let this_idx = auto_caps.len() as u32; let v = lower_expr(ctx, value_expr)?; + // No explicit release for these: `js_gc_temp_root_truncate` is a + // stack CUT, not a pop, and `rooted` was pushed before the loop — + // so the single `rooted_handle_release` at the end of this function + // drops the object handle AND every closure root above it. That + // depends on the push order, so keep `rooted_handle_begin` ahead of + // this loop and the release after the patch loop. + // (`gc::tests::temp_roots::truncate_drops_every_slot_above_the_base` + // pins the cut semantics.) let closure_root = protect_handle.then(|| temp_root_push_double(ctx, &v)); this_patches.push((closure_root, v.clone(), this_idx));