Skip to content

fix: release cloned mg_obj refcount after sv_magic#137

Draft
Koan-Bot wants to merge 2 commits into
garu:masterfrom
Koan-Bot:koan.atoomic/fix-mgobj-refcount-leak
Draft

fix: release cloned mg_obj refcount after sv_magic#137
Koan-Bot wants to merge 2 commits into
garu:masterfrom
Koan-Bot:koan.atoomic/fix-mgobj-refcount-leak

Conversation

@Koan-Bot

@Koan-Bot Koan-Bot commented May 4, 2026

Copy link
Copy Markdown
Contributor

What

Fix a refcount leak on the cloned mg_obj in the magic cloning loop.

Why

sv_magic() takes its own SvREFCNT_inc on the obj parameter, but the
caller's reference returned by sv_clone() was never released. Each clone()
call on a tied variable leaked one refcount per magic entry — the cloned tie
object's DESTROY never fired.

This affects tied hashes, arrays, scalars, and any SV whose magic chain contains
a recursively-cloned mg_obj.

How

After sv_magic(clone, obj, ...), call SvREFCNT_dec(obj) when obj was
produced by sv_clone() (tracked via a per-iteration obj_cloned flag).
Shared mg_obj references (e.g. PERL_MAGIC_qr) are not affected — only
the cloned path needs the decrement.

Testing

  • New t/28-tied-mgobj-leak.t with 5 tests covering tied hashes, arrays, and scalars
  • DESTROY tracking confirms the leak existed (0 DESTROY calls before fix)
  • Weakref-based detection verifies no surviving orphan after clone destruction
  • Refcount assertion validates the expected count (magic + lexical = 2)
  • Full test suite passes (29 programs, 367 subtests)

🤖 Generated with Claude Code


Quality Report

Changes: 3 files changed, 131 insertions(+)

Code scan: clean

Tests: passed (OK)

Branch hygiene: clean

Generated by Kōan post-mission quality pipeline

Koan-Bot and others added 2 commits May 4, 2026 07:46
sv_magic() takes its own SvREFCNT_inc on the obj parameter, but the
caller's reference returned by sv_clone() was never released.  This
leaked one refcount per magic entry per clone call — the cloned tie
object's DESTROY never fired.

Affects any SV with magic where mg_obj is recursively cloned: tied
hashes, arrays, scalars, and other non-skipped magic types.

Add t/28-tied-mgobj-leak.t with DESTROY tracking and weakref-based
leak detection for tied hashes, arrays, and scalars.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
install-with-cpm@v1 regressed on Perl 5.8-5.22 after a recent cpm
update requiring Perl 5.24+.  Bump to @v2 for action-based jobs
and use direct `cpm install` in the Docker-based Linux matrix
(where cpm is pre-installed).

Matches the approach approved by @atoomic in PR garu#120.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
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.

1 participant