Re: pltcl.so patch - Mailing list pgsql-hackers

Oh, so this is the later version.  Fine.  Let me know when it is ready.

---------------------------------------------------------------------------

Nigel J. Andrews wrote:
> 
> 
> Okay, I've looked again at spi_exec and I believe I can fix the bug I
> introduced and the memory leak. However, I have only looked quickly and not
> made these most recent changes to the execp version nor to the plpython
> code. Therefore I am not attaching a patch at the moment, just mentioning that
> I've straightened this out in my brain a bit more.
> 
> 
> On Wed, 25 Sep 2002, Nigel J. Andrews wrote:
> 
> > On 25 Sep 2002, Neil Conway wrote:
> > 
> > > "Nigel J. Andrews" <nandrews@investsystems.co.uk> writes:
> > > > Yes, I do get the similar results.
> > > > 
> > > > A quick investigation shows that the SPI_freetuptable at the end of
> > > > pltcl_SPI_exec is trying to free a tuptable of value 0x82ebe64
> > > > (which looks sensible to me) but which has a memory context of
> > > > 0x7f7f7f7f (the unallocated marker).
> > > 
> > > Attached is a patch against CVS HEAD which fixes this, I believe. The
> > > problem appears to be the newly added free of the tuptable at the end
> > > of pltcl_SPI_exec(). I've added a comment to that effect:
> > > 
> > >     /*
> > >      * Do *NOT* free the tuptable here. That's because if the loop
> > >      * body executed any SQL statements, it will have already free'd
> > >      * the tuptable itself, so freeing it twice is not wise. We could
> > >      * get around this by making a copy of SPI_tuptable->vals and
> > >      * feeding that to pltcl_set_tuple_values above, but that would
> > >      * still leak memory (the palloc'ed copy would only be free'd on
> > >      * context reset).
> > >      */
> > 
> > That's certainly where the fault was happening. However, that's where the
> > original memory leak problem was coming from (without the SPI_freetuptable
> > call). It could be I got that fix wrong and the extra calls you've added are
> > the right fix for that. I'll take a look to see what I can learn later.
> > 
> > > At least, I *think* that's the problem -- I've only been looking at
> > > the code for about 20 minutes, so I may be wrong. In any case, this
> > > makes both memleak() and memleak(1) work on my machine. Let me know if
> > > it works for you, and/or if someone knows of a better solution.
> > 
> > I'll have to check later.
> > 
> > > 
> > > I also added some SPI_freetuptable() calls in some places where Nigel
> > > didn't, and added some paranoia when dealing with statically sized
> > > buffers (snprintf() rather than sprintf(), and so on). I also didn't
> > > include Nigel's changes to some apparently unrelated PL/Python stuff
> > > -- this patch includes only the PL/Tcl changes.
> > 
> > I dare say the plpython needs to be checked by someone who knows how to since I
> > can well imagine the same nested call fault will exist there.
> > 
> > 
> > 
> 
> -- 
> Nigel J. Andrews
> 
> 
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 2: you can get off all lists at once with the unregister command
>     (send "unregister YourEmailAddressHere" to majordomo@postgresql.org)
> 

--  Bruce Momjian                        |  http://candle.pha.pa.us pgman@candle.pha.pa.us               |  (610)
359-1001+  If your life is a hard drive,     |  13 Roberts Road +  Christ can be your backup.        |  Newtown Square,
Pennsylvania19073