From: M Z on
The thing is, why it doesn't crash on 8.3.8 but crash on 8.4.2? Any idea? A
patch was applied to 8.3 but not to 8.4.2?

Thanks,
M Z

On Fri, Feb 5, 2010 at 1:45 PM, Robert Haas <robertmhaas(a)gmail.com> wrote:

> On Wed, Feb 3, 2010 at 8:49 AM, Alvaro Herrera
> <alvherre(a)commandprompt.com> wrote:
> > Robert Haas escribió:
> >> On Mon, Feb 1, 2010 at 5:23 PM, Andrew Dunstan <andrew(a)dunslane.net>
> wrote:
> >> > Robert Haas wrote:
> >> >> (2) add a very, very large warning that this will crash if you do
> >> >> almost anything with it.
> >> >
> >> > I think that's an exaggeration. Certain people are known to be using
> it
> >> > quite successfully.
> >>
> >> Hmm. Well, all I know is that the first thing I tried crashed the
> server.
> >>
> >> CREATE TABLE xpath_test (id integer NOT NULL, t xml);
> >> INSERT INTO xpath_test VALUES (1, '<doc><int>1</int></doc>');
> >> SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int', 'true')
> >> as t(id int4);
> >
> > This trivial patch lingering on my system fixes this crasher (this is
> > for the 8.3 branch). It makes the "problem in alloc set ExprContext"
> > warning show up instead.
> >
> > There are still lotsa other holes, but hey, this is a start ...
>
> Interestingly M Z found he couldn't reproduce this crash on 8.3. Can
> you? If so, +1 for applying this and backpatching it as far as make
> sense.
>
> ...Robert
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
From: Tom Lane on
M Z <jm80008(a)gmail.com> writes:
> The thing is, why it doesn't crash on 8.3.8 but crash on 8.4.2? Any idea?

Pure luck. Memory-clobber bugs like these are notoriously
nondeterministic. Any minor, logically unrelated change could make them
visible or not visible, because the clobber happens to clobber data that
is or is not in active use at the moment.

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: M Z on
Hi Alvaro,

I followed your instruction but put the patch on 8.4.2 as I found it
crashes. It looks like the server still crash in the same way. Can you and
anyone give me some ideas how to fix this bug?

==============================
conifer=# CREATE TABLE xpath_test (id integer NOT NULL, t xml);
CREATE TABLE
conifer=# INSERT INTO xpath_test VALUES (1, '<doc><int>1</int></doc>');
INSERT 0 1
conifer=# SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int',
'true') as t(id int4);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Failed.
!>
==============================

Best,
M Z


>
> > CREATE TABLE xpath_test (id integer NOT NULL, t xml);
> > INSERT INTO xpath_test VALUES (1, '<doc><int>1</int></doc>');
> > SELECT * FROM xpath_table('id', 't', 'xpath_test', '/doc/int', 'true')
> > as t(id int4);
>
> > Hmm. Well, all I know is that the first thing I tried crashed the
> server.
>


> This trivial patch lingering on my system fixes this crasher (this is
> for the 8.3 branch). It makes the "problem in alloc set ExprContext"
> warning show up instead.
>
> There are still lotsa other holes, but hey, this is a start ...
>
> Index: contrib/xml2/xpath.c
> ===================================================================
> RCS file: /home/alvherre/Code/cvs/pgsql/contrib/xml2/xpath.c,v
> retrieving revision 1.16.2.1
> diff -c -p -r1.16.2.1 xpath.c
> *** contrib/xml2/xpath.c 26 Mar 2008 01:19:11 -0000 1.16.2.1
> --- contrib/xml2/xpath.c 27 Jan 2010 15:30:56 -0000
> *************** xpath_table(PG_FUNCTION_ARGS)
> *** 793,798 ****
> --- 793,801 ----
> */
> pgxml_parser_init();
>
> + PG_TRY();
> + {
> +
> /* For each row i.e. document returned from SPI */
> for (i = 0; i < proc; i++)
> {
> *************** xpath_table(PG_FUNCTION_ARGS)
> *** 929,934 ****
> --- 932,944 ----
> if (xmldoc)
> pfree(xmldoc);
> }
> + }
> + PG_CATCH();
> + {
> + xmlCleanupParser();
> + PG_RE_THROW();
> + }
> + PG_END_TRY();
>
> xmlCleanupParser();
> /* Needed to flag completeness in 7.3.1. 7.4 defines it as a no-op. */
>
> --
> Alvaro Herrera
> http://www.CommandPrompt.com/
> The PostgreSQL Company - Command Prompt, Inc.
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
From: Alvaro Herrera on
M Z escribi�:
> Hi Alvaro,
>
> I followed your instruction but put the patch on 8.4.2 as I found it
> crashes. It looks like the server still crash in the same way. Can you and
> anyone give me some ideas how to fix this bug?

See src/backend/utils/adt/xml.c. Note the comment at the top:

/*
* Notes on memory management:
*
* Sometimes libxml allocates global structures in the hope that it can reuse
* them later on. This makes it impractical to change the xmlMemSetup
* functions on-the-fly; that is likely to lead to trying to pfree() chunks
* allocated with malloc() or vice versa. Since libxml might be used by
* loadable modules, eg libperl, our only safe choices are to change the
* functions at postmaster/backend launch or not at all. Since we'd rather
* not activate libxml in sessions that might never use it, the latter choice
* is the preferred one. However, for debugging purposes it can be awfully
* handy to constrain libxml's allocations to be done in a specific palloc
* context, where they're easy to track. Therefore there is code here that
* can be enabled in debug builds to redirect libxml's allocations into a
* special context LibxmlContext. It's not recommended to turn this on in
* a production build because of the possibility of bad interactions with
* external modules.
*/
/* #define USE_LIBXMLCONTEXT */



Then if you look at xpath.c in contrib/xml2 you notice that it's doing
exactly the thing that the core module says it's unreliable: using
palloc and friends in xmlMemSetup. So to fix the bug what's needed is
that the xmlMemSetup call in contrib is removed altogether, and all
memory is tracked and released by hand. It's rather tedious, and it's
also difficult to plug all resulting memory leaks. But AFAIUI doing
that would fix (some of?) the crashes. Not sure if your crash is in
this category.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

From: Tom Lane on
Alvaro Herrera <alvherre(a)commandprompt.com> writes:
> Then if you look at xpath.c in contrib/xml2 you notice that it's doing
> exactly the thing that the core module says it's unreliable: using
> palloc and friends in xmlMemSetup. So to fix the bug what's needed is
> that the xmlMemSetup call in contrib is removed altogether, and all
> memory is tracked and released by hand. It's rather tedious, and it's
> also difficult to plug all resulting memory leaks. But AFAIUI doing
> that would fix (some of?) the crashes. Not sure if your crash is in
> this category.

FWIW, the core xml code seems to have been pretty stable since we gave
up on trying to redirect libxml's memory allocations to palloc.
So what you basically need to do to xpath.c is something like this:
http://archives.postgresql.org/pgsql-committers/2009-05/msg00229.php

regards, tom lane

--
Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers