|From:||Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>|
|To:||Michael Paquier <michael(dot)paquier(at)gmail(dot)com>|
|Cc:||Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, "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 Fri, Jul 31, 2015 at 11:08 PM, Michael Paquier
> On Fri, Jul 31, 2015 at 9:54 PM, Haribabu Kommi
> <kommi(dot)haribabu(at)gmail(dot)com> wrote:
>> On Fri, Jul 31, 2015 at 7:13 PM, Heikki Linnakangas <hlinnaka(at)iki(dot)fi> wrote:
>>> On 07/31/2015 11:05 AM, Haribabu Kommi wrote:
>>>>> Thanks for the defect details.
>>>>> I am able to reproduce the issue on Head.
>>>>> This is because of many unnecessary "Expr Context" that are created in
>>>>> function "domain_check_input"
>>>>> using CreateStandaloneExprContext function under TopMemoryContext.
>>>>> This memory context is reset for future use by storing the context
>>>>> in "fcinfo->flinfo->fn_extra".
>>>>> But this pointer always set as NULL for every call. Because of this
>>>>> the memory was leaked.
>>>> Adding to this point, In "PLyObject_ToComposite" function the context
>>>> which was allocated earlier is freed without removing the context.
>>>> Because of this
>>>> reason for the next record, it again allocates the context and thus it
>>>> leading to a
>>>> memory leak.
>>>> Instead of reset the context in "domain_check_input" function, we can free
>>>> context, which can solve this problem. But I am not sure, is it right fix?
>>>> Instead of the above fix, either we need to teach plpython functions to
>>>> care of reusing the pointer or something?
>>> The problem is that perm_fmgr_info() is a crock, as explained in its
>>> comments. That crock leads to this kind of memory leaks.
>>> I'm not sure why we use that crock. It doesn't seem hard to just use a more
>>> short-lived memory context. I hacked together the attached patch, which
>>> fixes this particular test case, but I just used TopMemoryContext in most
>>> other places so this doesn't plug all the leaks. But I think we want
>>> something like this, but using an appropriate memory context in each
>>> PLy_typeinfo_init call. For some call sites, I think we do need to create a
>>> new memory context that can be easily freed in PLy_typeinfo_dealloc().
>>> Would you like to finish up that patch?
>> Thanks for providing details.
>> I find similar perm_fmgr_info() function in both plperl and pltcl modules also.
>> I will check the function impact on those two modules also and finish up
>> the patch.
> You may as well look at that:
> I have played with plperl and pltcl for exactly the same reasons less
> than 10 days ago, and sent some patches that switch some code paths to
> use temporary memory contexts instead of perl_fmgr_info.
The patches in the above specified link are already covered the memory
leaks in plperl and pltcl modules.
Here I attached an updated patch by changing the context into
PLyExecutionContext and CurrrentMemoryContext
except in PLy_procedure_create function. In PLy_procedure_create
function the TopMemoryContext only used.
Along with the above changes, I added a list_free_deep in
PLy_procedure_delete function to clean the
With the above changes, I didn't observe the memory leak.
|Next Message||mrupert||2015-08-03 14:15:52||BUG #13534: Pgadmin crash when changing font|
|Previous Message||paulovieira||2015-08-03 01:42:14||BUG #13533: jsonb_populate_record does not work when the value is a simple string|