From: Alexander Lakhin <exclusion@gmail.com>
To: Richard Guo <guofenglinux@gmail.com>
Cc: pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW UPDATE trigger
Date: Thu, 20 Apr 2023 13:00:00 +0300
Message-ID: <f5ff446f-72e2-44f7-80c6-4256cf7caeb3@gmail.com> (raw)
In-Reply-To: <CAMbWs48FHhuuuWbxygO4Wey7ahdaSwRjzJODKXx7=GFsge_aLg@mail.gmail.com>
References: <17798-0907404928dcf0dd@postgresql.org>
<87fe6c28-80b2-f709-a8db-d105e5240e39@gmail.com>
<CAMbWs4_pg7ab=OJF2CGLzs=noWEjujerxfo92sm3RL9ce=ogsQ@mail.gmail.com>
<CAMbWs48FHhuuuWbxygO4Wey7ahdaSwRjzJODKXx7=GFsge_aLg@mail.gmail.com>
Hello Richard,
19.04.2023 11:29, Richard Guo wrote:
> I have a second look at this issue and now I think the fix in v1 patch
> is correct. I think the comment needs to be updated for this change,
> maybe something like
>
> * (Typically, newslot was also generated by ExecGetUpdateNewTuple, so
> * that epqslot_clean will be that same slot and the copy step below
> - * is not needed.)
> + * is not needed. And we need to materialize newslot in this case,
> + * since its tuple might be dependent on oldslot's storage, which
> + * might not be a local copy and be freed before we fetch newslot's
> + * tuple.)
Thanks for looking into this!
I've updated the comment based on your suggestion. Please look at the patch
attached.
I hesitate to add the isolation test to the patch as adding a dedicated
test just for this case seems too wasteful to me. Maybe that scenario could
be added to one of the existing trigger-related tests, but I couldn't find a
test, which is generic enough for that.
Best regards,
Alexander
Attachments:v2-01-Fix-memory-access-in-BRU-trigger.patch (text/x-patch, 962B) downloaddiff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.cindex 4b295f8da5..8027498b73 100644--- a/src/backend/commands/trigger.c+++ b/src/backend/commands/trigger.c@@ -2981,7 +2981,9 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
*
* (Typically, newslot was also generated by ExecGetUpdateNewTuple, so
* that epqslot_clean will be that same slot and the copy step below
- * is not needed.)+ * is not needed. Though we need to materialize newslot in this case,+ * since its tuple might share the oldslot's buffer, which might+ * be freed on fetching trigtuple from oldslot.)
*/
if (epqslot_candidate != NULL)
{
@@ -2992,6 +2994,8 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate,
if (newslot != epqslot_clean)
ExecCopySlot(newslot, epqslot_clean);
+ else+ ExecMaterializeSlot(newslot);
}
trigtuple = ExecFetchSlotHeapTuple(oldslot, true, &should_free_trig);
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-bugs@postgresql.org
Cc: exclusion@gmail.com, guofenglinux@gmail.com, pgsql-bugs@lists.postgresql.org
Subject: Re: BUG #17798: Incorrect memory access occurs when using BEFORE ROW UPDATE trigger
In-Reply-To: <f5ff446f-72e2-44f7-80c6-4256cf7caeb3@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox