Fix infinite recursion when a plugin on pdoToolsOnFenomInit re-enters getFenom()#396
Open
ShevArtV wants to merge 1 commit into
Open
Fix infinite recursion when a plugin on pdoToolsOnFenomInit re-enters getFenom()#396ShevArtV wants to merge 1 commit into
ShevArtV wants to merge 1 commit into
Conversation
…sOnFenomInit CoreTools::getFenom() resolves the shared `fenom` service from the container, whose factory builds the Fenom instance. The Fenom constructor fired the pdoToolsOnFenomInit event before the container had memoized the instance, so any plugin attached to that event which re-enters getFenom() caused the factory to run again -> a new Fenom -> the event again -> ... until the PHP memory limit was exhausted. This is reliably triggered with pdotools_fenom_parser=1: loading a static element resolves its file through a media source, and modElement::getSourceFile() parses the source properties with the (pdo) parser, which calls getFenom(). If a static plugin is bound to pdoToolsOnFenomInit, the page dies with a fatal "Allowed memory size exhausted". Fire pdoToolsOnFenomInit once from getFenom(), after the instance is stored in the services container, guarded by a flag. A re-entrant getFenom() then returns the already-registered instance instead of constructing a new one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Problem
On MODX 3 (
3x-new), a static plugin attached to thepdoToolsOnFenomInitevent makes the whole page die with:The same happens for any code reachable from a
pdoToolsOnFenomInithandler that re-entersCoreTools::getFenom()while Fenom is still being constructed.How to reproduce
pdotools_fenom_parser = 1(use Fenom as the parser).pdoToolsOnFenomInitand mark it Static file (any handler, e.g. one that registers a custom modifier).A DB (non‑static) plugin on the same event works fine, which is why this is easy to miss.
Root cause
fenomis registered as a shared service inbootstrap.php:The container memoizes the instance only after the factory closure returns. But the
Fenomconstructor invokespdoToolsOnFenomInitbefore that:So if a plugin on that event re-enters
getFenom()→services->get('fenom'), the instance is not cached yet → the factory runs again → a newFenom→ the event again → … → memory exhausted.With
pdotools_fenom_parser = 1this is reliably hit by static elements: loading a static plugin resolves its file through a media source, andmodElement::getSourceFile()parses the source properties with the parser, which is the pdoParser and callsgetFenom()again. Trace of the re-entry:(MODX 2 / the
masterbranch is unaffected: itsgetSourceFile()only runs the parser when the file path itself contains tags, and never parses media‑source properties.)Fix
Fire
pdoToolsOnFenomInitonce, fromgetFenom(), after the instance is registered in the container — guarded by a flag. A re‑entrantgetFenom()then returns the already‑registered instance instead of building a new one. The event is removed from the constructor and exposed asFenom::invokeInitEvent().Validation
On a clean MODX 3 + pdoTools stand with
pdotools_fenom_parser = 1and a plugin onpdoToolsOnFenomInitthat registers a modifier:getChunk('@INLINE {$x\|myModifier}')returns the modified value in all cases;pdoToolsOnFenomInitis still dispatched exactly once per request.