I wrote:
> It seems to me there are probably other hazards here, because the tupdesc
> could possibly also go away before the slot does. I think what we ought
> to do is copy the tupdesc, so that we have a non-refcounted descriptor
> that we know has exactly the right lifespan. As attached.
Actually, after looking around some more, I realize that there is a
second mistake in 25936fd46: it ignored the comment on
AfterTriggerFreeQuery that
* Note: it's important for this to be safe if interrupted by an error
* and then called again for the same query level.
This is the reason why we end up in a recursive error and PANIC:
we keep trying to free the tupdesc again after the previous error.
If I fix that but omit the CreateTupleDescCopy step, then the
reproducer behaves much more sanely:
psql:bug17607.sql:29: WARNING: AbortSubTransaction while in ABORT state
psql:bug17607.sql:29: ERROR: BOOM !
CONTEXT: PL/pgSQL function on_tbl_parent_id_change_fn() line 3 at RAISE
ERROR: tupdesc reference 0x7fdef236a1b8 is not owned by resource owner SubTransaction
ROLLBACK
ROLLBACK
So what we actually need here is more like the attached.
regards, tom lane
diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c
index 59b38d00ed..1b1193997b 100644
--- a/src/backend/commands/trigger.c
+++ b/src/backend/commands/trigger.c
@@ -4340,11 +4340,13 @@ GetAfterTriggersStoreSlot(AfterTriggersTableData *table,
MemoryContext oldcxt;
/*
- * We only need this slot only until AfterTriggerEndQuery, but making
- * it last till end-of-subxact is good enough. It'll be freed by
- * AfterTriggerFreeQuery().
+ * We need this slot only until AfterTriggerEndQuery, but making it
+ * last till end-of-subxact is good enough. It'll be freed by
+ * AfterTriggerFreeQuery(). However, the passed-in tupdesc might have
+ * a different lifespan, so we'd better make a copy of that.
*/
oldcxt = MemoryContextSwitchTo(CurTransactionContext);
+ tupdesc = CreateTupleDescCopy(tupdesc);
table->storeslot = MakeSingleTupleTableSlot(tupdesc, &TTSOpsVirtual);
MemoryContextSwitchTo(oldcxt);
}
@@ -4640,7 +4642,12 @@ AfterTriggerFreeQuery(AfterTriggersQueryData *qs)
if (ts)
tuplestore_end(ts);
if (table->storeslot)
- ExecDropSingleTupleTableSlot(table->storeslot);
+ {
+ TupleTableSlot *slot = table->storeslot;
+
+ table->storeslot = NULL;
+ ExecDropSingleTupleTableSlot(slot);
+ }
}
/*
