Re: Lack of Sanity Checking in file 'pctcl.c' for PostgreSQL 9.4.x
Michael Paquier <michael.paquier@gmail.com>
From: Michael Paquier <michael.paquier@gmail.com>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
Cc: Tom Lane <tgl@sss.pgh.pa.us>, Bill Parker <wp02855@gmail.com>,
PostgreSQL mailing lists <pgsql-bugs@postgresql.org>
Date: 2015-07-22T07:57:47Z
Lists: pgsql-bugs
Attachments
- 0001-Replace-use-of-malloc-by-an-internal-memory-context-.patch (application/x-patch) patch 0001
- 0002-Replace-use-of-malloc-by-an-internal-memory-context-.patch (application/x-patch) patch 0002
- 0003-Remove-the-use-of-perm_fmgr_info-in-pl-Perl.patch (application/x-patch) patch 0003
On Mon, Jul 20, 2015 at 9:29 PM, Alvaro Herrera <alvherre@2ndquadrant.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
Commits
-
Fix (some of) pltcl memory usage
- b2efbb71dfb4 9.3.10 landed
- b0b6f8d71f03 9.5.0 landed
- 4c11967e7348 9.0.23 landed
- 49c30004073f 9.4.5 landed
- 3cb6ef9983b5 9.2.14 landed
- 14ade020ffa8 9.1.19 landed
- f8d67ca8d4cb 9.6.0 landed