Free libxml2/libxslt resources in a safer order.

tglsfdc · tglsfdc · commit a1cd04cd0dd6 · 2014-11-27T11:12:55.000-05:00
Mark Simonetti reported that libxslt sometimes crashes for him, and that
swapping xslt_process's object-freeing calls around to do them in reverse
order of creation seemed to fix it.  I've not reproduced the crash, but
valgrind clearly shows a reference to already-freed memory, which is
consistent with the idea that shutdown of the xsltTransformContext is
trying to reference the already-freed stylesheet or input document.
With this patch, valgrind is no longer unhappy.

I have an inquiry in to see if this is a libxslt bug or if we're just
abusing the library; but even if it's a library bug, we'd want to adjust
our code so it doesn't fail with unpatched libraries.

Back-patch to all supported branches, because we've been doing this in
the wrong(?) order for a long time.
diff --git a/contrib/xml2/xslt_proc.c b/contrib/xml2/xslt_proc.c
@@ -150,16 +150,16 @@ xslt_process(PG_FUNCTION_ARGS)
 	}
 	PG_CATCH();
 	{
-		if (stylesheet != NULL)
-			xsltFreeStylesheet(stylesheet);
 		if (restree != NULL)
 			xmlFreeDoc(restree);
-		if (doctree != NULL)
-			xmlFreeDoc(doctree);
-		if (xslt_sec_prefs != NULL)
-			xsltFreeSecurityPrefs(xslt_sec_prefs);
 		if (xslt_ctxt != NULL)
 			xsltFreeTransformContext(xslt_ctxt);
+		if (xslt_sec_prefs != NULL)
+			xsltFreeSecurityPrefs(xslt_sec_prefs);
+		if (stylesheet != NULL)
+			xsltFreeStylesheet(stylesheet);
+		if (doctree != NULL)
+			xmlFreeDoc(doctree);
 		xsltCleanupGlobals();
 
 		pg_xml_done(xmlerrcxt, true);
@@ -168,11 +168,11 @@ xslt_process(PG_FUNCTION_ARGS)
 	}
 	PG_END_TRY();
 
-	xsltFreeStylesheet(stylesheet);
 	xmlFreeDoc(restree);
-	xmlFreeDoc(doctree);
-	xsltFreeSecurityPrefs(xslt_sec_prefs);
 	xsltFreeTransformContext(xslt_ctxt);
+	xsltFreeSecurityPrefs(xslt_sec_prefs);
+	xsltFreeStylesheet(stylesheet);
+	xmlFreeDoc(doctree);
 	xsltCleanupGlobals();
 
 	pg_xml_done(xmlerrcxt, false);
