Re: Plpython crashing the backend in one easy step - fix

From: Bradley McLean <brad(at)bradm(dot)net>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: pgsql-hackers(at)postgresql(dot)org, jacobs(at)penguin(dot)theopalgroup(dot)com, pgman(at)candle(dot)pha(dot)pa(dot)us
Subject: Re: Plpython crashing the backend in one easy step - fix
Date: 2001-11-15 18:55:47
Message-ID: 20011115135547.A22776@bradm.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Okay, the attached patch contains the following:

- Kevin Jacob's patch to make plpython worthy of a 'trusted' language
by eliminating the ability to arbitrarily read OS files. He also did
a bunch of C declaration cleanups. (His patch has yet to appear on
either hackers or patches, and I believe is also a prerequisite for
plpython in 7.2 final).

- Changed every place that catches elog() longjmps to re-propogate.
- Added code to keep track of the current plpython function and add
it to the elog messages
- #ifdef 0 around some items that appear to be not in use.
- changes to the expected output of the regression tests, because:

Tom's requirement that we always reraise the longjmp is at odds with
the original design of plpython, which attempted to allow a plpython
function to use Python's exception handling to recover when an
embedded SPI call failed. There is no way to do this that I can
see without halting the propogation of the longjmp.

Thus, the semantics of 'execute' calls from plpython have been changed:
should the backend throw an error, the plpython function will be
summarily aborted. Previously, it could attempt to catch a python
exception.

Note that this does create some redundant exception handling paths
in upper level methods within this module that are no longer used.
I have not yet removed them, but will happily do so (in a careful
deliberate manner) once sure that there is no alternative way to
restore the python level exception handling.

Let me know if this isn't what's needed to get this up to snuff
for 7.2.

-Brad

* Tom Lane (tgl(at)sss(dot)pgh(dot)pa(dot)us) [011113 16:49]:
>
> Yes, as in "it's totally unsafe". Suppressing an elog(ERROR) is
> a *big* no-no at present, because way too much stuff relies on
> post-abort cleanup to clean up whatever problem is being reported.
> You cannot allow the transaction to continue after the error, and
> you most certainly mustn't cavalierly reset the error handling state.
>
> The only things you should be doing with longjmp trapping are
> (a) doing any cleanup that Python itself has to have before you
> re-propagate the longjmp, or
> (b) issuing elog(NOTICE) to help identify the error location
> before you re-propagate the longjmp.
>
> plpgsql contains an example of doing (b).
>
> Not propagating the longjmp, which is what the code seems to be doing
> at present, is not acceptable. I had not looked at this code closely,
> but as-is it is a huge reliability hazard. I will insist on removing
> plpython from 7.2 entirely if this can't be fixed before release.
>
> regards, tom lane

Attachment Content-Type Size
plpython.patch text/plain 21.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Sergio Pili 2001-11-15 19:35:02 WAS: [Fwd: PostgreSQL new commands proposal]
Previous Message Ross J. Reedstrom 2001-11-15 17:29:41 Re: problem: index on number not honoured