From c0b3406b5403540c8b94dacba0cea8c6ce04b988 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 10 Oct 2026 16:36:37 +0000 Subject: [PATCH] fix(reflection): skip closures over internal functions when walking the live closures A swap walks the live closure objects to find the copies of its entry (fake closures share the entry's run-time cache and borrow its live statics). The walk built a user-function view over every closure and asked it whether it was user-defined - but a closure over an INTERNAL function (a first-class callable of strlen(), Closure::fromCallable() over an internal function, the closure of an internal method) keeps its name in the internal_function member, and building the view threw "Attempt to read undefined field 'function_name'" before the question could be asked. Every redefine() that released a minted cache, took over live statics or retired an alias failed while such a closure was alive; PHP 8.6 keeps such closures in its callable caches, so there it failed always. The function type is now read off the union before any view is made, and internal closures are skipped. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2 --- src/Reflection/FunctionBodySwap.php | 14 ++-- .../RedefineLiveInternalClosureTest.php | 67 +++++++++++++++++++ tests/Stub/Alias/InternalClosureCounter.php | 29 ++++++++ 3 files changed, 105 insertions(+), 5 deletions(-) create mode 100644 tests/Reflection/RedefineLiveInternalClosureTest.php create mode 100644 tests/Stub/Alias/InternalClosureCounter.php 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; + } +}