On 2013-01-25 17:07:59 -0500, Tom Lane wrote:
> Andres Freund <and...@2ndquadrant.com> writes:
> > The bug got introduced in 46211da1b84bc3537e799ee1126098e71c2428e8 which
> > interestingly says "Using HTABs instead of Python dictionaries makes
> > error recovery easier, and allows for procedures to be cached based on
> > their OIDs, not their names." - but the caching seems to always have
> > used ("%u_%u", fn_oid, tgreloid) as its key which explains why the bug
> > didn't exist back then.
>
> > Anyway, as far as I can see only 9.1 upwards are affected.
>
> Patch applied with some editorialization, mostly cosmetic but one thing
> not so much: you introduced uninitialized-variable hazards into
> PLy_procedure_get, since if use_cache is false then found, entry, and
> proc wouldn't get set up.Gah. Now I am pissed at myself. I thought I had initialized 'found' to false which should be enough. > Neither version of gcc I tried complained about this, and I assume yours > didn't either, which is a bit annoying/scary. I wonder whether the > "volatile" marker prevents that? I think the control flow is just to complex for gcc, it probably cannot trace anything across a setjmp() althout it theoretically should be possible given the failure is in the branch that is executed unconditionally. Given that the real culprit is 'found' I don't think its the volatiles. > BTW, it seems to me that there is another bug in PLy_procedure_get, > which is that it's not thinking about recursive use of a procedure. > Consider > > 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. > > There are probably not that many people trying to use plpython functions > in such a way (note the recursion would have to happen via a SPI query, > not directly within Python). But still... 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. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services -- Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-bugs
