From 4d627f92bcac38a8e288b9ef5d9362aab1c5c463 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 09:26:25 +0200 Subject: [PATCH 1/2] docs(gc): note why the this-patch closure roots need no explicit release MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit read `rooted_handle_release` as a pop and flagged the `this_patches` closure roots as leaked. They are not: `js_gc_temp_root_truncate` is a stack CUT, and `rooted` is pushed before the property loop, so the single release at the end of the function drops the object handle and every closure root above it. That does depend on the push order, which was not written down — say so, and point at the unit test that pins the cut semantics. --- crates/perry-codegen/src/expr/object_literal.rs | 8 ++++++++ 1 file changed, 8 insertions(+) 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)); From 92f1e85d4d1dbba192e6a4b603de784924e682b5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ralph=20K=C3=BCpper?= Date: Wed, 29 Jul 2026 11:12:19 +0200 Subject: [PATCH 2/2] changelog: fragment for #6979 --- changelog.d/6979-temp-root-cut-invariant.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog.d/6979-temp-root-cut-invariant.md 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.