pgsql: Be more careful about Python refcounts while creating exception

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: pgsql-committers(at)postgresql(dot)org
Subject: pgsql: Be more careful about Python refcounts while creating exception
Date: 2016-12-09 20:27:47
Message-ID: E1cFRlj-0001Nv-22@gemulon.postgresql.org
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers

Be more careful about Python refcounts while creating exception objects.

PLy_generate_spi_exceptions neglected to do Py_INCREF on the new exception
objects, evidently supposing that PyModule_AddObject would do that --- but
it doesn't. This left us in a situation where a Python garbage collection
cycle could result in deletion of exception object(s), causing server
crashes or wrong answers if the exception objects are used later in the
session.

In addition, PLy_generate_spi_exceptions didn't bother to test for
a null result from PyErr_NewException, which at best is inconsistent
with the code in PLy_add_exceptions. And PLy_add_exceptions, while it
did do Py_INCREF on the exceptions it makes, waited to do that till
after some PyModule_AddObject calls, creating a similar risk for
failure if garbage collection happened within those calls.

To fix, refactor to have just one piece of code that creates an
exception object and adds it to the spiexceptions module, bumping the
refcount first.

Also, let's add an additional refcount to represent the pointer we're
going to store in a C global variable or hash table. This should only
matter if the user does something weird like delete the spiexceptions
Python module, but lack of paranoia has caused us enough problems in
PL/Python already.

The fact that PyModule_AddObject doesn't do a Py_INCREF of its own
explains the need for the Py_INCREF added in commit 4c966d920, so we
can improve the comment about that; also, this means we really want
to do that before not after the PyModule_AddObject call.

The missing Py_INCREF in PLy_generate_spi_exceptions was reported and
diagnosed by Rafa de la Torre; the other fixes by me. Back-patch
to all supported branches.

Discussion: https://postgr.es/m/CA+Fz15kR1OXZv43mDrJb3XY+1MuQYWhx5kx3ea6BRKQp6ezGkg@mail.gmail.com

Branch
------
REL9_4_STABLE

Details
-------
http://git.postgresql.org/pg/commitdiff/13a4b37b9806bb591aaadd745300b95baec80515

Modified Files
--------------
src/pl/plpython/plpy_plpymodule.c | 79 +++++++++++++++++++++++++--------------
1 file changed, 50 insertions(+), 29 deletions(-)

Browse pgsql-committers by date

  From Date Subject
Next Message Keith Fiske 2016-12-09 22:55:29 Re: [COMMITTERS] pgsql: Implement table partitioning.
Previous Message Keith Fiske 2016-12-09 18:23:44 Re: [COMMITTERS] pgsql: Implement table partitioning.