Skip to content

Commit 3960def

Browse files
committed
Fixed bug #61482, caused by the fix to bug #61418.
Turns out I'd forgotten to also update the destructor for the iterator returned by DirectoryIterator. The iterator for DirectoryIterator maintains the same ->current pointer throughout its existence (the DirectoryIterator itself) and returns it (the same object) everytime a value is requested from the iterator. Moving forward the iterator only changes the object. Previous code added two references to the object in get_iterator on the account of 1) the iterator memory living in its DirectoryIterator object and 2) the object being stored in iterator->current. This seems to be unnecessary. Iterators are not responsible for incrementing the refcount of the values they yield, that's up to the caller (the engine). What matters for the iterator is that the object exists as long as the iterator exists and this can be guaranteed by incremented the refcount only once. Consequently, I only add one reference in get_iterator (and reclaim it in the iterator destructor).
1 parent 8572533 commit 3960def

File tree

1 file changed

+8
-5
lines changed

1 file changed

+8
-5
lines changed

ext/spl/spl_directory.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1655,13 +1655,16 @@ zend_object_iterator *spl_filesystem_dir_get_iterator(zend_class_entry *ce, zval
16551655
static void spl_filesystem_dir_it_dtor(zend_object_iterator *iter TSRMLS_DC)
16561656
{
16571657
spl_filesystem_iterator *iterator = (spl_filesystem_iterator *)iter;
1658-
zval *zfree = (zval*)iterator->intern.data;
16591658

1660-
iterator->intern.data = NULL; /* mark as unused */
1661-
zval_ptr_dtor(&iterator->current);
1662-
if (zfree) {
1663-
zval_ptr_dtor(&zfree);
1659+
if (iterator->intern.data) {
1660+
zval *object = iterator->intern.data;
1661+
zval_ptr_dtor(&object);
16641662
}
1663+
/* Otherwise we were called from the owning object free storage handler as
1664+
* it sets
1665+
* iterator->intern.data to NULL.
1666+
* We don't even need to destroy iterator->current as we didn't add a
1667+
* reference to it in move_forward or get_iterator */
16651668
}
16661669
/* }}} */
16671670

0 commit comments

Comments
 (0)