[php-src] Issue #8079: Recent patch to SplFixedArray causes array_walk + json encode to hang

Issue: https://github.com/php/php-src/issues/8079 Comment Author: TysonAndre 1. More context: The problems we'd see with SplFixedArray and var_export are the problems we'd see in general with cyclic data structures in future additions to php. Mostly, I want to either have var_export working the way users would expect, or have a good explanation to include for the lack of var_export support, for RFCs such as https://wiki.php.net/rfc/deque before I'd started voting. I unexpectedly ran into this when writing test cases for https://github.com/TysonAndre/pecl-teds/ . General-purpose data structures would face the same problem as SplFixedArray is having, and fixing it for SplFixedArray would allow for fixing it for PECLs or future additions to php. Changing var_export was the only way I could think of that was safe to do that - I'd assume any changes to var_export would target 8.2, though 2. I managed to create a test case that's more realistic that still affects the latest 8.2.0 commit c77bbcd4642a982fd9fdaf32dbe444d925c7883f 3. > We have infinity recursion because of recursive data and incompatibility of recursion protection mechanism with SplFixedArray. This was why I'd initially asked about changing the recursion protection mechanism used (I'd believed it was impossible) - first checking for infinite recursion on the object before checking for infinite recursion on the get_properties_for result in var_export (and to better understand why it was done that way, if you'd remembered). You have much more experience than I do with internals, so I thought there may have been something I'd overlooked or an alternative approach > So I'm concerned that if json_encode and so on are called (e.g. due to __destruct, even in > normal applications not deliberately trying to trigger this), the calls to Z_OBJPROP_P will now leak > HashTable instances due to this change, and possibly prematurely free the original. ```php <?php class VarExportInDestruct { public $self; public function __construct(public SplFixedArray $parent) { $this->self = $this; } public function __destruct() { echo "In __destruct during cyclic garbage collection, e.g. var_export in debugging output\n"; var_export($this->parent); // same for var_export($this); echo "\n"; } } call_user_func(function () { $x = new SplFixedArray(2); $x[0] = $x; $x[1] = $x; new VarExportInDestruct($x); echo "Before gc_collect_cycles\n"; gc_collect_cycles(); echo "After gc_collect_cycles\n"; }); ``` Actual output with latest patch set: This outputs thousands of var_export warnings and runs out of memory - I added printf statements to spl_fixedarray_object_get_properties to debug ``` Warning: var_export does not handle circular references in %s on line 9 spl_fixedarray_object_get_properties 0x7f9a702ba480 spl_fixedarray_object_get_properties 0x7f9a702ba480 Duplicating properties table Warning: var_export does not handle circular references in %s on line 9 spl_fixedarray_object_get_properties 0x7f9a702ba4e0 spl_fixedarray_object_get_properties 0x7f9a702ba4e0 Duplicating properties table Warning: var_export does not handle circular references in %s on line 9 spl_fixedarray_object_get_properties 0x7f9a702ba540 spl_fixedarray_object_get_properties 0x7f9a702ba540 Duplicating properties table Warning: var_export does not handle circular references in %s on line 9 spl_fixedarray_object_get_properties 0x7f9a702ba5a0 ``` Expected output (e.g. php 8.2 before the patch) ``` Before gc_collect_cycles In __destruct during cyclic garbage collection, e.g. var_export in debugging output Warning: var_export does not handle circular references in %s on line 9 Warning: var_export does not handle circular references in %s on line 9 SplFixedArray::__set_state(array( 0 => NULL, 1 => NULL, )) After gc_collect_cycles ```