Re: XML::LibXSLT registered functions: a preliminary fix

(While in the middle of a reply to Robin, which will show up later in the list)

At 17:02 +0100 12/11/03, Petr Pajas wrote:
> I'm sorry for joining this discussion so late, I'm being too busy
> these days. I'd also like to thank Elizabeth for pointing this problem
> out.

You're welcome... I hope I can also be part of the solution...  ;-)


> Robin Berjon <robin.berjon@xxxxxxxxx> writes:
>  > I must have been sleeping or something. The twain can meet, at least
>  > somehow:
>  >    http://search.cpan.org/~rberjon/XML-XPathExt-0.01_01/XPathExt.pm
>  > That's a very very very alpha thing, it's there as a show of how it
> >  could be done. If XML::LibXSLT extensions stop blowing up on me, I'll
>  > be happy to finish and test it properly, and release a nicer version.
> I agree with Robin that the two modules are/should be able to share
> extension functions in the sense that a single perl sub can be
> registered as an extension function with any of them (although the
> registering API may slightly differ for those two modules). It means,
> that there should be no significant difference in how extension
> functions are written for XPathContext and LibXSLT.

I agree with that it would be nice.  However, at this point in time, 
my goal is to get it right for LibXSLT, as my customer is committed 
to that...


> ... The only real
> difference I know about is that ulike XPathContext, LibXSLT doesn't
> pass empty nodesets, so for example with LibXSLT, urn:foo(//para, 10)
> would call the Perl implementation subroutine with just @_=(10) if
> there was no node matching //para. It is obviously a bug and I'll
> patch it as soon as I'll find the spare 15 minutes to do it properly.

That indeed looks like a bug to me, yes...


> Also, it should be noted, that Liz's workaround is only needed for
> LibXSLT, not for XPathContext and I strongly discourage from using it
> for XPathContext because there is already something similar but less
> expensive implemented down on the XS level.

It only ever was intended for LibXSLT.


> I do appreciate Elizabeth's code (there are certaninly some nice
> points to learn from it and it will probably serve well enough as a
> temporary solution), but I still feel we should find a more direct
> solution. The first step to do so is to find the real cause of the
> problem, which isn't still obvious to me.

Agreed.


> Strangely enough, I'm able to reproduce the problem even when the
> extension function is completely empty: urn:foo { } In that case Liz's
> workaround doesn't even apply!

Please note that in 5.8.2, there is a bug with completely empty 
subroutines.  This may or may not be related to that.  Does this 
happen in < 5.8.2 also?


> In this case, the nodeset passed to the extension function originates
> from the inline XML code in XSLT.  It is most probably represented as
> XPATH_XSLT_TREE XPath object by libxslt (I'll have to check in a
> debugger). Each node is then cloned by LibXSLT before it is passed to
> Perl.

I don't think that's what I'm seeing: to test I just redefined 
XML::LibXML::Node::DESTROY and just had it display the refaddr of the 
object being destroyed.  As soon as the Perl sub is finished, the 
"cloned" node is destroyed, which apparently frees up libxml 
structures when it shouldn't (judging from the valgrind output).



> If Perl returns some nodes, they are clonned as well. Liz, you
> described the problem as:

My fix was only for nodes passed _to_ the Perl sub, _not_ when 
returned from it.  That seems to be fixed in the CVS version, albeit 
maybe with memory leaks (as Matt Sergeant seems to indicate).


>  >> I've traced this problem to DESTROY being called on the node that
> > >  is passed as a parameter to the function.  Since the node is part
> > >  of the stylesheet, the DESTROY starts deleting parts of the
>  >> stylesheet when it shouldn't.
> I don't understand two things here: why the destruction happens after
> the transformation and not at the end of the extension function call

Destruction happens at the end of the extension call...


> and why the nodes (which have been cloned by LibXSLT) are still part of
> the stylesheet.

Maybe they aren't cloned? (I'm on a limb here)


> One more note: The problem with the approach used in Liz's code is,
> that bypassing DESTROY will probably fix any segmentations faults
> because they usualy happen when free is called, which is most often in
> DESTROY. So it is a non-fix. The problem remains, it is just being
> masked (and code using this workaround will probably leak quite
> heavily :-().

Well, that's the odd thing.  If I run my sample program with 
XML::LibXSLT::Functions under valgrind with leak-checks enabled, I 
get:

==32189== 112 bytes in 1 blocks are definitely lost in loss record 12 of 30
==32189==    at 0x400296B2: malloc (vg_replace_malloc.c:153)
==32189==    by 0x80A1750: Perl_my_setenv (in 
/usr/local/bin/perl5.8.2-unthreaded)
==32189==    by 0x805FD91: perl_parse (in /usr/local/bin/perl5.8.2-unthreaded)
==32189==    by 0x805E257: main (in /usr/local/bin/perl5.8.2-unthreaded)
==32189==
==32189==
==32189== 124 bytes in 1 blocks are definitely lost in loss record 13 of 30
==32189==    at 0x400296B2: malloc (vg_replace_malloc.c:153)
==32189==    by 0x80A31EB: Perl_safesysmalloc (in 
/usr/local/bin/perl5.8.2-unthreaded)
==32189==    by 0x80947ED: Perl_pregcomp (in 
/usr/local/bin/perl5.8.2-unthreaded)
==32189==    by 0x80D1325: Perl_pp_regcomp (in 
/usr/local/bin/perl5.8.2-unthreaded)

which seem to indicate problems with Perl leaking itself, rather than 
anything libxml / XML::LibXML related.  The first leak also happens 
if you do:

  valgrind --leak-check=yes perl -e '1'

so that one should basically be ignored.  ;-)


> So anyone willing to help me with further investigation of this issue
> and finding a correct patch on LibXSLT is very welcome.

Check out my next mail...


Liz
_______________________________________________
Perl-XML mailing list
Perl-XML@xxxxxxxxxxxxxxxxxxxxxxxx
To unsubscribe: http://listserv.ActiveState.com/mailman/mysubs