Re: pltcl.so patch

From: "Nigel J(dot) Andrews" <nandrews(at)investsystems(dot)co(dot)uk>
To: Neil Conway <neilc(at)samurai(dot)com>
Cc: Ian Harding <ianh(at)tpchd(dot)org>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: pltcl.so patch
Date: 2002-09-25 06:35:49
Message-ID: Pine.LNX.4.21.0209250730200.20523-100000@ponder.fairway2k.co.uk
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 25 Sep 2002, Neil Conway wrote:

> "Nigel J. Andrews" <nandrews(at)investsystems(dot)co(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Manfred Koizar 2002-09-25 06:54:56 Re: [SQL] CURRENT_TIMESTAMP
Previous Message Shridhar Daithankar 2002-09-25 06:24:39 Re: Postgresql Automatic vacuum