Re: Memory leak with PL/Python trigger

On Fri, Jul 31, 2015 at 9:54 PM, Haribabu Kommi
<kommi.haribabu@gmail.com> wrote:
> On Fri, Jul 31, 2015 at 7:13 PM, Heikki Linnakangas <hlinnaka@iki.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.
--
Michael