call f

                ... somebody updates f's pg_proc tuple

                ... recursively call f


At this point PLy_procedure_get will decide the cache entry is obsolete
and happily trash the procedure data that the outer call is still using.
We ran into this long ago with plpgsql and fixed it by adding
reference-counting logic.  Probably the same needs to be done here.

I am afraid youre right. It seems ugly that all pl's have to reinvent
that kind of cache + invalidation logic. I am e.g. not sure they go far
enough in invalidating stuff like parameter types & io functions.

Pardon for coming late to the conversation.

First of all, thanks for fixing the trigger function caching bug, it trurly was a silly one.

The problem with reentrant calls of PLy_procedure_get and freeing an in-use PLyProcedure struct looks real enough. I took me some time to make it actually fail, though, as even a piece of code like this:

create or replace function nasty() returns void language plpythonu as $$
    if 'executed' in GD:
        return
    GD['executed'] = True
    plpy.execute('alter function nasty() cost 10')
    plpy.execute("select nasty()")
    plpy.error('oops')
$$;

select nasty();

was not bombing out, because after the reentrant call of PLy_get_procedure freed the PLyProcedure struct, it allocates another one for itself and it ends up in the exact same position in memory... I ended up adding a dummy PLy_malloc somewhere and got a segfault when plpy.error tried to access the current function name to add it to the errcontext.

We could add a refcount to PLyProcedure or reuse the PLy_execution_contexts scheme that's already in place.

When PLy_get_procedure decides it needs to recreate a cached function, it could walk the execution contexts stack looking for a matching pointer.

I'm not yet sure what it should do in that case. The only provision we have for freeing PLyProcedure structs is when they get evicted from the hash. Perhaps a refcount is the only way to handle it sanely.

By the way, I agree that the caching in PL/Python (and PLs in general) is ugly and ad hoc. Having to share memory management with the embedded PL doesn't help, though.

Cheers,
Jan


--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

Reply via email to