Re: Memory leak with PL/Python trigger

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
Date: 2015-08-03 05:12:56
Message-ID: CAJrrPGdRgHqSueBxd43xw-utovF4WB-4f8MXyYL798jAenKE-w@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-bugs

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

Attachment Content-Type Size
fix-plpython-typeinfo-leaks-2.patch application/octet-stream 14.8 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
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