|Subject:||register_element is broken|
|X-Mailer:||MIME-tools 5.504 (Entity 5.504)|
|Message-ID:||<rt-4.0.18-14116-1394205223-92.0-0-0 [...] rt.cpan.org>|
The implementation $stylesheet->register_element has some serious problems. Currently, the node returned fron the Perl callback is inserted into the result document using: x_PmmREFCNT_inc(PmmPROXYNODE(result)); xmlAddChild(ctxt->insert, result); 1. This leaks memory since the ref count will not be decreased. 2. The ownership of the node isn't transferred. This means that destruction of the result document can destroy the (libxml2) node even if it still has references from Perl space. Possible solutions: 1. Deep clone the node before inserting (slow). 2. Transfer ownership of the node to the new document directly using PmmFixOwner. This function has to be copied from XML::LibXML. 3. Transfer ownership indirectly by creating XML::LibXML::Node objects for the parent and child node and invoking the addChild method on the parent (complicated). IMO, the ultimate solution would be to rework the register_element interface. It would be more useful to expose the ctxt->insert node and let the Perl callback insert the nodes. This would solve the ownership problem and allow the insertion of multiple nodes which isn't possible with the current implementation. But it would break backward compatibility. OTOH, I doubt that this feature of XML::LibXLST is used at all.