Skip to content

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

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

feat(reflection): preserve the previous body of redefine() under an alias and accept method donors (PHP 8.4)#284
lisachenko merged 15 commits into
8.4from
feature/redefine-preserve-alias-8.4

Conversation

@lisachenko

@lisachenko lisachenko commented Oct 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Owner

What this changes

The PHP 8.4 port of #282: the same public surface (redefine(\Closure|FunctionLikeInterface $newBody, ?string $preserveAs = null), ReflectionClass::addMethod() accepting a method donor, RedefinitionException, SharedMemoryException::sharedEntryHasLinkedSubclasses(), the non-heap run-time cache rule, the donor signature check resolving self/parent per declaring class), cherry-picked onto 8.4 and reconciled with this branch's static-variable ownership model, which PHP 8.5 changed:

  • a closure donor owns its statics defaults table, so the swap duplicates it (duplicateStatics: true, the rule this branch already had); a method donor's table lives as long as the body the entry now holds a reference on, so it is shared (duplicateStatics: false, the same rule as for published overrides) - unless the donor's body is a closure's (a method installed with addMethod($closure) keeps its live slot pointing at its defaults) or carries a table minted by another swap, which get a duplicate whatever the flag says;
  • PendingBodySwap and FunctionBodySwap::releaseSwappedInBody() carry both the minted statics-defaults record of this branch and the minted run-time cache record of the feature; destroyPreviousBody() releases both.

The second commit is #283 (PHPStan 2.3 test findings), cherry-picked so the analysis is green here too.

This PR targets 8.4 as the minimum affected branch; once merged, the merge-up into 8.5 conflicts with #282 in FunctionBodySwap/PendingBodySwap (the 8.5 branch has no statics duplication) and #282 is the resolution for 8.5. The goaop/framework driver (goaop/framework#765) runs against this branch on PHP 8.4 and against #282 on PHP 8.5.

Fixes from the adversarial review of the port

An independent review of the port (statics ownership, run-time cache lifecycle, shared-memory and subclass sharing, engine definitions 8.4 vs 8.5) found nine defects, each reproduced on the pre-fix tree (segfault, zend_mm_heap corrupted, wrong dispatch) and fixed in ed9548b/615d9b3:

  • 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 (what the engine gives a closure over a heap-cached function), clones get a fresh minted cache, a frame still running on the block keeps it allocated.
  • Minted statics duplicate shared by two structs (8.4 only). An alias/override container adopting a body with a minted defaults table, or a method donor carrying one, shared the table the owner's next swap freed eagerly (use-after-free, double free). A minted table now belongs to exactly one zend_function: every adopter mints a duplicate of its own.
  • Minted record overwritten before the commit compared it (8.4 only). Consecutive closure-donor swaps leaked one defaults table per swap; the previous record is read before the swap overwrites it.
  • 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. PHP 8.4 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.
  • Closure-backed method used as a method donor (8.4 only). addMethod($closure) installs a body whose live slot points at its defaults table; taking that method as a donor shared the table (double free). Duplication now keys on the body shape, not on the argument type.

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. The matrix segfaults in every configuration on the pre-fix tree.

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 (5669853) 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.4.19) before after
redefine(), 1,500 subclasses sharing the entry 190 ms 7 ms
redefine(), unrelated classes - 2-3 ms
composer test (default suite) 67 s 24 s

Known, unrelated

  • On this PHP 8.4.19 build ZEngine\System\ExecutionDataTest::testGetThis corrupts the heap on origin/8.4 without any of these changes; the suite below was run with that test excluded.
  • addMethod($closure) followed by a destructor-path shutdown (USE_ZEND_ALLOC=0) aborts on origin/8.4 as well (the closure-backed container); the matrix therefore installs no closure-backed method.

Environment it was verified on

  • PHP version (full first line of php -v): PHP 8.4.19 (cli) (built: Oct 10 2026) (NTS)
  • Thread safety: NTS
  • OS / architecture: Linux x86_64
  • Debug build (--enable-debug)? no

Verified: the new tests in both runner modes (also under USE_ZEND_ALLOC=0, the reproducers under valgrind), composer test (minus testGetThis, see above), the opcache-active runner mode (test:opcache-runner, minus testGetThis), composer phpstan with PHPStan 2.3.1, composer cs:check, and the goaop/framework zengine driver group against this head.

Checklist

  • Targets the minimum affected version branch (master = newest
    supported minor, 8.4 = PHP 8.4, …) — fixes cascade upward, never
    downward
  • composer test passes on the matching PHP minor (with the pre-existing ExecutionDataTest::testGetThis abort excluded)
  • 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

lisachenko and others added 2 commits October 10, 2026 01:31
@lisachenko @claude
feat(reflection): preserve the previous body of redefine() under an a…
05ef32f 
…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.

The donor signature check renders `self` and `parent` resolved against each
side's own scope (declaring class, or the bound scope of a closure donor): PHP
8.4 reflection reports the keywords verbatim, so a method donor spelling the
target class is compared by what its types accept.

