diff --git a/src/Reflection/FunctionBodySwap.php b/src/Reflection/FunctionBodySwap.php index c3b8e45..51f7e9e 100644 --- a/src/Reflection/FunctionBodySwap.php +++ b/src/Reflection/FunctionBodySwap.php @@ -860,7 +860,8 @@ private static function isDefaultsTableShared( * walk over the object store * * Only user-function closures are of interest (an internal function's closure carries - * no op_array), so the list is narrowed to those. The views point into the closure + * no op_array, and no user-function view can be built over it), so the list is narrowed + * to those before any view is made. The views point into the closure * objects: the list describes the live closures only until user code runs again (a * destructor fired by a table release may create or drop closures), so the callers take * every decision that reads it BEFORE they destroy anything. @@ -876,11 +877,14 @@ private static function liveClosureFunctions(): array } $liveClosures = []; foreach (Core::$executor->objectStore->instancesOf(self::$closureClassAddress) as $closureObject) { - $closure = Core::cast(zend_closure::class, $closureObject->getRawValue()); - $closureFunction = ReflectionFunction::fromCData(Core::addr($closure->func)); - if ($closureFunction->isUserDefined()) { - $liveClosures[] = $closureFunction; + $closure = Core::cast(zend_closure::class, $closureObject->getRawValue()); + // The type is read off the union before any view is built over it: a closure + // over an internal function (strlen(...), Closure::fromCallable('strlen')) keeps + // its name in the internal_function member, where a zend_function view has none + if (($closure->func->type & Core::ZEND_USER_FUNCTION) === 0) { + continue; } + $liveClosures[] = ReflectionFunction::fromCData(Core::addr($closure->func)); } return $liveClosures; diff --git a/tests/Reflection/RedefineLiveInternalClosureTest.php b/tests/Reflection/RedefineLiveInternalClosureTest.php new file mode 100644 index 0000000..0457a78 --- /dev/null +++ b/tests/Reflection/RedefineLiveInternalClosureTest.php @@ -0,0 +1,67 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + * + */ +declare(strict_types=1); + +namespace ZEngine\Reflection; + +use PHPUnit\Framework\TestCase; +use ZEngine\Stub\Alias\InternalClosureCounter; + +/** + * A swap walks the live closures to find the copies of the entry (fake closures over it + * share its run-time cache and borrow its live statics). Closures over INTERNAL functions + * are live objects too - a first-class callable of strlen(), Closure::fromCallable() over + * an internal function, the closure of an internal method - and carry no user op_array: the + * walk must skip them before building a user-function view, which has no name field to + * read on an internal entry (it threw "Attempt to read undefined field 'function_name'" + * and failed every redefine() while such a closure was alive; PHP 8.6 keeps such closures + * in its callable caches, so there it failed always). + */ +final class RedefineLiveInternalClosureTest extends TestCase +{ + public function testSwapsRemovalsAndStaticsReleaseIgnoreClosuresOverInternalFunctions(): void + { + $firstClass = strlen(...); + $callable = \Closure::fromCallable('strtoupper'); + $method = (new \ReflectionMethod(\ArrayObject::class, 'count'))->getClosure(new \ArrayObject([1, 2])); + + $entry = (new ReflectionClass(InternalClosureCounter::class))->getMethod('counter'); + $entry->redefine(function (): int { + /** @var int $n */ + static $n = 10; + + return ++$n; + }); + // Instantiated after the first swap: under the opcache-active runner the class is + // copied out of shared memory by it, and instances made before keep the shared class + $instance = new InternalClosureCounter(); + self::assertSame(11, $instance->counter(), 'the live statics table of the swapped body materializes'); + + // Every release path that walks the live closures: the minted cache of the previous + // body (second swap), the live statics the preserved alias takes over, and what + // removeMethods() retires + $entry->redefine(function (): int { + /** @var int $n */ + static $n = 20; + + return ++$n; + }, preserveAs: 'counterPrevious'); + self::assertSame(21, $instance->counter()); + self::assertSame(22, $instance->counter()); + (new ReflectionClass(InternalClosureCounter::class))->removeMethods('counterPrevious'); + self::assertSame(23, $instance->counter()); + + self::assertSame(3, $firstClass('abc'), 'the closures over internal functions are untouched'); + self::assertSame('ABC', $callable('abc')); + self::assertSame(2, $method()); + } +} diff --git a/tests/Stub/Alias/InternalClosureCounter.php b/tests/Stub/Alias/InternalClosureCounter.php new file mode 100644 index 0000000..da48902 --- /dev/null +++ b/tests/Stub/Alias/InternalClosureCounter.php @@ -0,0 +1,29 @@ + + * + * This source file is subject to the license that is bundled + * with this source code in the file LICENSE. + * + */ +declare(strict_types=1); + +namespace ZEngine\Stub\Alias; + +/** + * Method with a static variable redefined while closures over internal functions are alive + * (RedefineLiveInternalClosureTest) + */ +class InternalClosureCounter +{ + public function counter(): int + { + /** @var int $n */ + static $n = 0; + + return ++$n; + } +}