From 7666e8b806bcf87e7627152c9dcefc92f427e6f0 Mon Sep 17 00:00:00 2001 From: Thomas Munro Date: Fri, 16 Aug 2019 11:25:02 +1200 Subject: [PATCH] Fix double-free when one trigger passes OLD to another trigger. In PLs that are capable of returning the old tuple directly (rather than making a copy), if one tuple returns OLD directly (without copying), ExecBRUpdateTriggers() would double-free it when processing the next trigger. This doesn't affect 12 and later, which rewrote the relevant code. Repair in back branches. Reported-by: Piotr Kosinski Discussion: https://postgr.es/m/CAFMLSdP0rd7LqC3j-H6Fh51FYSt5A10DDh-3%3DW4PPc4LLUQ8YQ%40mail.gmail.com --- src/backend/commands/trigger.c | 4 +++- src/test/regress/expected/triggers.out | 25 ++++++++++++++++++++++ src/test/regress/sql/triggers.sql | 29 ++++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index a716827c77..97ab357a24 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3052,7 +3052,9 @@ ExecBRUpdateTriggers(EState *estate, EPQState *epqstate, relinfo->ri_TrigFunctions, relinfo->ri_TrigInstrument, GetPerTupleMemoryContext(estate)); - if (oldtuple != newtuple && oldtuple != slottuple) + if (oldtuple != newtuple && + oldtuple != slottuple && + oldtuple != trigtuple) heap_freetuple(oldtuple); if (newtuple == NULL) { diff --git a/src/test/regress/expected/triggers.out b/src/test/regress/expected/triggers.out index 5be0009f00..e67a6a6a2d 100644 --- a/src/test/regress/expected/triggers.out +++ b/src/test/regress/expected/triggers.out @@ -283,6 +283,31 @@ select * from tttest where price_on <= 35 and price_off > 35 and price_id = 5; drop table tttest; drop sequence ttdummy_seq; -- +-- test correct handling of trigtuple lifetime when one trigger returns old +-- tuple and it becomes new tuple for the next trigger +-- +create table tttest (id int, bar int); +create or replace function tf1() returns trigger language plpgsql as $$ +begin + return old; +end; +$$; +create or replace function tf2() returns trigger language plpgsql as $$ +begin + new.bar := 42; + return new; +end; +$$; +create trigger t1 +before update on tttest +for each row execute procedure tf1(); +create trigger t2 +before update on tttest +for each row execute procedure tf2(); +insert into tttest values (1, 1); +update tttest set bar = bar where id = 1; +drop table tttest; +-- -- tests for per-statement triggers -- CREATE TABLE log_table (tstamp timestamp default timeofday()::timestamp); diff --git a/src/test/regress/sql/triggers.sql b/src/test/regress/sql/triggers.sql index 6376046c5d..300e79ee5f 100644 --- a/src/test/regress/sql/triggers.sql +++ b/src/test/regress/sql/triggers.sql @@ -190,6 +190,35 @@ select * from tttest where price_on <= 35 and price_off > 35 and price_id = 5; drop table tttest; drop sequence ttdummy_seq; +-- +-- test correct handling of trigtuple lifetime when one trigger returns old +-- tuple and it becomes new tuple for the next trigger +-- + +create table tttest (id int, bar int); +create or replace function tf1() returns trigger language plpgsql as $$ +begin + return old; +end; +$$; +create or replace function tf2() returns trigger language plpgsql as $$ +begin + new.bar := 42; + return new; +end; +$$; +create trigger t1 +before update on tttest +for each row execute procedure tf1(); +create trigger t2 +before update on tttest +for each row execute procedure tf2(); +insert into tttest values (1, 1); + +update tttest set bar = bar where id = 1; + +drop table tttest; + -- -- tests for per-statement triggers -- -- 2.22.0