Tom Lane escribió:

> We might be able to compromise by only resetting the context after
> an error, but this is still only possible if we have a way to make
> libxml let go of *all* pointers to alloc'd objects.  I don't understand
> your comment that xmlCleanupParser solves it --- we call that already,
> and it doesn't seem to be preventing the problem.

With the attached patch, it doesn't crash, but I see the added WARNING
four times in the log, which is proof that the cleanup thing is not
called as the code seems to think.

I wonder -- is this thing supposed to be reentrant?  I think that's the
whole problem with it.

(I think what I'm doing in xml_init in the non-first case is bogus
anyway -- but I post the patch to show my point.)

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
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 00:37:32 -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);
*************** xmlvalidate(PG_FUNCTION_ARGS)
*** 844,849 ****
--- 845,852 ----
  			xmlFreeParserCtxt(ctxt);
  		ctxt = NULL;
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  	}
  	PG_CATCH();
  	{
*************** xmlvalidate(PG_FUNCTION_ARGS)
*** 858,863 ****
--- 861,868 ----
  		if (ctxt)
  			xmlFreeParserCtxt(ctxt);
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  
  		PG_RE_THROW();
  	}
*************** xml_init(void)
*** 953,958 ****
--- 958,969 ----
  		xmlSetGenericErrorFunc(NULL, xml_errorHandler);
  
  		/* Set up memory allocation our way, too */
+ 		Assert(LibxmlContext == NULL);
+ 		LibxmlContext = AllocSetContextCreate(PortalContext,
+ 											  "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,979 ****
--- 985,998 ----
  		 * about, anyway.
  		 */
  		xmlSetGenericErrorFunc(NULL, xml_errorHandler);
+ 		if (LibxmlContext == NULL)
+ 			LibxmlContext = AllocSetContextCreate(PortalContext,
+ 												  "LibxmlContext",
+ 												  ALLOCSET_DEFAULT_MINSIZE,
+ 												  ALLOCSET_DEFAULT_INITSIZE,
+ 												  ALLOCSET_DEFAULT_MAXSIZE);
+ 		else
+ 			elog(WARNING, "LibxmlContext already exists");
  		xmlMemSetup(xml_pfree, xml_palloc, xml_repalloc, xml_pstrdup);
  	}
  }
*************** xml_parse(text *data, XmlOptionType xmlo
*** 1285,1290 ****
--- 1304,1311 ----
  			xmlFreeParserCtxt(ctxt);
  		ctxt = NULL;
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  	}
  	PG_CATCH();
  	{
*************** xml_parse(text *data, XmlOptionType xmlo
*** 1293,1298 ****
--- 1314,1321 ----
  		if (ctxt)
  			xmlFreeParserCtxt(ctxt);
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  
  		PG_RE_THROW();
  	}
*************** xml_text2xmlChar(text *in)
*** 1325,1331 ****
  static void *
  xml_palloc(size_t size)
  {
! 	return palloc(size);
  }
  
  
--- 1348,1354 ----
  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);
  }
  
  
--- 1369,1375 ----
  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)
--- 3391,3397 ----
  						"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)
*************** xpath(PG_FUNCTION_ARGS)
*** 3439,3444 ****
--- 3462,3469 ----
  		xmlFreeParserCtxt(ctxt);
  		ctxt = NULL;
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  	}
  	PG_CATCH();
  	{
*************** xpath(PG_FUNCTION_ARGS)
*** 3453,3458 ****
--- 3478,3485 ----
  		if (ctxt)
  			xmlFreeParserCtxt(ctxt);
  		xmlCleanupParser();
+ 		MemoryContextDelete(LibxmlContext);
+ 		LibxmlContext = NULL;
  
  		PG_RE_THROW();
  	}
---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?

               http://archives.postgresql.org

Reply via email to