Hi, On Fri, 7 Aug 2026 at 18:39, Amit Langote <[email protected]> wrote:
> On Thu, Aug 6, 2026 at 11:50 PM Amit Langote <[email protected]> > wrote: > > > > Hi, > > > > $SUBJECT was reported to me off-list. > > Patches attached. > Thanks for the patches! > 0001 is what I described upthread. InvalidateConstraintCacheCallBack() > unlinks the metadata from the cache entry rather than freeing it, so > the next check rebuilds it exactly as it does today, and the detached > object is chained onto a list that AtEOXact_RI() releases, at which > point no RI check can be on the stack. The call sites that test > riinfo->fpmeta == NULL are unchanged. > > 0002 is what was 0004 in the series I last posted at [1], and fixes a > separate leak in the same struct. ri_populate_fastpath_metadata() > copies the cast and equality FmgrInfos with fn_mcxt set to > TopMemoryContext, and fn_mcxt is scratch space for the called > function: record_eq(), and the record I/O functions generally, > allocate their per-call cache there and keep a pointer to it in > fn_extra. That scratch space outlives the metadata, so any cached data > is orphaned each time the metadata is discarded. 0002 gives the > FmgrInfos a context of their own and deletes it alongside the struct > in the AtEOXact_RI() cleanup. > > 0002 must follow 0001 because there's no safe place to delete the > context until 0001 adds the deferred release. In the callback it would > be a second use-after-free, this time in memory the called function > owns rather than memory we do: an in-flight record_eq() has fn_extra > pointing into the context and fn_mcxt at the context itself, so it > would read back freed state or palloc into a deleted context. > > Ayush Tiwari reviewed 0002 at [1], and while doing so, independently > raised the same lifetime problem this thread is about and 0001 is > meant to fix. > > I've added open items for both, noting that the second must be > committed with / after the first. > > [1] > https://www.postgresql.org/message-id/CA%2BHiwqHHK2D69%2BQqAom%2Bth1kjZGK-0dYnrZujkEGFWpX_ZtxqQ%40mail.gmail.com I tried the patches and the overall deferred-free approach looks sensible to me. One corner case made me wonder if `fpmeta` also needs to be latched for the whole multi-column batch flush. `ri_FastPathFlushLoop()` calls `build_index_scankeys()` for each row, and that function reads `riinfo->fpmeta` again. In a small test where the first row's cast renames the FK constraint, the invalidation clears that field and the second row reaches `Assert(fpmeta)`. I may be missing another invariant here, but passing a pointer latched at the start of the flush seems to avoid it, much like the array path already does. Apart from that, the commit/abort/prepare cleanup and the `fn_mcxt` ownership looked reasonable in my testing. Regards, Ayush
