>>
>> 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
