fix(reflection): free swap-minted resources of SHM-shared bodies; pin the optimizer-inlining semantics of redefine under opcache - #262
Merged
Conversation
…t SHM-donor swaps Root cause of the "first redefine() does not take effect" failure (the plateau child with opcache active): plateau_function()'s body was a single `return 'original';`, and when opcache caches a script its optimizer INLINES every same-file call to such a trivial constant-returning function (zend_try_inline_call, Zend/Optimizer/optimize_func_calls.c, optimizer pass 4 of the default opcache.optimization_level) - the dispatch call sites were replaced by the literal at cache time, so they did not exist at runtime and no redefine could ever reach them. The separate-file matrix leg stayed green because the optimizer cannot resolve a callee outside the script it compiles. This is a compile-time transformation, not a resolution path the copy-out could repoint: it is now a documented copy-out caveat in docs/hot-swap.md, together with its warm-cache twin - a caller whose run-time cache already resolved the shared-memory entry before the copy-out keeps dispatching it, because copy-out redirects name resolution only (the rule the class copy-out has always documented; the method leg additionally tripped over the "instances created before the copy-out keep the shared class entry" caveat). The library bug the same run exposed: destroyPreviousBody() returned early for a previous body without a refcount (a body shared with opcache SHM, e.g. any donor closure declared in a cached file), leaking the swap-minted HEAP_RT_CACHE run-time cache and a statics defaults duplicate on every swap - 16 bytes/cycle in the fixed-donor plateau series. destroy_op_array frees exactly those per-entry resources BEFORE its refcount check and returns without touching the shared arrays, so the destroy path now runs it for both lifetime classes instead of bailing out. Tests make the original failure shape a permanent regression test: - redefine-plateau.php returns a runtime-defined constant (no call site can be inlined at cache time) and instantiates PlateauClass inside the method dispatch (created after the first redefine's class copy-out); - RedefineLeakPlateauTest pins the child to opcache ON (jit off, file_update_protection=0), so every suite exercises the same-file first-redefine copy-out path; measured overheads stay 0 and the fixed-donor series is flat again (was +16000 bytes per 1000 cycles); - the opcache support matrix gains a same-file-redefine leg (a cold same-file call site must observe the writable copy through the repointed bucket) and an inlined-call-site-limitation leg pinning the documented pass-4 behavior, with the child's optimization_level pinned to the default pipeline. Fixes #242 Co-Authored-By: Claude <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BDcCQiYqbMkjRPyhWgLL6M
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
Fixes #242. The "first
redefine()does not take effect under opcache" turned out to be three stacked mechanisms, only one of them a z-engine bug:plateau_function()returned a constant, so opcache's pass 4 (zend_try_inline_call, bit0x8of the defaultopcache.optimization_level) replaced every same-file call site with the literal at cache time — the call doesn't exist at runtime, so no repointing can ever affect it. Proved by opcode dumps (INIT_FCALL/DO_UCALLgone), a 4-way probe matrix (only zero-arg/constant-body shapes fail), and single-bit bisection of the optimization level. Copy-out itself was verified correct (bucket repointed; dynamic$name()calls see the new body). Not fixable in the library — now pinned as named semantics: new matrix legssame-file-redefine: ok(a cold same-file call site must observe the copy) andinlined-call-site-limitation: ok(an inlined site stays baked; the leg fails loudly if a future engine changes the pass-4 behavior).zend_function*in its run-time cache slot — mirrors the documented class-side "resolution only is redirected" model;docs/hot-swap.mdgains the function-side caveats and corrects one sentence that claimed the opposite. A follow-up issue proposes an opt-in targeted cache patch (maintainer decision).FunctionBodySwap::destroyPreviousBody()bailed early for refcount-less previous bodies (any donor closure declared in a cached file shares its body with SHM), leaking the swap-mintedHEAP_RT_CACHE(16 B/swap) and minted statics duplicates. Verified against php-8.4.19'sdestroy_op_array: it frees exactly those per-entry resources before its refcount check and never touches a refcount-less body's shared arrays — the fix runs it for both lifetime classes. Plateau evidence:fixedDonorgrowth 16000 B → 0 over 1000 cycles.Test strategy: the plateau child pin flips from opcache-off (
TODO(#242)) to explicit opcache ON (JIT off), so every default-suite run permanently exercises the originally failing shape, with uninlinable runtime-defined bodies; the matrix legs pin the semantics under the default optimization level.Environment it was verified on
php -v): PHP 8.4.19 (cli) (built: Mar 30 2026 19:28:35) (NTS)--enable-debug)? yes — debug 8.4 container:--group opcache --fail-on-skippedOK (31 tests, 194 assertions),--group internal --process-isolationOK (165 tests); plateau + matrix children green under engine assertionsAlso: reproducer exit 0 with opcache on (3 stable runs, all series flat); default suite OK (503 tests); opcache-runner suite OK (501); PHPStan level max clean; cs-fixer clean.
Checklist
8.4) — fixes cascade upward, never downwardcomposer testpasses on the matching PHP minorcomposer phpstan(level max) andcomposer cs:checkare greenlayout_structstools/generator/symbols.phpunchanged — nothing underinclude/,stubs/or.phpstorm.meta.phptouched🤖 Generated with Claude Code
https://claude.ai/code/session_01BDcCQiYqbMkjRPyhWgLL6M
Generated by Claude Code