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

Elizabeth Mattijsen <liz@xxxxxxxxxx> writes:

> At 15:45 +0100 12/11/03, Robin Berjon wrote:
>>Elizabeth Mattijsen wrote:
>>> Well, I posted a message about this about 10 days ago or so: from
>>> what I understood there is that LibXSLT and XPathContext are two
>>> different modules, with two different approaches and never the
>>> twain shall meet.
>>I must have been sleeping or something.
>
> The message was posted on Nov 30, and titled "LibXSLT / XPathContext
> interaction".  You actually replied to it, so I guess you must have
> been asleep while typing...  ;-)
>
>
>>  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.
>
> Well, maybe we should try to get that together, although I must admit
> I don't think I'll be a user of XML::XPathExt.  But what I _do_ need
> is reliable registered Perl functions in XML::LibXSLT.  Ones that
> don't blow up and are reliable without (too much) leaking when used in
> mod_perl.
>
>
>>>> ...and that requires loading a module anyway so loading yours too
>>>> would be fine by me.
>>> Hmmm... on the other hand, maybe the whole "register_function"
>>> functionality should be taken out of the standard XML::LibXSLT
>>> module and put in XML::LibXSLT::Functions ?  Then you would have
>>> less to load if you're not using registered functions, and you
>>> would always have correctly functioning registered functions if you
>>> want to (by also using XML::LibXSLT::Functions).
>> That's certainly an option. Honestly I'd be so delighted to see this
>> work well that I have difficulty forming an opinion about which
>> place it would be best to put the functionality. I'm happy having to
>> dance around a bleeding chicken if it means I get extension
>> functions that work well ;)
>
> Hmmm...  images of BTVS appearing before my flu stricken eyes ;-)
>
> Ok, if I would create patches, where would I need to send them?   Here?
>
>
> Coming back to Petr Pajas remarks on LibXSLT / XPathContext interaction:
>
> At 16:57 +0100 12/4/03, Petr Pajas wrote:
>>If the extension function creates and returns new node(s), it goes as
>>follows
>>
>>1) the node it is referenced by Perl and LibXML's own refcounting
>>    machinery
>>
>>2) a XS wrapper gets the nodes from the Perl callback and wants to
>>    pass them to libxslt/xpath. But: unless some extra action is taken,
>>    Perl will try to free the nodes as soon as wrapper ends because the
>>    XS wrapper was the last one who holds a Perl reference to them
>>    (unless the app itself holds an extra reference).
>>
>>    There are two approaches:
>>
>>    Matt's way in LibXSLT: create a (deep) copy of the node, pass it to
>>    libxslt and let Perl/LibXML free the created node in the usual
>>    way. Problem: the copy looses its parent node as well as all its
>>    relation to other nodes within the nodeset, document, etc. It is
>>    basically a very different node (although it looks quite the same).
>
> Seeing how good my approach with XML::LibXSLT::Functions works, I'm
> wondering whether this approach shouldn't work for nodesets returned
> by Perl functions.
>
> I probably am looking at this way too simplisiticly, but I see three
> situations:
>
> 1. nodesets from the stylesheet passed as parameters to the Perl function:
>
> These have appropriate XML::LibXML::xxx objects created "around" the
> libxml nodeset.  However, when these objects go out of scope, DESTROY
> should _not_ be called on them (this is what XML::LibXSLT::Functions
> is now doing).

Not quite correct. Currently, LibXSLT makes copies of those nodes,
those are passed to the extension function and *should* be freed at
the end (unless there is some perl reference to them left - e.g. a
global variable created by the ext-func). What LibXSLT did wrong, as
it seems to me now, was wrapping them with wrong LibXML-ref-counting
proxy objects. Those proxy objects contained some pointers from the
stylesheet or the original document and were thus wrongly treated by
DESTROY (I'm probably not 100% correct here too, but my last patch
shows I'm not too far from correct). So the only problem is to make
the clones correctly. If that is ensured, this part should work
perfectly without hacking DESTROY.


> 2. nodesets that are created inside the Perl function and returned
>
> The problems with these nodesets is that these go out of scope before
> they can be handled by the Perl function caller.  I see two solutions
> to this:
>
> a. create a special routine for returning values
>
> so instead of:
>
>     return $nodes;
>
> you would have to say:
>
>     LibXSLT::return( $nodes );
>
> The advantage of this would be that $nodes would still be in scope,
> and that it could be registered with the same method as I created with
> XML::LibXSLT::Functions.
>
> b. do some XS trickery
>
> I'm pretty sure there must be some Hook::LexWrap / ENTER / ENTER XS
> magic that should extend the scope of the local variables of the Perl
> extension function to a surrounding wrapper subroutine.

Yes there is, but you probably on can operate with this within one
function, which is where the problem is. You have to: 1st unbind them
from the perl and LibXML refcounting mechanisms (but only if they are
not refered from elsewhere - ie. if they actually would be destroyed
by DESTROY), which is of possible to implement (although tricky),
2nd return them back to libxslt and unless you want leeks, say libxslt
to free them for you (but only if they are not referenced - also
implementable but tricky).

The best way to see how LibXML's referencing works is to look at
PmmREFCNT_dec in perl-libxml-mm.c. There could be a similar subroutine
which would do exactly the same but wouldn't actually free anything
but would only say if the PmmREFCNT reached zero or not. This function
could be used for the above mentioned "tricky" tests.

> 3. nodesets that are created and discarded inside the Perl extension function
>
> These should have their DESTROY called on them, because the libxml
> structures they refer to, are not needed anymore (is this assumption
> correct?  Or is this indeed the can of worms that is causing all the
> problems?).

You're right, these nodes behave well, there is no problem with them.
There is also no problem with nodes, that are created within an
extension function and left in a global variable for later purposes.

> I need to get a good working registered function implementation by the
> end of the year.  It may leak (if it has to), but it may not segfault.
> Ever.  That's my personal goal.
>
> Liz

You mean this year? ;-)

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