Re: Lack of Sanity Checking in file 'pctcl.c' for PostgreSQL 9.4.x

From: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>
To: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bill Parker <wp02855(at)gmail(dot)com>, PostgreSQL mailing lists <pgsql-bugs(at)postgresql(dot)org>
Subject: Re: Lack of Sanity Checking in file 'pctcl.c' for PostgreSQL 9.4.x
Date: 2015-07-22 07:57:47
Message-ID: CAB7nPqRxvq+Q66UFzD9wa5UAftYN4WAUADbjXKFrync96kf-VQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs

On Mon, Jul 20, 2015 at 9:29 PM, Alvaro Herrera
<alvherre(at)2ndquadrant(dot)com> wrote:
> Michael Paquier wrote:
>
>> > With some additional effort, we could get rid of perm_fmgr_info, at
>> > least in pltcl. (That hack was introduced in a3ed622b63b and
>> > 7748e9e7e5a back in 2001 and we never actually fixed it ...)
>>
>> Yes it seems so, even for plperl and plpython there seem to be some
>> room for improvement at quick glance... This looks like a master-only
>> change to me though (and I am sure we are on the same page). For
>> back-branches something like the previous patch is definitely safer.
>
> Pushed.
>
> All in all, the whole business of memory allocation in pltcl is pretty
> nasty. If people is really interested in pltcl, that should probably be
> fixed to avoid memory leaks. For one thing, the query cache hash is
> global to the interpreter, not local to the function; whenever a
> function is recompiled, the old SPI_prepared plans linger forever in the
> interpreter (for the life of the session). It would be nicer if those
> were tied to the function lifetime. For this, the hashtable would have
> to be moved to the query desc struct. pltcl_spi_prepare would have to
> know the currently executing function desc struct ...

If this is done, the same can be directly applied to plperl. For
plpython, we are going to visibly need a memory context initialized at
some point and rely on it for allocations instead of TopMemoryContext
when doing the various allocations... Do you think it is worth it in
this case?

For what it's worth, I have been playing for a couple of minutes with
the code of pltcl and plperl to replace the remaining malloc calls by
a memory context and remove the calls to perm_func_ctx. This does not
fix the memory leaks of pltcl when functions are recompiled by this is
working somewhat nicely with plperl by relying on free_plperl_function
and check-world passes. So attached they are. This seem to improve a
bit things to me. Thoughts?
--
Michael

Attachment Content-Type Size
0001-Replace-use-of-malloc-by-an-internal-memory-context-.patch application/x-patch 6.9 KB
0002-Replace-use-of-malloc-by-an-internal-memory-context-.patch application/x-patch 4.8 KB
0003-Remove-the-use-of-perm_fmgr_info-in-pl-Perl.patch application/x-patch 2.1 KB

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message p.buongiovanni 2015-07-22 08:25:38 BUG #13513: Turning a table into a view
Previous Message ziyu-v 2015-07-22 06:40:42 BUG #13512: insert/select bytea met memory alloc issue