PHP 8.4 specifics of this port: the statics-ownership bookkeeping of this
branch stays in place next to the run-time cache records - a closure donor
owns its statics defaults table, so the swap duplicates it (duplicateStatics:
true), while a method donor's table lives as long as the body the entry now
holds a reference on (duplicateStatics: false, the same rule as for published
overrides); PendingBodySwap and releaseSwappedInBody() carry both records.

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

(cherry picked from commits 5471958, 67e183b and 91a170c of feature/redefine-preserve-alias)

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
test: keep the test suite clean under PHPStan 2.3
31261d2 
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 4c0316e)
@lisachenko lisachenko mentioned this pull request Oct 10, 2026
Draft
4 tasks done
lisachenko and others added 13 commits October 10, 2026 04:24
@lisachenko @claude
fix(reflection): keep engine copies of a swapped entry valid and rele…
ed9548b 
…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);
- on 8.4 an alias or override adopting a body whose statics defaults were
  minted by a swap, and a method donor carrying such a table or a closure's
  own table (a method published from a closure), shared it and had it freed
  from under them; the adopter now duplicates it like a closure donor's, the
  container release returns the duplicate, and the previous record is read
  before the new swap overwrites it, so the eager release of a minted table
  matches again instead of leaking one table per swap;
- 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, the minted duplicate 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.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
docs(hot-swap): describe the engine copies a swap must respect and wh…
287c55a 
…at preserveAs keeps

Fake closures and interface clones of a swapped entry, the live statics a
preserved alias takes over, the subclasses an alias reaches, the CHANGED
propagation of overrides, the per-adopter statics duplicates on 8.4 and the
resources removeMethods() releases - as implemented by ed9548b.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
test(reflection): pin the run-time cache release against fake closure…
5966b08 
…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.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
test(reflection): keep the mid-flight scenario's state on the test class
b1f400d 
Same shape as on the 8.5 branch, where a closure donor's bound variables stay
with the closure object and the swapped-in body would read them unbound.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
test(reflection): pin the ownership of the statics defaults a swap mi…
3fa313b 
…nts on 8.4

Every adopter of a body carrying a minted defaults table - the preserving
alias, an addMethod() override, an entry taking it as a method donor - mints a
duplicate of its own and keeps it when the source entry's next swap releases
the source's, the alias continues on the live statics it took over while a
never-called alias starts from its duplicate of the defaults, and consecutive
closure-donor swaps of a function and of a method stay memory-flat (the
previous duplicate used to leak once per swap).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko @claude
test(reflection): pin the preserved alias in the tables of every subc…
004b48f 
…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
@lisachenko @claude
test(reflection): pin the live statics a preserved alias takes over a…
6761937 
…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
@claude
fix(reflection): judge immutability by the owning class, not the entr…
615d9b3 
…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
@claude
test(reflection): cover ZEND_ACC_CHANGED propagation of addMethod() o…
e39e2dd 
…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
@claude
test(reflection): cover interface-induced engine clones of a redefine…
5aea050 
…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
@claude
test(reflection): cover a closure-backed method used as a method donor
09acee1 
On PHP 8.4 a method installed from a closure keeps its live statics slot
pointing at its defaults table. Taking that method as a donor must give the
entry its own duplicate, so the two methods count independently, the donor
can be removed under a live entry, and a plain method donor still shares.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@claude
test(hot-swap): extend the alias matrix with the destructor-path scen…
43f8f41 
…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.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@claude
perf(reflection): survey the tables once per swap instead of once per…
5669853 
… 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 7 ms with 1,500 subclasses
sharing the entry and 2-3 ms among unrelated classes; the default suite runs
in 24 s instead of 67 s.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko lisachenko mentioned this pull request Oct 10, 2026
Merged
5 of 6 tasks
lisachenko pushed a commit to goaop/framework that referenced this pull request Oct 10, 2026
@claude
ci(zengine): install the 8.5 branch of z-engine now that the feature …
726df22 
…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
@lisachenko
lisachenko marked this pull request as ready for review October 10, 2026 16:20
@lisachenko
lisachenko merged commit 0128a09 into 8.4 Oct 10, 2026
21 checks passed
@lisachenko
lisachenko deleted the feature/redefine-preserve-alias-8.4 branch October 10, 2026 16:21
lisachenko pushed a commit to goaop/framework that referenced this pull request Oct 10, 2026
@claude
ci(zengine): install the 8.4 branch of z-engine now that the feature …
d8785fc 
…is merged there

lisachenko/z-engine#284 was merged into 8.4, so the PHP 8.4 driver leg, the
coverage job and the install instructions require 8.4.x-dev instead of the
feature branch; both minors now run against the version branches.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01VLQcsvgdQUzwBFsoKUAcG2
@lisachenko lisachenko mentioned this pull request Oct 10, 2026
Merged
6 tasks done
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@lisachenko @claude