Index: src/backend/access/transam/xact.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/access/transam/xact.c,v retrieving revision 1.256 diff -c -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 15 Jan 2008 01:26:02 -0000 *************** *** 45,50 **** --- 45,51 ---- #include "utils/inval.h" #include "utils/memutils.h" #include "utils/relcache.h" + #include "utils/xml.h" /* *************** *** 1671,1676 **** --- 1672,1678 ---- AtEOXact_GUC(true, 1); AtEOXact_SPI(true); + AtEOXact_xml(); AtEOXact_on_commit_actions(true); AtEOXact_Namespace(true); /* smgrcommit already done */ *************** *** 1880,1885 **** --- 1882,1888 ---- /* PREPARE acts the same as COMMIT as far as GUC is concerned */ AtEOXact_GUC(true, 1); AtEOXact_SPI(true); + AtEOXact_xml(); AtEOXact_on_commit_actions(true); AtEOXact_Namespace(true); /* smgrcommit already done */ *************** *** 2021,2026 **** --- 2024,2030 ---- AtEOXact_GUC(false, 1); AtEOXact_SPI(false); + AtEOXact_xml(); AtEOXact_on_commit_actions(false); AtEOXact_Namespace(false); smgrabort(); *************** *** 3851,3856 **** --- 3855,3861 ---- AtEOXact_GUC(false, s->gucNestLevel); AtEOSubXact_SPI(false, s->subTransactionId); + AtEOXact_xml(); AtEOSubXact_on_commit_actions(false, s->subTransactionId, s->parent->subTransactionId); AtEOSubXact_Namespace(false, s->subTransactionId, Index: src/backend/utils/adt/xml.c =================================================================== RCS file: /cvsroot/pgsql/src/backend/utils/adt/xml.c,v retrieving revision 1.67 diff -c -r1.67 xml.c *** src/backend/utils/adt/xml.c 12 Jan 2008 21:14:08 -0000 1.67 --- src/backend/utils/adt/xml.c 15 Jan 2008 01:26:02 -0000 *************** *** 24,41 **** */ /* ! * Note on memory management: Via callbacks, libxml is told to use ! * palloc and friends for memory management. Sometimes, libxml ! * allocates global structures in the hope that it can reuse them ! * later on, but if "later" is much later, the memory context ! * management of PostgreSQL will have blown those structures away ! * without telling libxml about it. Therefore, it is important to ! * call xmlCleanupParser() or perhaps some other cleanup function ! * after using such functions, for example something from ! * libxml/parser.h or libxml/xmlsave.h. Unfortunately, you cannot ! * readily tell from the API documentation when that happens, so ! * careful evaluation is necessary when introducing new libxml APIs ! * here. */ #include "postgres.h" --- 24,53 ---- */ /* ! * Notes on memory management: ! * ! * Via callbacks, libxml is told to use palloc and friends for memory ! * management, within a context that we reset at transaction end (and also at ! * subtransaction abort) to prevent memory leaks. Resetting at transaction or ! * subtransaction abort is necessary since we might have thrown a longjmp ! * while some data structures were not linked from anywhere persistent. ! * Resetting at transaction commit might not be necessary, but seems a good ! * idea to forestall long-term leaks. ! * ! * Sometimes libxml allocates global structures in the hope that it can reuse ! * them later on. Therefore, before resetting LibxmlContext, we must tell ! * libxml to discard any global data it has. The libxml API documentation is ! * not very good about specifying this, but for now we assume that ! * xmlCleanupParser() will get rid of anything we need to worry about. ! * ! * We use palloc --- which will throw a longjmp on error --- for allocation ! * callbacks that officially should act like malloc, ie, return NULL on ! * out-of-memory. This is a bit risky since there is a chance of leaving ! * persistent libxml data structures in an inconsistent partially-constructed ! * state, perhaps leading to crash in xmlCleanupParser(). However, as of ! * early 2008 it is *known* that libxml can crash on out-of-memory due to ! * inadequate checks for NULL returns, so this behavior seems the lesser ! * of two evils. */ #include "postgres.h" *************** *** 80,87 **** --- 92,102 ---- #ifdef USE_LIBXML static StringInfo xml_err_buf = NULL; + static MemoryContext LibxmlContext = NULL; static void xml_init(void); + static void xml_memory_init(void); + static void xml_memory_cleanup(void); static void *xml_palloc(size_t size); static void *xml_repalloc(void *ptr, size_t size); static void xml_pfree(void *ptr); *************** *** 784,802 **** text *data = PG_GETARG_TEXT_P(0); text *dtdOrUri = PG_GETARG_TEXT_P(1); bool result = false; ! xmlParserCtxtPtr ctxt = NULL; ! xmlDocPtr doc = NULL; ! xmlDtdPtr dtd = NULL; xml_init(); - /* We use a PG_TRY block to ensure libxml parser is cleaned up on error */ - PG_TRY(); - { xmlInitParser(); ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR, "could not allocate parser context"); doc = xmlCtxtReadMemory(ctxt, (char *) VARDATA(data), --- 799,814 ---- text *data = PG_GETARG_TEXT_P(0); text *dtdOrUri = PG_GETARG_TEXT_P(1); bool result = false; ! xmlParserCtxtPtr ctxt; ! xmlDocPtr doc; ! xmlDtdPtr dtd; xml_init(); xmlInitParser(); ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY, "could not allocate parser context"); doc = xmlCtxtReadMemory(ctxt, (char *) VARDATA(data), *************** *** 830,867 **** "validation against DTD failed"); #if 0 ! if (uri) ! xmlFreeURI(uri); ! uri = NULL; #endif ! if (dtd) ! xmlFreeDtd(dtd); ! dtd = NULL; ! if (doc) ! xmlFreeDoc(doc); ! doc = NULL; ! if (ctxt) ! xmlFreeParserCtxt(ctxt); ! ctxt = NULL; ! xmlCleanupParser(); ! } ! PG_CATCH(); ! { ! #if 0 ! if (uri) ! xmlFreeURI(uri); ! #endif ! if (dtd) ! xmlFreeDtd(dtd); ! if (doc) ! xmlFreeDoc(doc); ! if (ctxt) ! xmlFreeParserCtxt(ctxt); ! xmlCleanupParser(); ! ! PG_RE_THROW(); ! } ! PG_END_TRY(); PG_RETURN_BOOL(result); #else /* not USE_LIBXML */ --- 842,852 ---- "validation against DTD failed"); #if 0 ! xmlFreeURI(uri); #endif ! xmlFreeDtd(dtd); ! xmlFreeDoc(doc); ! xmlFreeParserCtxt(ctxt); PG_RETURN_BOOL(result); #else /* not USE_LIBXML */ *************** *** 915,920 **** --- 900,918 ---- } + /* + * xml cleanup function for transaction end. This is also called on + * subtransaction abort; see notes at top of file for rationale. + */ + void + AtEOXact_xml(void) + { + #ifdef USE_LIBXML + xml_memory_cleanup(); + #endif + } + + #ifdef USE_LIBXML /* *************** *** 953,966 **** xmlSetGenericErrorFunc(NULL, xml_errorHandler); /* Set up memory allocation our way, too */ ! xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup); /* Check library compatibility */ LIBXML_TEST_VERSION; - /* The above calls xmlInitParser(); must clean up dangling pointers */ - xmlCleanupParser(); - first_time = false; } else --- 951,961 ---- xmlSetGenericErrorFunc(NULL, xml_errorHandler); /* Set up memory allocation our way, too */ ! xml_memory_init(); /* Check library compatibility */ LIBXML_TEST_VERSION; first_time = false; } else *************** *** 977,983 **** * about, anyway. */ xmlSetGenericErrorFunc(NULL, xml_errorHandler); ! xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup); } } --- 972,978 ---- * about, anyway. */ xmlSetGenericErrorFunc(NULL, xml_errorHandler); ! xml_memory_init(); } } *************** *** 1210,1216 **** * 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, --- 1205,1211 ---- * 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, *************** *** 1219,1226 **** int32 len; xmlChar *string; xmlChar *utf8string; ! xmlParserCtxtPtr ctxt = NULL; ! xmlDocPtr doc = NULL; len = VARSIZE(data) - VARHDRSZ; /* will be useful later */ string = xml_text2xmlChar(data); --- 1214,1221 ---- int32 len; xmlChar *string; xmlChar *utf8string; ! xmlParserCtxtPtr ctxt; ! xmlDocPtr doc; len = VARSIZE(data) - VARHDRSZ; /* will be useful later */ string = xml_text2xmlChar(data); *************** *** 1234,1246 **** xml_init(); - /* We use a PG_TRY block to ensure libxml parser is cleaned up on error */ - PG_TRY(); - { xmlInitParser(); ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR, "could not allocate parser context"); if (xmloption_arg == XMLOPTION_DOCUMENT) --- 1229,1238 ---- xml_init(); xmlInitParser(); ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY, "could not allocate parser context"); if (xmloption_arg == XMLOPTION_DOCUMENT) *************** *** 1248,1254 **** /* * 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, --- 1240,1246 ---- /* * 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, *************** *** 1284,1305 **** doc->standalone = standalone; } ! if (ctxt) ! xmlFreeParserCtxt(ctxt); ! ctxt = NULL; ! xmlCleanupParser(); ! } ! PG_CATCH(); ! { ! if (doc) ! xmlFreeDoc(doc); ! if (ctxt) ! xmlFreeParserCtxt(ctxt); ! xmlCleanupParser(); ! ! PG_RE_THROW(); ! } ! PG_END_TRY(); return doc; } --- 1276,1282 ---- doc->standalone = standalone; } ! xmlFreeParserCtxt(ctxt); return doc; } *************** *** 1323,1334 **** /* * Wrappers for memory management functions */ static void * xml_palloc(size_t size) { ! return palloc(size); } --- 1300,1348 ---- /* + * Manage the special context used for all libxml allocations + */ + static void + xml_memory_init(void) + { + /* + * Create memory context if not there already. We make it a child of + * TopMemoryContext, even though our current policy is that it doesn't + * survive past transaction end, because we want to be really really + * sure it doesn't go away before we've called xmlCleanupParser(). + */ + if (LibxmlContext == NULL) + LibxmlContext = AllocSetContextCreate(TopMemoryContext, + "LibxmlContext", + ALLOCSET_DEFAULT_MINSIZE, + ALLOCSET_DEFAULT_INITSIZE, + ALLOCSET_DEFAULT_MAXSIZE); + + /* Re-establish the callbacks even if already set */ + xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup); + } + + static void + xml_memory_cleanup(void) + { + if (LibxmlContext != NULL) + { + /* Give libxml a chance to clean up dangling pointers */ + xmlCleanupParser(); + + /* And flush the context */ + MemoryContextDelete(LibxmlContext); + LibxmlContext = NULL; + } + } + + /* * Wrappers for memory management functions */ static void * xml_palloc(size_t size) { ! return MemoryContextAlloc(LibxmlContext, size); } *************** *** 1349,1355 **** static char * xml_pstrdup(const char *string) { ! return pstrdup(string); } --- 1363,1369 ---- static char * xml_pstrdup(const char *string) { ! return MemoryContextStrdup(LibxmlContext, string); } *************** *** 3262,3272 **** xmltype *data = PG_GETARG_XML_P(1); ArrayType *namespaces = PG_GETARG_ARRAYTYPE_P(2); ArrayBuildState *astate = NULL; ! xmlParserCtxtPtr ctxt = NULL; ! xmlDocPtr doc = NULL; ! xmlXPathContextPtr xpathctx = NULL; ! xmlXPathCompExprPtr xpathcomp = NULL; ! xmlXPathObjectPtr xpathobj = NULL; char *datastr; int32 len; int32 xpath_len; --- 3276,3286 ---- xmltype *data = PG_GETARG_XML_P(1); ArrayType *namespaces = PG_GETARG_ARRAYTYPE_P(2); ArrayBuildState *astate = NULL; ! xmlParserCtxtPtr ctxt; ! xmlDocPtr doc; ! xmlXPathContextPtr xpathctx; ! xmlXPathCompExprPtr xpathcomp; ! xmlXPathObjectPtr xpathobj; char *datastr; int32 len; int32 xpath_len; *************** *** 3363,3371 **** xpath_expr[xpath_len + 2] = '\0'; xpath_len += 2; - /* We use a PG_TRY block to ensure libxml parser is cleaned up on error */ - PG_TRY(); - { xmlInitParser(); /* --- 3377,3382 ---- *************** *** 3374,3380 **** */ ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_INTERNAL_ERROR, "could not allocate parser context"); doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0); if (doc == NULL) --- 3385,3391 ---- */ ctxt = xmlNewParserCtxt(); if (ctxt == NULL) ! xml_ereport(ERROR, ERRCODE_OUT_OF_MEMORY, "could not allocate parser context"); doc = xmlCtxtReadMemory(ctxt, (char *) string, len, NULL, NULL, 0); if (doc == NULL) *************** *** 3382,3388 **** "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) --- 3393,3399 ---- "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) *************** *** 3424,3430 **** (errmsg("could not create XPath object"))); xmlXPathFreeCompExpr(xpathcomp); - xpathcomp = NULL; /* return empty array in cases when nothing is found */ if (xpathobj->nodesetval == NULL) --- 3435,3440 ---- *************** *** 3445,3476 **** } xmlXPathFreeObject(xpathobj); - xpathobj = NULL; xmlXPathFreeContext(xpathctx); - xpathctx = NULL; xmlFreeDoc(doc); - doc = NULL; xmlFreeParserCtxt(ctxt); - ctxt = NULL; - xmlCleanupParser(); - } - PG_CATCH(); - { - if (xpathcomp) - xmlXPathFreeCompExpr(xpathcomp); - if (xpathobj) - xmlXPathFreeObject(xpathobj); - if (xpathctx) - xmlXPathFreeContext(xpathctx); - if (doc) - xmlFreeDoc(doc); - if (ctxt) - xmlFreeParserCtxt(ctxt); - xmlCleanupParser(); - - PG_RE_THROW(); - } - PG_END_TRY(); if (res_nitems == 0) PG_RETURN_ARRAYTYPE_P(construct_empty_array(XMLOID)); --- 3455,3463 ---- Index: src/include/utils/xml.h =================================================================== RCS file: /cvsroot/pgsql/src/include/utils/xml.h,v retrieving revision 1.22 diff -c -r1.22 xml.h *** src/include/utils/xml.h 1 Jan 2008 19:45:59 -0000 1.22 --- src/include/utils/xml.h 15 Jan 2008 01:26:02 -0000 *************** *** 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,