Thread: plpython crash (PG 92)

On tor, 2012-04-26 at 17:32 +0500, Asif Naeem wrote:
> PFA test case. It used simple select statement to retrieve data via
> plpython. It crashes latest pg 9.2 with the following stack trace i.e.

> Apparently it is being crashed because of invalid related pointer value of
> pfree() *header->context->methods->free_p. It is reproducible with latest
> version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks.

This is because of this code:

static void
PLy_result_dealloc(PyObject *arg)
{   PLyResultObject *ob = (PLyResultObject *) arg;
   Py_XDECREF(ob->nrows);   Py_XDECREF(ob->rows);   Py_XDECREF(ob->status);   if (ob->tupdesc)   {
FreeTupleDesc(ob->tupdesc); // <-- dies here       ob->tupdesc = NULL;   }
 
   arg->ob_type->tp_free(arg);
}

I must have been confused about the tuple descriptor APIs.  ob->tupdesc
is created using CreateTupleDescCopy(), which copies the refcount of the
original tuple descriptor, thus causing a failure when the (seemingly
still referenced) tupdesc is freed.  Is this behavior correct and
useful?

The dominant coding practice appears to be to not explicitly free copied
tuple descriptors, so maybe that should be done here as well.

Peter Eisentraut <peter_e@gmx.net> writes:
> I must have been confused about the tuple descriptor APIs.  ob->tupdesc
> is created using CreateTupleDescCopy(), which copies the refcount of the
> original tuple descriptor,

Um, surely not.  That would be nonsensical, and anyway a look at the
code shows it isn't doing that.

> thus causing a failure when the (seemingly still referenced) tupdesc
> is freed.  Is this behavior correct and useful?

I think there must be some mistake in your analysis.
        regards, tom lane

Peter Eisentraut <peter_e@gmx.net> writes:
> On tor, 2012-04-26 at 17:32 +0500, Asif Naeem wrote:
>> PFA test case. It used simple select statement to retrieve data via
>> plpython. It crashes latest pg 9.2 with the following stack trace i.e.

>> Apparently it is being crashed because of invalid related pointer value of
>> pfree() *header->context->methods->free_p. It is reproducible with latest
>> version of python i.e. Python-2.7.3 and Python-3.2.3. Thanks.

> This is because of this code:

I traced through this test case.  The situation is:

(1) PLy_spi_execute_fetch_result is executed with CurrentMemoryContext
being the SPI Proc context for the current SPI invocation, so that's
where CreateTupleDescCopy creates the tupledesc that's placed into
the PLyResultObject.

(2) As we fall out of the SETOF function with the first result row,
PLy_exec_function does SPI_finish() (plpy_exec.c:146).  That causes
the SPI contexts to go away.  The PLyResultObject's tupdesc pointer
is now pointing at freed memory, and in a cassert build, that memory
has been actively wiped.

(3) The main executor calls back to plpython for the next value-per-call
result.  Control goes to the PyIter_Next call at plpy_exec.c:108, which
decides that we're done iterating (since this example only returns one
row), and evidently it tries to deallocate the PLyResultObject
immediately.  Whether that happens immediately or later, though,
you're screwed because FreeTupleDesc will be invoked on garbage.

I'm inclined to think that the best fix is for
PLy_spi_execute_fetch_result to copy the tupledesc into
TopMemoryContext, not the current context.  This is a tad scary from a
memory leakage standpoint, but I suppose that if python fails to recover
the PLyResultObject, this isn't the only memory that's going to be
leaked.

This area appears to be shy a regression test case or two, in any event.
        regards, tom lane

On lör, 2012-04-28 at 00:32 -0400, Tom Lane wrote:
> I'm inclined to think that the best fix is for
> PLy_spi_execute_fetch_result to copy the tupledesc into
> TopMemoryContext, not the current context.  This is a tad scary from a
> memory leakage standpoint, but I suppose that if python fails to recover
> the PLyResultObject, this isn't the only memory that's going to be
> leaked.
> 
> This area appears to be shy a regression test case or two, in any event.

Fixed like that.