Re: [BUGS] BUG #7656: PL/Perl SPI_freetuptable() segfault

On 11/13/2012 12:17 PM, Tom Lane wrote:
> pgmail@joh.to writes:
>> I have a reproducible segmentation fault in PL/Perl.  I have yet to narrow
>> down the test case to something sensible, but I do have a backtrace:
>> 219        while (context->firstchild != NULL)
>> (gdb) bt
>> #0  0x0000000104e90782 in MemoryContextDeleteChildren (context=0x1000002bd)
>> at mcxt.c:219
>> #1  0x0000000104e906a8 in MemoryContextDelete (context=0x1000002bd) at
>> mcxt.c:174
>> #2  0x0000000104bbefb5 in SPI_freetuptable (tuptable=0x7f9ae4289230) at
>> spi.c:1003
>> #3  0x000000011ec9928b in plperl_spi_execute_fetch_result
>> (tuptable=0x7f9ae4289230, processed=1, status=-6) at plperl.c:2900
>> #4  0x000000011ec98f27 in plperl_spi_exec (query=0x7f9ae4155f80
>> "0x7f9ae3e3fe50", limit=-439796840) at plperl.c:2821
>> #5  0x000000011ec9b5f7 in XS__spi_exec_query (my_perl=0x7f9ae40cce00,
>> cv=0x7f9ae4148e90) at SPI.c:69
>> While trying to narrow down the test case I noticed what the problem was: I
>> was calling spi_execute_query() instead of spi_execute_prepared().
> Hm.  It looks like SPI_execute failed as expected (note the status
> passed to plperl_spi_execute_fetch_result is -6 which is
> SPI_ERROR_ARGUMENT), but it did not reset SPI_tuptable, which led to
> plperl_spi_execute_fetch_result trying to call SPI_freetuptable on what
> was probably an already-deleted tuple table.
>
> One theory we could adopt on this is that this is
> plperl_spi_execute_fetch_result's fault and it shouldn't be trying to
> free a tuple table unless status > 0.
>
> Another theory we could adopt is that SPI functions that are capable of
> setting SPI_tuptable ought to clear it at start, to ensure that they
> return it as null on failure.
>
> The latter seems like a "nicer" fix but I'm afraid it might have
> unexpected side-effects.  It would certainly be a lot more invasive.


These aren't mutually exclusive, though, are they? It seems reasonable 
to do the minimal fix for the stable branches (looks like it's just a 
matter of moving the call up a couple of lines in plperl.c) and make the 
nicer fix just for the development branch.

cheers

andrew