fix: prevent elname SV leak when Start/End handlers die#260
Merged
Conversation
When a Start or End handler throws an exception via die/croak, the call_sv longjmp skips the cleanup code after it: av_push(context) in startElement and SvREFCNT_dec(elname) in endElement. The element name SV leaks on every handler exception. Fix: register elname with SAVEFREESV inside the ENTER/LEAVE scope so Perl's scope cleanup frees it on exception unwind. On normal exit, SvREFCNT_inc before LEAVE keeps it alive for the post-scope cleanup. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #260 +/- ##
==========================================
- Coverage 75.73% 75.63% -0.10%
==========================================
Files 1 1
Lines 1092 1096 +4
Branches 342 344 +2
==========================================
+ Hits 827 829 +2
Misses 59 59
- Partials 206 208 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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
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.
What
Fix memory leak of element name SVs when Start or End handlers throw exceptions.
Why
When a Start handler
dies,call_svlongjmps pastav_push(cbv->context, elname)— the element name SV (refcount 1) is never stored or freed. Similarly in endElement,SvREFCNT_dec(elname)is skipped. Every handler exception leaks one SV. This adds up in applications that parse untrusted XML and use handlers that validate/reject content by dying.How
Use
SAVEFREESV(elname)inside theENTER/LEAVEscope that wraps thecall_sv. This registers the SV for automatic cleanup if the scope unwinds via exception. On the normal (non-exception) path,SvREFCNT_inc_simple_void(elname)beforeLEAVEpreserves the SV for the post-scope operations (av_pushin startElement,SvREFCNT_decin endElement).Audited all 19 XS callbacks — startElement and endElement are the only ones with non-mortal SVs across a
call_svboundary. All other callbacks properly usesv_2mortalbefore pushing SVs.Testing
t/handler_die.twith 6 tests: Start/End die propagation, parser reuse after exception, and stress test (10 consecutive exception cycles)🤖 Generated with Claude Code
Quality Report
Changes: 2 files changed, 91 insertions(+), 1 deletion(-)
Code scan: clean
Tests: passed (OK)
Branch hygiene: clean
Generated by Kōan post-mission quality pipeline