Skip to content

Handle GC cycles properly in intl iterators #17355

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions ext/intl/breakiterator/breakiterator_iterators.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ typedef struct zoi_break_iter_parts {
static void _breakiterator_parts_destroy_it(zend_object_iterator *iter)
{
zval_ptr_dtor(&iter->data);
efree(iter);
}

static void _breakiterator_parts_get_current_key(zend_object_iterator *iter, zval *key)
Expand Down Expand Up @@ -223,7 +222,7 @@ static const zend_object_iterator_funcs breakiterator_parts_it_funcs = {
_breakiterator_parts_move_forward,
_breakiterator_parts_rewind,
zoi_with_current_invalidate_current,
NULL, /* get_gc */
zoi_with_current_get_gc,
};

void IntlIterator_from_BreakIterator_parts(zval *break_iter_zv,
Expand Down
43 changes: 39 additions & 4 deletions ext/intl/common/common_enum.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ void zoi_with_current_dtor(zend_object_iterator *iter)
{
zoi_with_current *zoiwc = (zoi_with_current*)iter;
zval_ptr_dtor(&zoiwc->wrapping_obj);
ZVAL_UNDEF(&zoiwc->wrapping_obj);
}

U_CFUNC int zoi_with_current_valid(zend_object_iterator *iter)
Expand Down Expand Up @@ -80,6 +81,14 @@ static void string_enum_current_move_forward(zend_object_iterator *iter)
} //else we've reached the end of the enum, nothing more is required
}

HashTable *zoi_with_current_get_gc(zend_object_iterator *iter, zval **table, int *n)
{
zoi_with_current *zoiwc = reinterpret_cast<zoi_with_current*>(iter);
*table = &zoiwc->wrapping_obj;
*n = 1;
return nullptr;
}

static void string_enum_rewind(zend_object_iterator *iter)
{
zoi_with_current *zoi_iter = (zoi_with_current*)iter;
Expand All @@ -106,7 +115,6 @@ static void string_enum_rewind(zend_object_iterator *iter)
static void string_enum_destroy_it(zend_object_iterator *iter)
{
delete (StringEnumeration*)Z_PTR(iter->data);
efree(iter);
}

static const zend_object_iterator_funcs string_enum_object_iterator_funcs = {
Expand All @@ -117,7 +125,7 @@ static const zend_object_iterator_funcs string_enum_object_iterator_funcs = {
string_enum_current_move_forward,
string_enum_rewind,
zoi_with_current_invalidate_current,
NULL, /* get_gc */
zoi_with_current_get_gc,
};

U_CFUNC void IntlIterator_from_StringEnumeration(StringEnumeration *se, zval *object)
Expand All @@ -135,18 +143,43 @@ U_CFUNC void IntlIterator_from_StringEnumeration(StringEnumeration *se, zval *ob
ZVAL_UNDEF(&((zoi_with_current*)ii->iterator)->current);
}

static void IntlIterator_objects_free(zend_object *object)
static void IntlIterator_objects_dtor(zend_object *object)
{
IntlIterator_object *ii = php_intl_iterator_fetch_object(object);

if (ii->iterator) {
((zoi_with_current*)ii->iterator)->destroy_it(ii->iterator);
OBJ_RELEASE(&ii->iterator->std);
ii->iterator = NULL;
}
}

static void IntlIterator_objects_free(zend_object *object)
{
IntlIterator_object *ii = php_intl_iterator_fetch_object(object);

intl_error_reset(INTLITERATOR_ERROR_P(ii));

zend_object_std_dtor(&ii->zo);
}

static HashTable *IntlIterator_object_get_gc(zend_object *obj, zval **table, int *n)
{
IntlIterator_object *ii = php_intl_iterator_fetch_object(obj);
if (ii->iterator) {
zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
zend_get_gc_buffer_add_obj(gc_buffer, &ii->iterator->std);
zend_get_gc_buffer_use(gc_buffer, table, n);
} else {
*table = nullptr;
*n = 0;
}
if (obj->properties == nullptr && obj->ce->default_properties_count == 0) {
return nullptr;
} else {
return zend_std_get_properties(obj);
}
}

static zend_object_iterator *IntlIterator_get_iterator(
zend_class_entry *ce, zval *object, int by_ref)
{
Expand Down Expand Up @@ -273,7 +306,9 @@ U_CFUNC void intl_register_common_symbols(int module_number)
sizeof IntlIterator_handlers);
IntlIterator_handlers.offset = XtOffsetOf(IntlIterator_object, zo);
IntlIterator_handlers.clone_obj = NULL;
IntlIterator_handlers.dtor_obj = IntlIterator_objects_dtor;
IntlIterator_handlers.free_obj = IntlIterator_objects_free;
IntlIterator_handlers.get_gc = IntlIterator_object_get_gc;

register_common_symbols(module_number);
}
1 change: 1 addition & 0 deletions ext/intl/common/common_enum.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,7 @@ U_CFUNC void zoi_with_current_dtor(zend_object_iterator *iter);
U_CFUNC int zoi_with_current_valid(zend_object_iterator *iter);
U_CFUNC zval *zoi_with_current_get_current_data(zend_object_iterator *iter);
U_CFUNC void zoi_with_current_invalidate_current(zend_object_iterator *iter);
U_CFUNC HashTable *zoi_with_current_get_gc(zend_object_iterator *iter, zval **table, int *n);

#ifdef __cplusplus
using icu::StringEnumeration;
Expand Down
19 changes: 19 additions & 0 deletions ext/intl/tests/IntlIterator_cycle_management.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
--TEST--
IntlIterator cycle management
--EXTENSIONS--
intl
--FILE--
<?php
function break_cycle_test() {
$obj = IntlCalendar::getKeywordValuesForLocale('calendar', 'fa_IR', true);
unset($obj);
var_dump(gc_collect_cycles());
}
break_cycle_test();
break_cycle_test();
break_cycle_test();
?>
--EXPECT--
int(1)
int(1)
int(1)
Loading