dom: report the DOMXPath callback node list to the cycle collector

[iliaal]

php_dom_xpath_callbacks_get_gc() traced php_ns and namespaces but not node_list, which holds the nodes passed to and returned from php:function callbacks. A cycle from a DOMXPath through node_list to a node and back to the same DOMXPath was invisible to the collector, so it was never freed.

node_list is still cleared at object destruction rather than at the end of each evaluation. Clearing it eagerly is not safe, because a callback can run a nested query()/evaluate() on the same object and the inner call would free wrappers the outer evaluation is still using.

[devnexen]

can the following be tested ?

--TEST--
XSLTProcessor: cycle collection triggered while the php:function node list is torn down
--EXTENSIONS--
dom
xsl
--FILE--
<?php
class GcElement extends DOMElement
{
    public function __destruct()
    {
        gc_collect_cycles();
    }
}

$xml = new DOMDocument();
$xml->loadXML('<root><a/><b/><c/><d/><e/></root>');
$xml->registerNodeClass(DOMElement::class, GcElement::class);

$xsl = new DOMDocument();
$xsl->loadXML(<<<XSL
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:php="http://php.net/xsl">
<xsl:template match="/"><out><xsl:for-each select="/root/*"><xsl:value-of select="php:function('cb', .)"/></xsl:for-each></out></xsl:template>
</xsl:stylesheet>
XSL);

function cb(array $nodes): string
{
    return $nodes[0]->nodeName;
}

$proc = new XSLTProcessor();
$proc->registerPHPFunctions();
$proc->importStylesheet($xsl);

$root_buffer = $proc;
unset($root_buffer);

echo $proc->transformToXml($xml);
echo 'done', PHP_EOL;
?>
--EXPECT--
<?xml version="1.0"?>
<out xmlns:php="http://php.net/xsl">abcde</out>
done

[iliaal]

That exact test passes with and without the patch, so it wouldn't pin anything: transformToXml() clears the node list at the end of every transform (xsltprocessor.c:419), so a completed transform leaves nothing to close a cycle through.

It is reachable though, just not via a completed transform. A callback that suspends a Fiber never reaches that cleanup, and then the processor leaks on unpatched master. Added ext/xsl/tests/xsltprocessor_callback_node_list_gc.phpt doing exactly that, red before the patch and green after.

[devnexen]

Ok let s try this one out (debug build)

--TEST--
XSLTProcessor: cycle collection triggered while the php:function node list is torn down
--EXTENSIONS--
dom
xsl
--FILE--
<?php
class GcElement extends DOMElement
{
    private static int $destroyed = 0;

    public function __destruct()
    {
        /* Collect once the node list teardown has already freed some entries. */
        if (++self::$destroyed === 3) {
            gc_collect_cycles();
        }
    }
}

$xml = new DOMDocument();
$xml->loadXML('<root><a/><b/><c/><d/><e/><f/><g/><h/></root>');
$xml->registerNodeClass(DOMElement::class, GcElement::class);

$xsl = new DOMDocument();
$xsl->loadXML(<<<XSL
<xsl:stylesheet version="1.0" xmlns:xsl="http://www.w3.org/1999/XSL/Transform" xmlns:php="http://php.net/xsl">
<xsl:template match="/"><out><xsl:for-each select="/root/*"><xsl:value-of select="php:function('cb', .)"/></xsl:for-each></out></xsl:template>
</xsl:stylesheet>
XSL);

function cb(array $nodes): string
{
    return $nodes[0]->nodeName;
}

$proc = new XSLTProcessor();
$proc->registerPHPFunctions();
$proc->importStylesheet($xsl);

$root_buffer = $proc;
unset($root_buffer);

echo $proc->transformToXml($xml);
echo 'done', PHP_EOL;
?>
--EXPECT--
<?xml version="1.0"?>
<out xmlns:php="http://php.net/xsl">abcdefgh</out>
done

[iliaal]

That one bites. Valgrind shows gc_mark_grey() reading and writing entries that zend_hash_destroy() had already freed, reached through the new get_gc while clean_node_list() was still mid-teardown. Detached the table before destroying it, and added your test as ext/xsl/tests/xsltprocessor_callback_node_list_gc_teardown.phpt.