Fix leak when iterating IntlBreakIterator parts iterators - #23464
Open
iliaal wants to merge 1 commit into
Open
Conversation
devnexen
requested changes
Aug 26, 2026
|
|
||
| ((zoi_with_current*)ii->iterator)->destroy_it = _breakiterator_parts_destroy_it; | ||
| ZVAL_OBJ_COPY(&((zoi_with_current*)ii->iterator)->wrapping_obj, Z_OBJ_P(object)); | ||
| ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->wrapping_obj); |
Member
There was a problem hiding this comment.
the teardown in zoi_with_current_dtor() needs to move into the iterator's own dtor, otherwise _breakiterator_parts_destroy_it() releases the BreakIterator mid iteration and zoi_bit->bio dangles. Iterating a temporary parts iterator segfaults on the second element today, and a test doing that (iterating a call result directly, not a variable) should come with it.
Contributor
Author
There was a problem hiding this comment.
Moved destroy_it into the iterator dtor. Test iterates a call result.
| --TEST-- | ||
| IntlPartsIterator must not retain the current element after destruction | ||
| --SKIPIF-- | ||
| <?php if (!extension_loaded('intl')) die('skip intl extension not available'); ?> |
Member
There was a problem hiding this comment.
this is old fashion tests, new tests should not do this.
getPartsIterator() leaked because wrapping_obj was a counted self-reference, so the iterator never reached destruction and the current element was retained. wrapping_obj stays UNDEF; current and the backing BreakIterator are released from the iterator dtor, not the IntlIterator object dtor, so iterating a temporary parts iterator does not dangle. The string enumeration iterator still self-references because move_forward/rewind need the owner. Closes phpGH-23464
iliaal
force-pushed
the
fix/intl-enum-current-leak-84
branch
from
August 26, 2026 11:46
be7ba61 to
1c43e15
Compare
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
2 participants
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.
Iterating IntlBreakIterator::getPartsIterator() leaked memory on every loop because the IntlPartsIterator held a counted self-reference through its embedded zend_object_iterator's wrapping_obj, so refcount destruction could never complete, and the retained current element was additionally never released at iterator destruction. zoi_with_current_dtor() now invalidates the current element and the parts iterator leaves wrapping_obj UNDEF as the plain BreakIterator iterator already does, making teardown deterministic; the string enumeration iterator keeps its self-reference because move_forward and rewind need the owner for error handling.