Re: Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)

From: Amit Langote <amitlangote09(at)gmail(dot)com>
To: Lucas Jeffrey <lucas(dot)jeffrey(at)anachronics(dot)com>
Cc: Trakshan Mishra <trakshanmishra477(at)gmail(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Andrés Krüger <andres(dot)kruger(at)anachronics(dot)com>, Rodolfo Campero <rodolfo(dot)campero(at)anachronics(dot)com>
Subject: Re: Re: [PATCH] Fix segmentation fault caused by reentrancy in RI_Fkey_cascade_del (ri_triggers.c)
Date: 2026-10-23 09:58:22
Message-ID: CA+HiwqFeiAxNVqS=H5GHtSTB3hivFjLCHJcaC+WL59_ny72Gxw@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Oct 23, 2026 at 6:01 AM Lucas Jeffrey
<lucas(dot)jeffrey(at)anachronics(dot)com> wrote:
> El jue, 22 oct 2026 a las 8:18, Amit Langote (<amitlangote09(at)gmail(dot)com>) escribió:
>> On Wed, Oct 21, 2026 at 9:02 PM Amit Langote <amitlangote09(at)gmail(dot)com> wrote:
>> > Thanks for the patch. I'm going to study it and post an updated
>> > version tomorrow. I'd like to handle errors as well as Lucas's patch
>> > does but I'd stick to using PG_TRY in all the branches instead of
>> > resowner in some and PG_TRY in others. I believe subtransaction
>> > aborts are handled correctly by PG_TRY-based solution but I'd need to
>> > test that. In any case, I want to avoid hash table and resowner calls
>> > in the per-row path and keep the tracking code as minimal as possible.
>>
>> Sorry, ran out of time today, so will post the patch tomorrow.
>
> Thanks both for the work on this. I've been iterating on Amit's v1 plus
> Trakshan's stack, and added error handling on top. Patch attached (v9).
>
> Amit, does the approach used to defer freeing the plans look OK to you?
> To keep it conservative I don't free anything while the error is being
> handled; maybe that's not needed and we could free them in the
> PG_FINALLY instead.

Thanks for v9. I have more or less the same patch in my local tree
that I was about to post. Yeah, it makes sense to keep plan freeing
out of the error handling path.

So, here's v10, which streamlines the stack code by putting it in
functions instead of directly in ri_PerformCheck as in your v9. It
also keeps the tests from your v8-0001 and adds tests for behavior on
subxact rollbacks. You'll notice that the commit message also details
the history of what you found, what the buildfarm reported, etc.

I've also attached patch files for the back-branches, which I've
tested down to v14. v14 has a test file conflict so it needs a
different patch file. v15 through v18 share the same patch file.

As mentioned earlier, the commit message lists you as the author, with
Trakshan and me as co-authors, which I think reflects how this came
together.

I’d like to commit this early next week unless there are objections.

--
Thanks, Amit Langote

Attachment Content-Type Size
v10-REL_15-18_STABLE-0001-Don-t-free-a-cached-RI-plan-tha.patch application/octet-stream 25.9 KB
v10-0001-Don-t-free-a-cached-RI-plan-that-may-be-in-use.patch application/octet-stream 28.1 KB
v10-REL_14_STABLE-0001-Don-t-free-a-cached-RI-plan-that-m.patch application/octet-stream 25.8 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alexander Lakhin 2026-10-23 10:00:00 Re: [PG19] Promotion hangs behind a backend blocked on a stuck client
Previous Message Álvaro Herrera 2026-10-23 09:55:31 Re: Resetting snapshots during the first phase of [CREATE |RE]INDEX CONCURRENTLY