Re: Memory leak with PL/Python trigger

On Fri, Jul 31, 2015 at 11:08 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> 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
>>>>> pointer
>>>>> in "fcinfo->flinfo->fn_extra".
>>>>> But this pointer always set as NULL for every call. Because of this
>>>>> reason
>>>>> the memory was leaked.
>>>>
>>>>
>>>> Adding to this point, In "PLyObject_ToComposite" function the context
>>>> pointer
>>>> 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.
>>>
>>>
>>> Yep.
>>>
>>>> Instead of reset the context in "domain_check_input" function, we can free
>>>> that
>>>> 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
>>>> taking
>>>> 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:
> http://www.postgresql.org/message-id/CAB7nPqRxvq+Q66UFzD9wa5UAftYN4WAUADbjXKFrync96kf-VQ@mail.gmail.com
> 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
proc->trftypes.

With the above changes, I didn't observe the memory leak.

Regards,
Hari Babu
Fujitsu Australia