Skip to content

feat(reflection): preserve the previous body of redefine() under an alias and accept method donors - #282

Merged
lisachenko merged 14 commits into
8.5from
feature/redefine-preserve-alias
Oct 10, 2026
Merged

lisachenko merged 14 commits into
8.5from
feature/redefine-preserve-alias

Conversation

@lisachenko

@lisachenko lisachenko commented Oct 10, 2026 •

Copy link
Copy Markdown
Owner

What this changes

Runtime method weaving needs two things from the engine that redefine() did not offer: swapping a method body for one compiled elsewhere while the original body stays callable, and overriding an inherited method in a class that does not declare it. This PR adds both on the existing redefine() surface instead of a new interceptMethod() API.

  • redefine(\Closure|FunctionLikeInterface $newBody, ?string $preserveAs = null): a user method of another class (the donor) is adopted as the new body in place, exactly like a closure body. The entry keeps its identity (name, scope, prototype, visibility, static/final flags) while the compiled body, signature, attributes and doc comment follow the donor. The donor's signature must be compatible with the entry's one (self/parent resolved against each side's own scope).
  • preserveAs: the previous body is published before the swap as a private method of the declaring class (marked ZEND_ACC_CHANGED when an ancestor declares a private method of that name, shared into every subclass linked before, taking over the live static variables of the body it preserves) or, for a function, as a second global function. It stays callable and can be swapped back in with a later redefine().
  • ReflectionClass::addMethod() accepts a method donor too and publishes an override of an inherited method with the engine's inheritance rules: prototype kept, no override of a final method, no visibility narrowing, same static-ness, parent-private redeclaration marked CHANGED (and the bit propagated from a CHANGED inherited method). The magic slots of the class and the method tables of subclasses linked before follow the override.
  • FunctionBodySwap allocates the fresh run-time cache of a swapped or published entry from request memory without ZEND_ACC_HEAP_RT_CACHE and releases it on the next swap of the entry, after detaching the engine copies that still point at it (see below). A heap-flagged cache shared by a parent and a subclass method table was efree()d once per table at a destructor-path shutdown (USE_ZEND_ALLOC=0), once for every subclass linked after a swap.
  • New RedefinitionException (named factories) and SharedMemoryException::sharedEntryHasLinkedSubclasses(): a class whose entry lives in opcache shared memory and already has linked subclasses is refused, since the subclass tables point at the shared entry.
  • docs/hot-swap.md documents method donors, preserveAs, the addMethod() override, the run-time cache rule, the engine copies a swap must respect and the cost of a swap. README.md links the section.

Known limitation (documented): a private or final method whose body is a literal may be inlined by the opcache optimizer into its callers, so a swap of it is invisible at those call sites. On PHP 8.5 a closure donor's captured (use) variables stay with the closure object and are not carried into the entry, as before this PR.

The PHP 8.4 port is #284 (8.4, the minimum affected branch; its merge-up into 8.5 conflicts in FunctionBodySwap/PendingBodySwap because 8.5 has no statics duplication, and this PR is the resolution). The goaop/framework z-engine driver (goaop/framework#765) consumes this API on both minors.

Fixes from the adversarial review

An independent review of the feature found nine defects, each reproduced on the pre-fix tree (segfault, zend_mm_heap corrupted, wrong dispatch); the six that exist on 8.5 are fixed in 5d3f079/9e97ad5 (the three others concern the statics duplicates PHP 8.4 mints, see #284):

  • Minted run-time cache freed under its copies. The engine copies a swapped entry, cache pointer included, into every fake closure made over it (Closure::fromCallable(), first-class callable, ReflectionMethod::getClosure()) and into the interface-induced clone a subclass gets of an inherited entry; the next swap freed the block under them (use-after-free). Before a block is released its copies are found: fake closures get a private heap copy flagged ZEND_ACC_HEAP_RT_CACHE, clones get a fresh minted cache, a frame still running on the block keeps it allocated.
  • preserveAs orphaned the materialized live statics table. The alias now takes over the live table of the body it preserves (the preserved body keeps its state), and the entry starts afresh.
  • removeMethods() of a preserved alias leaked its live statics and minted cache. FunctionBodySwap::retireUnpublishedEntry() releases what the engine never will once the bucket is deleted from the only table holding it.
  • Alias not shared into subclasses linked before that override the method. The swapped body's $this->alias() reached through parent:: failed with "Call to undefined method"; the alias is now published into every subclass linked before (an inheriting subclass receives the pointer with inherited-bucket ownership, a redeclaring subclass gets ZEND_ACC_CHANGED on its own method), and a shared-memory entry with any linked subclass is refused.
  • publishOverride() cleared ZEND_ACC_CHANGED. The engine propagates the bit from a CHANGED (non-private) inherited method; without it an ancestor's private method was bypassed from the ancestor's own scope.
  • Interface-induced engine clones invisible to the swap. A subclass implementing an interface that declares an inherited method holds a struct copy of the entry (do_inheritance_check_on_method), not the pointer: it kept the previous body after a redefine(), missed the alias, was skipped by an addMethod() override on an intermediate class and let a shared-memory ancestor be copied out from under it. Clones now follow the swap in place, carry the alias, are reached by overrides and guarded like pointer holders; immutability is judged by the holder class, not by the body-level ZEND_ACC_IMMUTABLE an entry adopts from an opcache-cached donor.

Each defect has a regression test group of its own (tests/Reflection/Redefine*Test.php, AddMethodChangedFlagTest.php, EntryPublicationsTest.php, stubs under tests/Stub/Alias/), and tests/HotSwap/scripts/redefine-alias-matrix.php runs five destructor-path scenarios under both allocators and both opcache modes.

Performance

A swap used to walk the whole class table for every question it asked about its entry and the object store once per check, building a reflection wrapper per visited class: about 190 ms per redefine() in a process holding 1,800 classes. EntryPublications (beb5d51) takes one survey per redefinition through an integer view of the class table (HashTable::pointerValueAddresses(), the technique ObjectStore already uses) and answers every question of the entry swap, the clone swaps and their commits from memory; a commit gathers the live closures once, before anything is destroyed. MethodPublisher reaches the linked subclasses through ReflectionClass::getLinkedSubclasses(), the same walk with a parent-chain check.

1,800-class process (PHP 8.5.11) before after
redefine(), 1,500 subclasses sharing the entry ~190 ms 5 ms
redefine(), unrelated classes - 2 ms

Environment it was verified on

  • PHP version (full first line of php -v): PHP 8.5.11 (cli) (built: Sep 24 2026 13:49:29) (NTS)
  • Thread safety: NTS
  • OS / architecture: Linux x86_64
  • Debug build (--enable-debug)? no

Verified with opcache on and off, and with the JIT disabled (opcache.jit=off, opcache.jit_buffer_size=0): the tracing JIT on PHP 8.5 has a known upstream bug. The new tests pass in both runner modes (also under USE_ZEND_ALLOC=0), composer test, composer phpstan (PHPStan 2.3.1) and composer cs:check are green, and the goaop/framework zengine driver group passes against this head. Two failures of the local opcache-active runner mode (ClassSpecializerSlotTest segfault, FunctionLikeInfoTest type errors) reproduce on origin/8.5 without this PR and are not in CI's job.

Checklist

  • Targets the minimum affected version branch (master = newest
    supported minor, 8.4 = PHP 8.4, …) — fixes cascade upward, never
    downward (targets 8.5; the 8.4 port is feat(reflection): preserve the previous body of redefine() under an alias and accept method donors (PHP 8.4) #284, see above)
  • composer test passes on the matching PHP minor
  • composer phpstan (level max) and composer cs:check are green
  • Tests added or updated; structs the code dereferences are listed in
    layout_structs in tools/generator/symbols.php (no new structs are dereferenced)
  • If tools/generator/symbols.php changed: regenerated with
    composer gen-headers and committed the result — nothing under
    include/, stubs/ or .phpstorm.meta.php was hand-edited (unchanged)
  • Conventional Commits used for the commit messages

🤖 Generated with Claude Code

https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2


Generated by Claude Code

…lias and accept method donors

redefine() now takes `\Closure|FunctionLikeInterface $newBody` and an optional
`?string $preserveAs`:

- a user method of another class (the donor) can be adopted as the new body in
  place, exactly like a closure body; the entry keeps its identity (name, scope,
  prototype, visibility, static/final flags) while the compiled body, signature,
  attributes and doc comment follow the donor;
- with `preserveAs` the previous body is published before the swap as a private
  method of the declaring class (ZEND_ACC_CHANGED when an ancestor declares a
  private method of that name, shared into subclasses that were linked before)
  or as a second global function, so it stays callable and can be swapped back
  in with a later redefine().

ReflectionClass::addMethod() accepts a method donor as well and publishes an
override of an inherited method with the engine's inheritance rules (prototype
kept, no final override, no visibility narrowing, same static-ness, parent-private
redeclaration marked CHANGED); the class's magic slots and the method tables of
subclasses linked before follow the override.

FunctionBodySwap allocates the fresh run-time cache of a swapped or published
entry from request memory WITHOUT ZEND_ACC_HEAP_RT_CACHE and releases it on the
next swap of the entry: a heap-flagged cache shared by a parent and a subclass
method table was efree()d once per table at a destructor-path shutdown
(USE_ZEND_ALLOC=0), i.e. for every subclass linked after a swap.

New RedefinitionException (named factories) and
SharedMemoryException::sharedEntryHasLinkedSubclasses(); docs/hot-swap.md
documents method donors, preserveAs, the addMethod() override and the run-time
cache rule.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
lisachenko and others added 4 commits October 10, 2026 01:20
…odySwap header

The class header still described the fresh run-time cache as heap-flagged
and freed by the engine; since the previous commit it is request memory
without ZEND_ACC_HEAP_RT_CACHE that z-engine releases itself.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…onor signature check

PHP 8.4 reflection reports the self and parent keywords of a type
verbatim, PHP 8.5 resolves them to the class names: comparing the
stringified types made a method donor spelling the target class fail
the signature check on 8.4 while it passed on 8.5. The signature now
renders each side with the keywords resolved against its own scope
(the declaring class, or the bound scope of a closure donor), which is
what the types accept at run time on both versions; static stays as is.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
PHPStan 2.3 folds more constants through loops and casts: the nested
iteration count of the get-iterator scenario and the property snapshot
of the resolution-handlers test read as constant on the first iteration,
which made their guards look always-true and the loops look endless, and
an ArrayObject built without a type reads as holding nothing.

- the scenario compares the nested iteration count with the item count
  of the iterated instance instead of a literal;
- the resolution-handlers test asserts the snapshot with assertSame()
  instead of a guard ending the loop with fail();
- the error-callback test declares the element type of its log.

No behaviour of the tests changes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
(cherry picked from commit 8fc1949)
…ading it

Completes the previous commit for PHPStan 2.3: ArrayObject::offsetGet() may
return null, so the entry is asserted before its keys are read.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2

Copy link
Copy Markdown
Owner Author

CI: PHPStan (level max) was red on every head of this PR, and it is not this PR's. The five findings are in tests/Memory/scenarios/get-iterator-hook.php, tests/Reflection/ReflectionClassResolutionHandlersTest.php and tests/System/Hook/ErrorCallbackHookTest.php, which this PR does not touch; they appear on origin/8.4 and origin/8.5 as well since PHPStan 2.3.1 (the lock file is not committed, so CI resolves the newest release). The fix is #283 against 8.4 (the minimum affected branch), and the same commit is cherry-picked into this PR (3843b85, with b807faa completing the ArrayObject typing), so the check is green here without waiting for the merge-up.

Two more notes for the review of this PR:

  • 91a170c resolves self/parent against each side's own scope in the donor signature check. PHP 8.4 reflection reports the keywords verbatim while 8.5 resolves them, so without it a method donor spelling the target class failed the check on 8.4 only.
  • Unrelated to this PR: on a PHP 8.4.19 build (NTS, release) ZEngine\System\ExecutionDataTest::testGetThis corrupts the heap (zend_mm_heap corrupted, corrupted double-linked list under USE_ZEND_ALLOC=0) on origin/8.4 without any of these changes; the 8.4 port of this feature is feature/redefine-preserve-alias-8.4 and was verified with that one test excluded.

Generated by Claude Code

lisachenko and others added 9 commits October 10, 2026 04:49
…ase what the swap owns

An in-place body swap and the publications built on it (the preserveAs alias,
the addMethod() override) copied the entry structure where the engine also
copies it, and freed or shared what those copies still used:

- the run-time cache minted for an entry was freed by its next swap while a
  fake closure made over the entry (Closure::fromCallable, a first-class
  callable, ReflectionMethod::getClosure) or the engine clone a subclass
  implementing an interface holds still read it (use-after-free). The release
  now finds those copies first: a live frame on the block keeps it, a fake
  closure gets a private heap copy the engine frees with it, a clone gets a
  fresh minted cache (ObjectStore::instancesOf walks EG(objects_store) without
  letting its own handles alias the slots it reads);
- the materialized live statics of a preserved body were orphaned: the alias
  now takes them over (the preserved body keeps its state), a swap destroys
  the previous body's table unless a fake closure or a clone borrowed it, and
  removeMethods() releases the live statics and the minted cache of an entry
  it unpublishes from the only table holding it;
- the alias reached only subclasses sharing the entry pointer: every subclass
  linked before receives it now (a subclass declaring the name itself is
  marked ZEND_ACC_CHANGED), the shared-memory guard refuses any linked
  subclass, and an override carries ZEND_ACC_CHANGED whenever the inherited
  method does, as the engine propagates it;
- engine clones of an inherited entry (subclass implementing an interface that
  declares the method) follow a redefinition in place and are replaced by an
  override like pointer-sharing buckets.

Port of the 8.4 fix (ed9548b on feature/redefine-preserve-alias-8.4) without
its 8.4-only statics-ownership parts: on 8.5 no donor kind owns the defaults
table, so nothing is minted or duplicated for it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…s, clones and live frames

Fake closures of every kind made over a redefined method or function keep
executing the previous body on a private heap copy of the block the next swap
retires, an interface clone of a subclass linked after a swap follows the next
swap with a minted cache of its own, a fake closure redefining its own method
mid-flight keeps the block it runs on, a hundred swaps next to a live fake
closure stay memory-flat, and a fake closure over a preserved alias survives a
redefinition of the alias.

The mid-flight scenario keeps its state in static properties of the test class:
on PHP 8.5 a closure donor's bound variables stay with the closure object, so
the entry adopting the body would read them unbound.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…lass

An alias reaches the subclasses linked before that override the redefined
method (the preserved body runs on their instances through parent::), a
subclass declaring a private method of the alias name keeps it and gets
ZEND_ACC_CHANGED so the base scope still resolves the base alias, the
shared-memory guard refuses both shapes when the base is immutable, and
subclasses linked after inherit the alias the engine's way - with one body
share per publishing table.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…nd removeMethods() releases

The alias of a method or function continues on the very live statics table
the preserved body had materialized while the new body starts afresh,
removing the alias destroys that table and releases the minted run-time cache
the bucket destructor cannot reach, publish/call/restore/remove cycles stay
within the container baseline, a table a fake closure borrows survives both a
swap and the removal, and an alias still published by a subclass table keeps
its statics until the last table drops it.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…y flags

A method entry that adopted a body from an opcache-cached donor carries
ZEND_ACC_IMMUTABLE as a body-level flag. An interface-induced engine clone
copied from such an entry, or a subclass method redeclaring an alias, then
looked immutable although its class lives in request memory, so redefine()
skipped the clone (the subclass kept the previous body) and
markRedeclaration() refused a perfectly writable class.

Decide by the holder class: a clone inside a mutable class follows the
swap, a redeclaring mutable subclass receives ZEND_ACC_CHANGED, and only a
shared-memory class is refused.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…verrides

An override installed over an inherited method must keep the engine's
inheritance semantics: it carries ZEND_ACC_CHANGED when the inherited
method was CHANGED or private (so an ancestor's private method still wins
from the ancestor's own scope), takes the inherited prototype otherwise,
and a fresh method gets neither.

Covers: a three-level chain whose leaf overrides a CHANGED method and a
subclass linked afterwards, an early-linked subclass tree, a private
ancestor method shadowed by an override, and a plain override that must
not acquire the flag.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…d entry

A subclass that implements an interface declaring an inherited method holds
an engine clone of the entry rather than the entry pointer. The clone must
follow redefine() in place, carry a preserveAs alias, be reachable by an
addMethod() override through the clone-holding descendants, keep its own
run-time cache after the swap, and be refused only when its class lives in
shared memory.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
… question

A body swap used to walk the whole class table for every question it asked
about its entry (its published shares, its clones, the shares of each clone,
and again at each commit for the cache release and the statics checks) and
the object store once per check, building a reflection wrapper per visited
class: about 190 ms per redefine() in a process holding 1,800 classes, with
the swap tests pacing their plateau loops around it.

EntryPublications takes one survey per redefinition - the buckets named like
the entry, read through an integer view of the class table
(HashTable::pointerValueAddresses(), the technique ObjectStore already uses)
with one method lookup per class and no wrapper until a class actually
publishes the name - and answers every question of the entry swap, the clone
swaps and their commits from memory. A commit gathers the live closures once
and takes every decision that reads them before anything is destroyed. The
survey keeps addresses only, so the object-store walk does not pay for its
own bookkeeping. MethodPublisher reaches the linked subclasses through
ReflectionClass::getLinkedSubclasses(), the same walk with a parent-chain
check instead of a wrapper per class.

Each clone is listed once, with the class the engine separated it for (found
by ancestry: table order lies after delayed early binding), so a
redefinition swaps a clone shared into a grand-subclass once instead of once
per table publishing it.

Same 1,800-class process: a redefine() takes 5 ms with 1,500 subclasses
sharing the entry and 2 ms among unrelated classes.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
…arios of the fixes

The matrix script runs under both allocators and both opcache modes and
shuts the process down through the destructor path, which is where the
fixed defects used to crash. Five scenarios are added: fake closures over a
redefined method surviving its next redefinition, an interface clone linked
between two redefinitions following the second, a closure donor with statics
preserved under an alias and swapped back before the alias is removed, an
addMethod() override under a redeclared ancestor-private method carrying
ZEND_ACC_CHANGED, and an overriding subclass linked before a preserving
redefine (refused on a shared-memory entry, served through parent::
otherwise). Every scenario also pins the engine state it relies on by
pointer identity and flag bits, and the hot loop drives the new entries.

On PHP 8.5 the statics scenario pins the refcount-guarded defaults table
the alias and the restored body share with the preserved body, instead of
the duplicates PHP 8.4 mints.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko
lisachenko marked this pull request as ready for review October 10, 2026 16:10
@lisachenko
lisachenko merged commit 230feb4 into 8.5 Oct 10, 2026
21 checks passed
@lisachenko
lisachenko deleted the feature/redefine-preserve-alias branch October 10, 2026 16:10
lisachenko pushed a commit to goaop/framework that referenced this pull request Oct 10, 2026
…is merged there

lisachenko/z-engine#282 was merged into 8.5 and its feature branch deleted, so
the PHP 8.5 leg and the install instructions point at dev-8.5. PHP 8.4 keeps
pinning the feature branch until lisachenko/z-engine#284 lands.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants