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

Elizabeth Mattijsen <liz@xxxxxxxxxx> writes:

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

So do I :-)

>>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?

I'll check later, but the subroutine doesn't have to be completely
empty. It may for example contain some print and then return (). I
only wanted to point out, that it doesn't matter whether some of the
arguments appear on the output or not.

>>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).

Hm, I think I found the problematic lines. Check the attached patch
(without your module).

I don't say it is a fix, because I remove three lines the original
purpose of which I don't know. But the fact is, that your problematic
code passes valgrind now and also all self test pass (while before
10functions.t caused a sigsegv on my setup as well as somebody else's
from the list - most probably the same issue). That's a good knews,
isn't it :-)

I'm going to investigate if the removed lines had any meaning and if
yes, what should be done to preserve them.

> 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).

Yes. I'll check that later.

The patch is in the attachment.
--

LibXSLT.xs.patch
Description: Text Data