From 72498a4a134933fd5f693c0f3440612bec210f8b Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Thu, 17 Feb 2022 10:35:51 +0100 Subject: [PATCH 1/2] Fix GH-7958: Nested CallbackFilterIterator is leaking memory We implement `zend_object_iterator_funcs.get_gc` for user iterators to avoid the memory leak. --- Zend/tests/gh7958.phpt | 43 ++++++++++++++++++++++++++++++++++++++++++ Zend/zend_interfaces.c | 13 ++++++++++++- Zend/zend_interfaces.h | 1 + 3 files changed, 56 insertions(+), 1 deletion(-) create mode 100644 Zend/tests/gh7958.phpt diff --git a/Zend/tests/gh7958.phpt b/Zend/tests/gh7958.phpt new file mode 100644 index 0000000000000..d9f3b8940a4a4 --- /dev/null +++ b/Zend/tests/gh7958.phpt @@ -0,0 +1,43 @@ +--TEST-- +GH-7958 (Nested CallbackFilterIterator is leaking memory) +--FILE-- +iterator = new ArrayIterator($data); + echo '-- c ' . spl_object_id($this) . "\n"; + } + + public function __destruct() + { + echo '-- d ' . spl_object_id($this) . "\n"; + } + + public function filter() + { + $this->iterator = new \CallbackFilterIterator($this->iterator, fn() => true); + $this->iterator->rewind(); + } +} + +$action = new Action(['a', 'b']); +$action->filter(); +$action->filter(); +print_r(iterator_to_array($action->iterator)); +$action = null; +gc_collect_cycles(); +echo "==DONE==\n"; +?> +--EXPECT-- +-- c 1 +Array +( + [0] => a + [1] => b +) +-- d 1 +==DONE== diff --git a/Zend/zend_interfaces.c b/Zend/zend_interfaces.c index 54a2d61c38c29..eecc808556739 100644 --- a/Zend/zend_interfaces.c +++ b/Zend/zend_interfaces.c @@ -182,6 +182,17 @@ ZEND_API void zend_user_it_rewind(zend_object_iterator *_iter) } /* }}} */ +ZEND_API HashTable *zend_user_it_get_gc(zend_object_iterator *_iter, zval **table, int *n) +{ + zend_user_iterator *iter = (zend_user_iterator*)_iter; + zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); + if (!Z_ISUNDEF(iter->it.data)) { + zend_get_gc_buffer_add_zval(gc_buffer, &iter->it.data); + } + zend_get_gc_buffer_use(gc_buffer, table, n); + return NULL; +} + static const zend_object_iterator_funcs zend_interface_iterator_funcs_iterator = { zend_user_it_dtor, zend_user_it_valid, @@ -190,7 +201,7 @@ static const zend_object_iterator_funcs zend_interface_iterator_funcs_iterator = zend_user_it_move_forward, zend_user_it_rewind, zend_user_it_invalidate_current, - NULL, /* get_gc */ + zend_user_it_get_gc, }; /* {{{ zend_user_it_get_iterator */ diff --git a/Zend/zend_interfaces.h b/Zend/zend_interfaces.h index 2799e2c438154..a8351ee9a7823 100644 --- a/Zend/zend_interfaces.h +++ b/Zend/zend_interfaces.h @@ -55,6 +55,7 @@ ZEND_API void zend_user_it_get_current_key(zend_object_iterator *_iter, zval *ke ZEND_API zval *zend_user_it_get_current_data(zend_object_iterator *_iter); ZEND_API void zend_user_it_move_forward(zend_object_iterator *_iter); ZEND_API void zend_user_it_invalidate_current(zend_object_iterator *_iter); +ZEND_API HashTable *zend_user_it_get_gc(zend_object_iterator *_iter, zval **table, int *n); ZEND_API void zend_user_it_new_iterator(zend_class_entry *ce, zval *object, zval *iterator); ZEND_API zend_object_iterator *zend_user_it_get_new_iterator(zend_class_entry *ce, zval *object, int by_ref); From 33d27ff52e15168d76f08da7b0b145962f0a722c Mon Sep 17 00:00:00 2001 From: "Christoph M. Becker" Date: Fri, 18 Feb 2022 13:21:22 +0100 Subject: [PATCH 2/2] No need to create a GC buffer for a single zval --- Zend/zend_interfaces.c | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/Zend/zend_interfaces.c b/Zend/zend_interfaces.c index eecc808556739..b032d1d4e0b4c 100644 --- a/Zend/zend_interfaces.c +++ b/Zend/zend_interfaces.c @@ -185,11 +185,8 @@ ZEND_API void zend_user_it_rewind(zend_object_iterator *_iter) ZEND_API HashTable *zend_user_it_get_gc(zend_object_iterator *_iter, zval **table, int *n) { zend_user_iterator *iter = (zend_user_iterator*)_iter; - zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create(); - if (!Z_ISUNDEF(iter->it.data)) { - zend_get_gc_buffer_add_zval(gc_buffer, &iter->it.data); - } - zend_get_gc_buffer_use(gc_buffer, table, n); + *table = &iter->it.data; + *n = 1; return NULL; }