Tom Lane escribió:
> Alvaro Herrera <[EMAIL PROTECTED]> writes:
> > Perhaps a better idea is to create a separate LibxmlContext memcxt,
> > child of QueryContext, and have xml_palloc etc always use that. That
> > way it won't be reset between calls. It probably also means we could
> > wire xml reset to transaction abort, making it a bit simpler.
>
> Might as well go back to letting it use malloc :-(.
>
> I actually don't see a problem with letting it use malloc for stuff that
> it is going to manage for itself. I guess the problem comes with
> transient return values of libxml functions; we'd want to explicitly
> move those into palloc-based storage and then free() them.
>
> This looks like a rather large rewrite though. Peter, have you any
> better ideas?
OK, after a lot of research I think the best way to deal with this is
what I suggest above. With the attached patch, I cannot cause the
system to crash with any of the given examples.
Furthermore this may help clean up the PG_TRY blocks that are currently
sprinkled through the xml.c code.
Handling of subtransactions is no good at this point, but I think that
could easily be improved.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Index: src/backend/access/transam/xact.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/transam/xact.c,v
retrieving revision 1.256
diff -c -p -r1.256 xact.c
*** src/backend/access/transam/xact.c 3 Jan 2008 21:23:15 -0000 1.256
--- src/backend/access/transam/xact.c 10 Jan 2008 21:00:58 -0000
***************
*** 45,50 ****
--- 45,51 ----
#include "utils/inval.h"
#include "utils/memutils.h"
#include "utils/relcache.h"
+ #include "utils/xml.h"
/*
*************** CommitTransaction(void)
*** 1678,1683 ****
--- 1679,1685 ----
AtEOXact_ComboCid();
AtEOXact_HashTables(true);
AtEOXact_PgStat(true);
+ AtEOXact_xml();
pgstat_report_xact_timestamp(0);
CurrentResourceOwner = NULL;
*************** AbortTransaction(void)
*** 2028,2033 ****
--- 2030,2036 ----
AtEOXact_ComboCid();
AtEOXact_HashTables(false);
AtEOXact_PgStat(false);
+ AtEOXact_xml();
pgstat_report_xact_timestamp(0);
/*
Index: src/backend/utils/adt/xml.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/adt/xml.c,v
retrieving revision 1.64
diff -c -p -r1.64 xml.c
*** src/backend/utils/adt/xml.c 1 Jan 2008 19:45:53 -0000 1.64
--- src/backend/utils/adt/xml.c 10 Jan 2008 20:45:48 -0000
*************** XmlOptionType xmloption;
*** 80,85 ****
--- 80,86 ----
#ifdef USE_LIBXML
static StringInfo xml_err_buf = NULL;
+ static MemoryContext LibxmlContext = NULL;
static void xml_init(void);
static void *xml_palloc(size_t size);
*************** xml_init(void)
*** 953,958 ****
--- 954,965 ----
xmlSetGenericErrorFunc(NULL, xml_errorHandler);
/* Set up memory allocation our way, too */
+ Assert(LibxmlContext == NULL);
+ LibxmlContext = AllocSetContextCreate(TopTransactionContext,
+ "LibxmlContext",
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup);
/* Check library compatibility */
*************** xml_init(void)
*** 974,983 ****
--- 981,1007 ----
* about, anyway.
*/
xmlSetGenericErrorFunc(NULL, xml_errorHandler);
+ if (LibxmlContext == NULL)
+ LibxmlContext = AllocSetContextCreate(TopTransactionContext,
+ "LibxmlContext",
+ ALLOCSET_DEFAULT_MINSIZE,
+ ALLOCSET_DEFAULT_INITSIZE,
+ ALLOCSET_DEFAULT_MAXSIZE);
xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup);
}
}
+ void
+ AtEOXact_xml(void)
+ {
+ if (LibxmlContext == NULL)
+ return;
+
+ MemoryContextDelete(LibxmlContext);
+ LibxmlContext = NULL;
+
+ xmlCleanupParser();
+ }
/*
* SQL/XML allows storing "XML documents" or "XML content". "XML
*************** print_xml_decl(StringInfo buf, const xml
*** 1207,1213 ****
* Convert a C string to XML internal representation
*
* TODO maybe, libxml2's xmlreader is better? (do not construct DOM,
! * yet do not use SAX - see xml_reader.c)
*/
static xmlDocPtr
xml_parse(text *data, XmlOptionType xmloption_arg, bool preserve_whitespace,
--- 1231,1237 ----
* Convert a C string to XML internal representation
*
* TODO maybe, libxml2's xmlreader is better? (do not construct DOM,
! * yet do not use SAX - see xmlreader.c)
*/
static xmlDocPtr
xml_parse(text *data, XmlOptionType xmloption_arg, bool preserve_whitespace,
*************** xml_parse(text *data, XmlOptionType xmlo
*** 1245,1251 ****
/*
* Note, that here we try to apply DTD defaults
* (XML_PARSE_DTDATTR) according to SQL/XML:10.16.7.d: 'Default
! * valies defined by internal DTD are applied'. As for external
* DTDs, we try to support them too, (see SQL/XML:10.16.7.e)
*/
doc = xmlCtxtReadDoc(ctxt, utf8string,
--- 1269,1275 ----
/*
* Note, that here we try to apply DTD defaults
* (XML_PARSE_DTDATTR) according to SQL/XML:10.16.7.d: 'Default
! * values defined by internal DTD are applied'. As for external
* DTDs, we try to support them too, (see SQL/XML:10.16.7.e)
*/
doc = xmlCtxtReadDoc(ctxt, utf8string,
*************** xml_text2xmlChar(text *in)
*** 1325,1331 ****
static void *
xml_palloc(size_t size)
{
! return palloc(size);
}
--- 1349,1355 ----
static void *
xml_palloc(size_t size)
{
! return MemoryContextAlloc(LibxmlContext, size);
}
*************** xml_pfree(void *ptr)
*** 1346,1352 ****
static char *
xml_pstrdup(const char *string)
{
! return pstrdup(string);
}
--- 1370,1376 ----
static char *
xml_pstrdup(const char *string)
{
! return MemoryContextStrdup(LibxmlContext, string);
}
*************** xpath(PG_FUNCTION_ARGS)
*** 3368,3374 ****
"could not parse XML data");
xpathctx = xmlXPathNewContext(doc);
if (xpathctx == NULL)
! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR,
"could not allocate XPath context");
xpathctx->node = xmlDocGetRootElement(doc);
if (xpathctx->node == NULL)
--- 3392,3398 ----
"could not parse XML data");
xpathctx = xmlXPathNewContext(doc);
if (xpathctx == NULL)
! xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY,
"could not allocate XPath context");
xpathctx->node = xmlDocGetRootElement(doc);
if (xpathctx->node == NULL)
Index: src/include/utils/xml.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/utils/xml.h,v
retrieving revision 1.22
diff -c -p -r1.22 xml.h
*** src/include/utils/xml.h 1 Jan 2008 19:45:59 -0000 1.22
--- src/include/utils/xml.h 10 Jan 2008 21:01:19 -0000
*************** extern char *map_sql_identifier_to_xml_n
*** 75,80 ****
--- 75,82 ----
extern char *map_xml_name_to_sql_identifier(char *name);
extern char *map_sql_value_to_xml_value(Datum value, Oid type);
+ extern void AtEOXact_xml(void);
+
typedef enum
{
XMLBINARY_BASE64,
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
