Re: Memory leak with PL/Python trigger - Mailing list pgsql-bugs
From | Haribabu Kommi |
---|---|
Subject | Re: Memory leak with PL/Python trigger |
Date | |
Msg-id | CAJrrPGdRgHqSueBxd43xw-utovF4WB-4f8MXyYL798jAenKE-w@mail.gmail.com Whole thread Raw |
In response to | Re: Memory leak with PL/Python trigger (Michael Paquier <michael.paquier@gmail.com>) |
Responses |
Re: Memory leak with PL/Python trigger
(Haribabu Kommi <kommi.haribabu@gmail.com>)
|
List | pgsql-bugs |
On Fri, Jul 31, 2015 at 11:08 PM, Michael Paquier <michael.paquier@gmail.com> wrote: > 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. 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
pgsql-bugs by date: