Give each compiled function its own memory context#22
Merged
Conversation
Implements the FIXME that has been in this file since the beginning: a compiled function's descriptor and everything hanging off it (the name, fmgr info records and whatever they allocate) now live in a private memory context, child of TopMemoryContext, deleted wholesale when the function is redefined. fmgr info records consequently no longer accumulate in TopMemoryContext across redefinitions. This also closes a use-after-free window: the old code freed the stale descriptor while the compiled-function cache still pointed at it, and only re-added the cache entry later in recompilation. Any error raised in between -- realistically an unluckily timed statement cancel or an out-of-memory inside a syscache lookup -- left the cache holding a dangling pointer that the next call would dereference. The cache entry is now removed before the old context is deleted, and a compilation failure just leaves the entry absent. Compilation errors before the new descriptor is published reclaim its context via PG_TRY; a PHP-level eval failure keeps the established poisoned-xmin retry scheme. The scattered free() pairs in error paths are gone, and the inner "pointer" variable is renamed, silencing a long-standing -Wshadow warning. New coverage cases exercise the previously untested recompilation path: 50 consecutive CREATE OR REPLACE cycles with a call between each, ALTER FUNCTION invalidation, and recovery after replacing a function with an uncompilable body. Measured with a 5000-redefinition churn: descriptor-side memory is flat; the remaining ~1.8kB/redefinition of backend growth is Zend's per-eval compilation arena, identical before and after this change, and bounded by how often functions are redefined in a session. Co-Authored-By: Claude Fable 5 <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
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.
Implements the FIXME that has been in
plphp.csince the beginning: each compiled function's descriptor and all its subsidiary data (name, fmgr info records, and whatever those allocate) now live in a private memory context, child ofTopMemoryContext, deleted wholesale when the function is redefined.What this fixes
fmgr_info_cxttargets the function's context instead ofTopMemoryContext, so non-builtin I/O function state no longer accumulates across redefinitions.PG_TRYreclaims the context on any compilation error before the descriptor is published; the scatteredfree()pairs are gone; a long-standing-Wshadowwarning with them.Honest measurement
A 5000×
CREATE OR REPLACEchurn shows descriptor-side memory flat; the residual ~1.8 kB/redefinition is Zend's per-eval compilation arena — identical before and after, bounded by redefinition frequency, and not addressable from plphp.Tests
The recompilation path had zero coverage before; new cases in
coverage: 50 consecutive redefine-and-call cycles,ALTER FUNCTIONinvalidation, and recovery after replacing a function with an uncompilable body. Full suite green on PG 11–18 +jsonb_plphpin the container sweep.🤖 Generated with Claude Code