Re: fix crash with Python 3.11

Peter Eisentraut <peter.eisentraut@enterprisedb.com> writes:
> The way plpy.commit() and plpy.rollback() handle errors is not ideal. 
> They end up just longjmping back to the main loop, without telling the 
> Python interpreter about it.  This hasn't been a problem so far, 
> apparently, but with Python 3.11-to-be, this appears to cause corruption 
> in the state of the Python interpreter.  This is readily reproducible 
> and will cause crashes in the plpython_transaction test.
> The fix is that we need to catch the PostgreSQL error and turn it into a 
> Python exception, like we do for other places where plpy.* methods call 
> into PostgreSQL internals.

I agree that the existing code is broken and works only accidentally,
but I fear this patch does little to improve matters.  In particular,
it returns control to Python without having done anything to clean
up the now-invalid state of the transaction system.  (That is,
there's nothing analogous to PLy_spi_subtransaction_abort's
RollbackAndReleaseCurrentSubTransaction call in these new PG_CATCH
blocks).  The existing test cases apparently fail to trip over that
because Python just throws the exception again, but what if someone
writes a plpython function that catches the exception and then tries
to perform new SPI actions?

I think a possible fix is:

1. Before entering the PG_TRY block, check for active subtransaction(s)
and immediately throw a Python error if there is one.  (This corresponds
to the existing errors "cannot commit while a subtransaction is active"
and "cannot roll back while a subtransaction is active".  The point is
to reduce the number of system states we have to worry about below.)

2. In the PG_CATCH block, after collecting the error data do
    AbortOutOfAnyTransaction();
    StartTransactionCommand();
which gets us into a good state with no active subtransactions.

I'm not sure that those two are the best choices of xact.c
entry points, but there's precedent for that in autovacuum.c
among other places.  (autovacuum seems to think that blocking
interrupts is a good idea too.)

            regards, tom lane