-
Notifications
You must be signed in to change notification settings - Fork 7.8k
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
RecursiveCallbackFilterIterator regression in 8.1.18 #11972
Labels
Comments
This regressed in 49b2ff5 @Girgias @NathanFreeman |
Oh god, not this sort of issue again ;-; I will have a look if @NathanFreeman is not available. |
Can you produce a simpler, reproducible? I really cannot comprehend what is going on in that code due to the size of the array and I have never done anything with Drupal. Especially as the recursion code seems to work as expected with a simple case such as: <?php
$arr1 = [];
$arr2 = [$arr1];
$arr1[] = $arr2;
$std1 = new stdClass();
$std2 = new stdClass();
$std1->prop1 = $std2;
$std1->prop2 = $arr1;
$std2->prop2A = $arr2;
$std2->prop2B = $std1;
$arr1[] = $std1;
$arr2[] = $std1;
$test_class = new RecursiveFilterTest();
$test_class->traverse($arr1); |
nielsdos
added a commit
to nielsdos/php-src
that referenced
this issue
Aug 30, 2023
When you do an assignment between two zvals (no, not zval*), you copy all fields. This includes the additional u2 data. So that means for example the Z_NEXT index gets copied, which in some cases can therefore cause a cycle in zend_hash lookups. Instead of doing an assignment, we should be doing a ZVAL_COPY (or ZVAL_COPY_VALUE for non-refcounting cases). This avoids copying u2.
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.1: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.2: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.3: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
When you do an assignment between two zvals (no, not zval*), you copy all fields. This includes the additional u2 data. So that means for example the Z_NEXT index gets copied, which in some cases can therefore cause a cycle in zend_hash lookups. Instead of doing an assignment, we should be doing a ZVAL_COPY (or ZVAL_COPY_VALUE for non-refcounting cases). This avoids copying u2. Closes GH-12086.
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.1: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.2: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
nielsdos
added a commit
that referenced
this issue
Aug 30, 2023
* PHP-8.3: Fix GH-11972: RecursiveCallbackFilterIterator regression in 8.1.18
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Description
I think I've found a regression that was introduced in php-8.1.18. The following test case completes in 8.1.17, but hangs silently in 8.1.18 or any greater version. The use case is that I'm going through a really large render array in Drupal and leaving bread crumbs so that I can later find them in a
RecursiveCallbackFilterIterator
to break out of infinite loops.The following simplified test case:
Resulted in:
But I expected this output instead:
The reason we think it's a PHP bug is because it seems to hang on
isset($current['#override_mode_breadcrumb'])
. Similar things like empty cause it to hang too.PHP Version
8.1.18
Operating System
No response
The text was updated successfully, but these errors were encountered: