On Fri, Aug 14, 2009 at 09:48:33AM +0200, Daniel Veillard wrote:
On Thu, Aug 13, 2009 at 06:12:38PM -0700, David Lutterkort wrote:
> We used to initialize libxslt (and register extension modules) every time
> ncf_init was called, and deregister/free libxslt globals every time
> ncf_close was called. That breaks when libnetcf is used together with other
> libraries that use libxslt, since they might continue using it after a call
> to ncf_close.
>
> To work around this, we now initialize and register our extension modules
> only once, essentially statically, and just live with the small leak that
> not unregistering and cleaning up globals causes.
>
> Bug report and suggested fix from Daniel Veillard (<veillard(a)redhat.com>)
Yes, thanks, the patch does what I think is needed to avoid threading
problems with libxslt in netcf, unfortunately the modules registered
globally can't be unplugged in ncf_close as other threads may still use
netcf and need those, same for the global variables of libxslt.
My suggestion is that applications using netcf should call
- xslt_ext_unregister(); <- or another public cleanup function
to be called when libnetcf is not used
anymore in the application
- xsltCleanupGlobals();
- xmlCleanupParser();
when they are about to exit, this will allow to have clean runs when
using valgrind --leak-check or other leak checking tools on the
application. The amount of memory left used is rather minimal, most of
the data like the stylesheet are still cleaned up on ncf_close()
the amount maintained by the libraries should be in the order of
a few kilobytes, neglectible by current standards.
Actually I think there is a nicer way to do this, by never
registering any global extensions, just registering the functions
on the transformation context. This mean you just call xsltInit()
from the initialization routine, and forget about any global
registration altogether, no need to add pthread specific changes
either, see patch attached. It also take care of one of your
TODOs i.e detect XSLT failures, you should just check that
return values from apply_stylesheet() are not NULL (left TODO).
Oh another cleanup is that you should not call
xmlDocDumpFormatMemory() on an XSLT output document but use
one of the xsltSaveResultTo...() function which will handle
the xsl:output parameters from the stylesheet, in the case of
drv_xml_desc() use xsltSaveResultToString()
http://xmlsoft.org/XSLT/html/libxslt-xsltutils.html#xsltSaveResultToString
not in that patch since it's orthogonal to the topic of this patch,
I think the patch works okay, the only thing missing is that
int register_exts(xsltTransformContextPtr ctxt)
should be exported internally in some .h but I wasn't sure where you
would prefer to do this... Compiles and passes regression tests for
me,
Daniel
--
Daniel Veillard | libxml Gnome XML XSLT toolkit
http://xmlsoft.org/
daniel(a)veillard.com | Rpmfind RPM search engine
http://rpmfind.net/
http://veillard.com/ | virtualization library
http://libvirt.org/