|From:||Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>|
|To:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>|
|Cc:||Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, "Behn, Edward (EBEHN)" <EBEHN(at)arinc(dot)com>, "pgsql-bugs(at)postgresql(dot)org" <pgsql-bugs(at)postgresql(dot)org>|
|Subject:||Re: Memory leak with PL/Python trigger|
|Views:||Raw Message | Whole Thread | Download mbox|
On Thu, Aug 20, 2015 at 6:18 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com> writes:
>> Here I attached updated patch with a context allocation in
>> "PLy_procedure_create" function and the same context is reset
>> in every function call of "PLy_procedure_get" for all PLY types.
> This isn't really going in the direction I had in mind.
> I think what we want is to get rid of *all* of plpython's retail
> allocations in TopMemoryContext. All the long-lived data about
> a given procedure, starting with its PLyProcedure struct and
> certainly including all the associated PLyTypeInfo stuff, ought
> to be in a per-procedure context, similarly to the way plpgsql
> manages it. Then you can just delete that context if the procedure
> definition changes, and not need any retail cleanup.
> Also, you can't just reset the context being used as fn_mcxt
> without any regard for the possibility that functions have
> stored pointers into that space (probably in their fn_extra).
> Really you ought to redo fmgr_info() if you do that. That
> means that this approach is fundamentally giving up the ability
> to cache type lookup data across calls at all, which is surely
> not what we want.
I created a memory context in PLy_procedure_create function and all
further on related to PLy_procedure are allocated from that new context. This
context is freed whenever the procedure gets deleted.
Similar changes are done for PLyPlanObject and PLyCursorObject objects.
PLy_malloc0 and PLy_strdup function usage is completely removed
by replacing them with palloc0 and pstrdup. PLy_malloc function usage
> I would envision the PLyTypeInfo structs as usually living in the
> per-procedure context, and that's where fn_mcxt would point as well.
> There might be some cases where we have to have shorter-lived
> PLyTypeInfos, but the normal case ought to work like that.
For the temporary PLyTypeInfos, a temporary memory context is allocated
and used. Instead of using a CurrentMemoryContext or PLyExecutionContext,
I used a new temporary context. The same is freed at the end of the function.
Is the attached patch is in the right direction to the handle the problem?
|Next Message||Thomas Munro||2015-09-23 07:16:09||Re: BUG #13601: bit as quoted column in output|
|Previous Message||Vicky Soni - Quipment India||2015-09-23 06:35:13||Re: BUG #13601: bit as quoted column in output|