Lists: | pgsql-hackers |
---|
From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | memory leak in libxml2 - fix |
Date: | 2010-11-26 08:59:24 |
Message-ID: | AANLkTikFEjsvjgkOao232ePY2+6h1nH+wpZP=Gx32rTJ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Hello
our customer showed a very significant memory leak in xml2.
try to
select xpath_number('<data>' || generate_series || '</data>','/data')
from generate_series(1,500000);
attention! take all memory very fast
It never release a memory allocated for context and doctree.
Regards
Pavel Stehule
Attachment | Content-Type | Size |
---|---|---|
xml2memleakfix.diff | text/x-patch | 1.6 KB |
From: | Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 10:28:14 |
Message-ID: | AANLkTi=Z6FOV_M90c=L7jSmgwKhJn+Yei3ZopnACr4rv@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
On Fri, Nov 26, 2010 at 17:59, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> our customer showed a very significant memory leak in xml2.
> It never release a memory allocated for context and doctree.
Why did you change doctree and ctxt to global variables?
I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
at the end of pgxml_xpath(), but is it enough to enable the code?
--
Itagaki Takahiro
From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 10:42:26 |
Message-ID: | AANLkTimQM0=R17fOzh3REPxp+c=ura41mE65aLkS-Er+@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
2010/11/26 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
> On Fri, Nov 26, 2010 at 17:59, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
>> our customer showed a very significant memory leak in xml2.
>> It never release a memory allocated for context and doctree.
>
> Why did you change doctree and ctxt to global variables?
> I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
> at the end of pgxml_xpath(), but is it enough to enable the code?
>
I am thinking, so you must not to call xmlFreeDoc(doctree) early.
Probably xmlXPathCastToXXX reading a doctree.
xmlXPathFreeContext(ctxt); xmlFreeDoc(doctree); are called now only
when function doesn't return a value.
Regards
Pavel Stehule
> --
> Itagaki Takahiro
>
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
Cc: | Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 19:14:08 |
Message-ID: | 14418.1290798848@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
> 2010/11/26 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
>> Why did you change doctree and ctxt to global variables?
>> I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
>> at the end of pgxml_xpath(), but is it enough to enable the code?
> I am thinking, so you must not to call xmlFreeDoc(doctree) early.
> Probably xmlXPathCastToXXX reading a doctree.
Those static variables are really ugly, and what's more this patch only
stops some of the leakage. Per experimentation, the result object from
pgxml_xpath has to be freed too, once it's been safely converted to
whatever the end result type is. You can see this by watching
select sum(xpath_number('<data>' || generate_series || '</data>','/data')) from
generate_series(1,500000);
which still shows leakage with the submitted patch. I cleaned it up
as per attached, which doesn't show any leakage.
regards, tom lane
Attachment | Content-Type | Size |
---|---|---|
xml2-leakage-2.patch | text/x-patch | 10.0 KB |
From: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 19:56:39 |
Message-ID: | 1290801329-sup-5495@alvh.no-ip.org |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Excerpts from Tom Lane's message of vie nov 26 16:14:08 -0300 2010:
> Those static variables are really ugly, and what's more this patch only
> stops some of the leakage. Per experimentation, the result object from
> pgxml_xpath has to be freed too, once it's been safely converted to
> whatever the end result type is. You can see this by watching
>
> select sum(xpath_number('<data>' || generate_series || '</data>','/data')) from
> generate_series(1,500000);
>
> which still shows leakage with the submitted patch. I cleaned it up
> as per attached, which doesn't show any leakage.
This looks great. As this fixes a problem that was reported to us two
days ago, I'm interested in backpatching it. Are you going to do it?
Otherwise I can work on that.
--
Álvaro Herrera <alvherre(at)commandprompt(dot)com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Alvaro Herrera <alvherre(at)commandprompt(dot)com> |
Cc: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 20:05:22 |
Message-ID: | 15327.1290801922@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
Alvaro Herrera <alvherre(at)commandprompt(dot)com> writes:
> This looks great. As this fixes a problem that was reported to us two
> days ago, I'm interested in backpatching it. Are you going to do it?
Yeah, I'm on it. It's a bit tedious because the back branches are
different ...
regards, tom lane
From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: memory leak in libxml2 - fix |
Date: | 2010-11-26 20:12:53 |
Message-ID: | AANLkTinG2Z+giEnO2Kwm_zJobyaqDA_FPHSeqvAhhDNv@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Lists: | pgsql-hackers |
2010/11/26 Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>:
> Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> writes:
>> 2010/11/26 Itagaki Takahiro <itagaki(dot)takahiro(at)gmail(dot)com>:
>>> Why did you change doctree and ctxt to global variables?
>>> I'm not sure why /* xmlFreeDoc(doctree); */ is commented out
>>> at the end of pgxml_xpath(), but is it enough to enable the code?
>
>> I am thinking, so you must not to call xmlFreeDoc(doctree) early.
>> Probably xmlXPathCastToXXX reading a doctree.
>
> Those static variables are really ugly, and what's more this patch only
> stops some of the leakage. Per experimentation, the result object from
> pgxml_xpath has to be freed too, once it's been safely converted to
> whatever the end result type is. You can see this by watching
>
> select sum(xpath_number('<data>' || generate_series || '</data>','/data')) from
> generate_series(1,500000);
>
> which still shows leakage with the submitted patch. I cleaned it up
> as per attached, which doesn't show any leakage.
great
thank you very much
regards
Pavel Stehule
>
> regards, tom lane
>
>