Thread: Triggers might be caching broken fmgr info

Try this with current sources:

Add this line

update pg_proc set prosrc = 'foobar' where proname = 'check_primary_key';

to the end of regress/sql/create_function_1.sql.  (The function is created
earlier in this file.)  This simulates the situation where the
Dynamic_library_path (under development) has become bad between creation
and use of a function.

Then run the regression tests (I ran installcheck).  You will get a
segfault in the 'triggers' test which looks something like this:

DEBUG:  StartTransactionCommand
DEBUG:  query: insert into fkeys2 values (10, '1', 1);
DEBUG:  ProcessQuery
ERROR:  Can't find function foobar in file /home/peter/pgsql/src/test/regress/../../../contrib/spi/refint.so
DEBUG:  AbortCurrentTransaction
DEBUG:  StartTransactionCommand
DEBUG:  query: insert into fkeys2 values (30, '3', 2);
DEBUG:  ProcessQuery
pg-install/bin/postmaster: reaping dead processes...

The core file ends like this:

#0  0x7f7f7f7f in ?? ()
#1  0x80f01ad in ExecBRInsertTriggers (estate=0x82a14e8, rel=0x4035bb34,   trigtuple=0x82a1b0c) at trigger.c:900

(I don't know what the #0 is trying to tell me.)

My best guess is that the trigger fmgr lookup in trigger.c:846
if (trigger->tgfunc.fn_oid == InvalidOid)    fmgr_info(trigger->tgfoid, &trigger->tgfunc);

might be reading out of an incompletely initialized trigger->tgfunc
structure.  This is supported by the fact that if I move the

finfo->fn_oid = functionId;

assignment in fmgr_info() to the very end of that function, this passes
gracefully (because the elog comes before the assignment).

This might even be a workable fix, but I'm wondering whether elog(ERROR)
should not flush the trigger cache.

-- 
Peter Eisentraut   peter_e@gmx.net   http://funkturm.homeip.net/~peter

Peter Eisentraut <peter_e@gmx.net> writes:
> My best guess is that the trigger fmgr lookup in trigger.c:846
>     if (trigger->tgfunc.fn_oid == InvalidOid)
>         fmgr_info(trigger->tgfoid, &trigger->tgfunc);
> might be reading out of an incompletely initialized trigger->tgfunc
> structure.

Yes, that's the problem.

> This is supported by the fact that if I move the
> finfo->fn_oid = functionId;
> assignment in fmgr_info() to the very end of that function, this passes
> gracefully (because the elog comes before the assignment).

I think this is an OK fix, since utils/fmgr/README documents as valid
the technique trigger.c is using:
fn_oid = InvalidOid can be usedto denote a not-yet-initialized FmgrInfo struct.

fmgr_info is clearly failing to live up to the implications of that
commitment.  Please move the fn_oid assignment, and insert a note
that it must be last...

> This might even be a workable fix, but I'm wondering whether elog(ERROR)
> should not flush the trigger cache.

The cache in question is part of the relcache, and no we don't want to
flush it on every elog(ERROR).
        regards, tom lane