Thread

1. memory leak in libxml2 - fix

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-11-26T08:59:24Z

   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
   

2. Re: memory leak in libxml2 - fix

   Itagaki Takahiro <itagaki.takahiro@gmail.com> — 2010-11-26T10:28:14Z

   On Fri, Nov 26, 2010 at 17:59, Pavel Stehule <pavel.stehule@gmail.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
   
   

3. Re: memory leak in libxml2 - fix

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-11-26T10:42:26Z

   2010/11/26 Itagaki Takahiro <itagaki.takahiro@gmail.com>:
   > On Fri, Nov 26, 2010 at 17:59, Pavel Stehule <pavel.stehule@gmail.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
   >
   
   

4. Re: memory leak in libxml2 - fix

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-11-26T19:14:08Z

   Pavel Stehule <pavel.stehule@gmail.com> writes:
   > 2010/11/26 Itagaki Takahiro <itagaki.takahiro@gmail.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
   
   

5. Re: memory leak in libxml2 - fix

   Alvaro Herrera <alvherre@commandprompt.com> — 2010-11-26T19:56:39Z

   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@commandprompt.com>
   The PostgreSQL Company - Command Prompt, Inc.
   PostgreSQL Replication, Consulting, Custom Development, 24x7 support
   
   

6. Re: memory leak in libxml2 - fix

   Tom Lane <tgl@sss.pgh.pa.us> — 2010-11-26T20:05:22Z

   Alvaro Herrera <alvherre@commandprompt.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
   
   

7. Re: memory leak in libxml2 - fix

   Pavel Stehule <pavel.stehule@gmail.com> — 2010-11-26T20:12:53Z

   2010/11/26 Tom Lane <tgl@sss.pgh.pa.us>:
   > Pavel Stehule <pavel.stehule@gmail.com> writes:
   >> 2010/11/26 Itagaki Takahiro <itagaki.takahiro@gmail.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
   >
   >