Pavel Stehule <pavel.steh...@gmail.com> writes:
> 2010/11/26 Itagaki Takahiro <itagaki.takah...@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
diff --git a/contrib/xml2/xpath.c b/contrib/xml2/xpath.c
index 0df7647..e92ab66 100644
*** a/contrib/xml2/xpath.c
--- b/contrib/xml2/xpath.c
*************** Datum xpath_table(PG_FUNCTION_ARGS);
*** 40,45 ****
--- 40,54 ----
void pgxml_parser_init(void);
+ /* workspace for pgxml_xpath() */
+
+ typedef struct
+ {
+ xmlDocPtr doctree;
+ xmlXPathContextPtr ctxt;
+ xmlXPathObjectPtr res;
+ } xpath_workspace;
+
/* local declarations */
static xmlChar *pgxmlNodeSetToText(xmlNodeSetPtr nodeset,
*************** static text *pgxml_result_to_text(xmlXPa
*** 51,57 ****
static xmlChar *pgxml_texttoxmlchar(text *textstring);
! static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath);
/*
--- 60,69 ----
static xmlChar *pgxml_texttoxmlchar(text *textstring);
! static xmlXPathObjectPtr pgxml_xpath(text *document, xmlChar *xpath,
! xpath_workspace *workspace);
!
! static void cleanup_workspace(xpath_workspace *workspace);
/*
*************** PG_FUNCTION_INFO_V1(xpath_nodeset);
*** 221,245 ****
Datum
xpath_nodeset(PG_FUNCTION_ARGS)
{
! xmlChar *xpath,
! *toptag,
! *septag;
! int32 pathsize;
! text *xpathsupp,
! *xpres;
!
! /* PG_GETARG_TEXT_P(0) is document buffer */
! xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
! toptag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! septag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(3));
! pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
! xpath = pgxml_texttoxmlchar(xpathsupp);
! xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath),
! toptag, septag, NULL);
pfree(xpath);
--- 233,254 ----
Datum
xpath_nodeset(PG_FUNCTION_ARGS)
{
! text *document = PG_GETARG_TEXT_P(0);
! text *xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
! xmlChar *toptag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! xmlChar *septag = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(3));
! xmlChar *xpath;
! text *xpres;
! xmlXPathObjectPtr res;
! xpath_workspace workspace;
! xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(document, xpath, &workspace);
! xpres = pgxml_result_to_text(res, toptag, septag, NULL);
! cleanup_workspace(&workspace);
pfree(xpath);
*************** PG_FUNCTION_INFO_V1(xpath_list);
*** 257,279 ****
Datum
xpath_list(PG_FUNCTION_ARGS)
{
! xmlChar *xpath,
! *plainsep;
! int32 pathsize;
! text *xpathsupp,
! *xpres;
!
! /* PG_GETARG_TEXT_P(0) is document buffer */
! xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
! plainsep = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
! xpath = pgxml_texttoxmlchar(xpathsupp);
! xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath),
! NULL, NULL, plainsep);
pfree(xpath);
--- 266,286 ----
Datum
xpath_list(PG_FUNCTION_ARGS)
{
! text *document = PG_GETARG_TEXT_P(0);
! text *xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
! xmlChar *plainsep = pgxml_texttoxmlchar(PG_GETARG_TEXT_P(2));
! xmlChar *xpath;
! text *xpres;
! xmlXPathObjectPtr res;
! xpath_workspace workspace;
! xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(document, xpath, &workspace);
! xpres = pgxml_result_to_text(res, NULL, NULL, plainsep);
! cleanup_workspace(&workspace);
pfree(xpath);
*************** PG_FUNCTION_INFO_V1(xpath_string);
*** 288,300 ****
Datum
xpath_string(PG_FUNCTION_ARGS)
{
xmlChar *xpath;
int32 pathsize;
! text *xpathsupp,
! *xpres;
!
! /* PG_GETARG_TEXT_P(0) is document buffer */
! xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
--- 295,307 ----
Datum
xpath_string(PG_FUNCTION_ARGS)
{
+ text *document = PG_GETARG_TEXT_P(0);
+ text *xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
xmlChar *xpath;
int32 pathsize;
! text *xpres;
! xmlXPathObjectPtr res;
! xpath_workspace workspace;
pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
*************** xpath_string(PG_FUNCTION_ARGS)
*** 305,317 ****
/* We could try casting to string using the libxml function? */
xpath = (xmlChar *) palloc(pathsize + 9);
- memcpy((char *) (xpath + 7), VARDATA(xpathsupp), pathsize);
strncpy((char *) xpath, "string(", 7);
xpath[pathsize + 7] = ')';
xpath[pathsize + 8] = '\0';
! xpres = pgxml_result_to_text(pgxml_xpath(PG_GETARG_TEXT_P(0), xpath),
! NULL, NULL, NULL);
pfree(xpath);
--- 312,327 ----
/* We could try casting to string using the libxml function? */
xpath = (xmlChar *) palloc(pathsize + 9);
strncpy((char *) xpath, "string(", 7);
+ memcpy((char *) (xpath + 7), VARDATA(xpathsupp), pathsize);
xpath[pathsize + 7] = ')';
xpath[pathsize + 8] = '\0';
! res = pgxml_xpath(document, xpath, &workspace);
!
! xpres = pgxml_result_to_text(res, NULL, NULL, NULL);
!
! cleanup_workspace(&workspace);
pfree(xpath);
*************** PG_FUNCTION_INFO_V1(xpath_number);
*** 326,346 ****
Datum
xpath_number(PG_FUNCTION_ARGS)
{
xmlChar *xpath;
- int32 pathsize;
- text *xpathsupp;
float4 fRes;
-
xmlXPathObjectPtr res;
!
! /* PG_GETARG_TEXT_P(0) is document buffer */
! xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
!
! pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(PG_GETARG_TEXT_P(0), xpath);
pfree(xpath);
if (res == NULL)
--- 336,352 ----
Datum
xpath_number(PG_FUNCTION_ARGS)
{
+ text *document = PG_GETARG_TEXT_P(0);
+ text *xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
xmlChar *xpath;
float4 fRes;
xmlXPathObjectPtr res;
! xpath_workspace workspace;
xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(document, xpath, &workspace);
!
pfree(xpath);
if (res == NULL)
*************** xpath_number(PG_FUNCTION_ARGS)
*** 348,353 ****
--- 354,361 ----
fRes = xmlXPathCastToNumber(res);
+ cleanup_workspace(&workspace);
+
if (xmlXPathIsNaN(fRes))
PG_RETURN_NULL();
*************** PG_FUNCTION_INFO_V1(xpath_bool);
*** 360,380 ****
Datum
xpath_bool(PG_FUNCTION_ARGS)
{
xmlChar *xpath;
- int32 pathsize;
- text *xpathsupp;
int bRes;
-
xmlXPathObjectPtr res;
!
! /* PG_GETARG_TEXT_P(0) is document buffer */
! xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
!
! pathsize = VARSIZE(xpathsupp) - VARHDRSZ;
xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(PG_GETARG_TEXT_P(0), xpath);
pfree(xpath);
if (res == NULL)
--- 368,384 ----
Datum
xpath_bool(PG_FUNCTION_ARGS)
{
+ text *document = PG_GETARG_TEXT_P(0);
+ text *xpathsupp = PG_GETARG_TEXT_P(1); /* XPath expression */
xmlChar *xpath;
int bRes;
xmlXPathObjectPtr res;
! xpath_workspace workspace;
xpath = pgxml_texttoxmlchar(xpathsupp);
! res = pgxml_xpath(document, xpath, &workspace);
!
pfree(xpath);
if (res == NULL)
*************** xpath_bool(PG_FUNCTION_ARGS)
*** 382,387 ****
--- 386,393 ----
bRes = xmlXPathCastToBoolean(res);
+ cleanup_workspace(&workspace);
+
PG_RETURN_BOOL(bRes);
}
*************** xpath_bool(PG_FUNCTION_ARGS)
*** 390,438 ****
/* Core function to evaluate XPath query */
static xmlXPathObjectPtr
! pgxml_xpath(text *document, xmlChar *xpath)
{
! xmlDocPtr doctree;
! xmlXPathContextPtr ctxt;
xmlXPathObjectPtr res;
xmlXPathCompExprPtr comppath;
- int32 docsize;
! docsize = VARSIZE(document) - VARHDRSZ;
pgxml_parser_init();
! doctree = xmlParseMemory((char *) VARDATA(document), docsize);
! if (doctree == NULL)
return NULL; /* not well-formed */
! ctxt = xmlXPathNewContext(doctree);
! ctxt->node = xmlDocGetRootElement(doctree);
/* compile the path */
comppath = xmlXPathCompile(xpath);
if (comppath == NULL)
{
! xmlFreeDoc(doctree);
xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
"XPath Syntax Error");
}
/* Now evaluate the path expression. */
! res = xmlXPathCompiledEval(comppath, ctxt);
xmlXPathFreeCompExpr(comppath);
if (res == NULL)
! {
! xmlXPathFreeContext(ctxt);
! xmlFreeDoc(doctree);
- return NULL;
- }
- /* xmlFreeDoc(doctree); */
return res;
}
static text *
pgxml_result_to_text(xmlXPathObjectPtr res,
xmlChar *toptag,
--- 396,456 ----
/* Core function to evaluate XPath query */
static xmlXPathObjectPtr
! pgxml_xpath(text *document, xmlChar *xpath, xpath_workspace *workspace)
{
! int32 docsize = VARSIZE(document) - VARHDRSZ;
xmlXPathObjectPtr res;
xmlXPathCompExprPtr comppath;
! workspace->doctree = NULL;
! workspace->ctxt = NULL;
! workspace->res = NULL;
pgxml_parser_init();
! workspace->doctree = xmlParseMemory((char *) VARDATA(document), docsize);
! if (workspace->doctree == NULL)
return NULL; /* not well-formed */
! workspace->ctxt = xmlXPathNewContext(workspace->doctree);
! workspace->ctxt->node = xmlDocGetRootElement(workspace->doctree);
/* compile the path */
comppath = xmlXPathCompile(xpath);
if (comppath == NULL)
{
! cleanup_workspace(workspace);
xml_ereport(ERROR, ERRCODE_EXTERNAL_ROUTINE_EXCEPTION,
"XPath Syntax Error");
}
/* Now evaluate the path expression. */
! res = xmlXPathCompiledEval(comppath, workspace->ctxt);
! workspace->res = res;
!
xmlXPathFreeCompExpr(comppath);
if (res == NULL)
! cleanup_workspace(workspace);
return res;
}
+ /* Clean up after processing the result of pgxml_xpath() */
+ static void
+ cleanup_workspace(xpath_workspace *workspace)
+ {
+ if (workspace->res)
+ xmlXPathFreeObject(workspace->res);
+ workspace->res = NULL;
+ if (workspace->ctxt)
+ xmlXPathFreeContext(workspace->ctxt);
+ workspace->ctxt = NULL;
+ if (workspace->doctree)
+ xmlFreeDoc(workspace->doctree);
+ workspace->doctree = NULL;
+ }
+
static text *
pgxml_result_to_text(xmlXPathObjectPtr res,
xmlChar *toptag,
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
