Commit 0c913c22081 for php.net
commit 0c913c22081d0c0ecc90afcc8f993c8c7449d1d2
Author: Ilia Alshanetsky <ilia@ilia.ws>
Date: Thu Sep 10 06:51:22 2026 -0400
dom: report the xpath callback node list to the cycle collector (#23596)
php_dom_xpath_callbacks_get_gc() traced php_ns and namespaces but not
node_list, which holds the nodes handed to and returned from php:function
callbacks, so a cycle running DOMXPath to node_list to node and back was
invisible to the collector. XSLTProcessor shares the registry and is
affected whenever a transform does not reach its own node list cleanup,
which a callback suspending a Fiber achieves. Reporting the list also makes
it reachable while being torn down, so clean_node_list() detaches the table
before destroying it; otherwise a node destructor calling
gc_collect_cycles() has the collector read and write entries that
zend_hash_destroy() already freed.
Closes GH-23596
diff --git a/NEWS b/NEWS
index bacb68f387d..4efc8258fc2 100644
--- a/NEWS
+++ b/NEWS
@@ -39,6 +39,9 @@ PHP NEWS
the null namespace in spec-following mode. (Ilia Alshanetsky)
. Fixed stale getElementsByClassName() and other node list caches after
className/classList writes and attribute removals. (Ilia Alshanetsky)
+ . Fixed reference cycles through DOMXPath and XSLTProcessor php:function
+ callback arguments and return values not being collectable.
+ (Ilia Alshanetsky)
- Hash:
. Fixed hash_file() reporting argument #1 ($algo) instead of argument #2
diff --git a/ext/dom/tests/DOMXPath_callback_node_list_gc.phpt b/ext/dom/tests/DOMXPath_callback_node_list_gc.phpt
new file mode 100644
index 00000000000..7e68224e390
--- /dev/null
+++ b/ext/dom/tests/DOMXPath_callback_node_list_gc.phpt
@@ -0,0 +1,28 @@
+--TEST--
+DOMXPath callback node list is reported to the cycle collector
+--EXTENSIONS--
+dom
+--FILE--
+<?php
+$doc = new DOMDocument();
+$doc->loadXML('<r><a/></r>');
+$xp = new DOMXPath($doc);
+$xp->registerNamespace('php', 'http://php.net/xpath');
+$xp->registerPhpFunctions();
+
+function cb($n) {
+ @$n[0]->back = $GLOBALS['the_xp'];
+ return true;
+}
+
+$GLOBALS['the_xp'] = $xp;
+$wr = WeakReference::create($xp);
+$xp->query('/r/a[php:function("cb", .)]');
+
+unset($xp, $GLOBALS['the_xp']);
+gc_collect_cycles();
+
+var_dump($wr->get() === null);
+?>
+--EXPECT--
+bool(true)
diff --git a/ext/dom/xpath_callbacks.c b/ext/dom/xpath_callbacks.c
index 6e8545fc65c..cb817b41dd3 100644
--- a/ext/dom/xpath_callbacks.c
+++ b/ext/dom/xpath_callbacks.c
@@ -53,9 +53,10 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_ctor(php_dom_xpath_callbacks *regist
PHP_DOM_EXPORT void php_dom_xpath_callbacks_clean_node_list(php_dom_xpath_callbacks *registry)
{
if (registry->node_list) {
- zend_hash_destroy(registry->node_list);
- FREE_HASHTABLE(registry->node_list);
+ HashTable *node_list = registry->node_list;
registry->node_list = NULL;
+ zend_hash_destroy(node_list);
+ FREE_HASHTABLE(node_list);
}
}
@@ -104,6 +105,12 @@ static void php_dom_xpath_callback_ns_get_gc(php_dom_xpath_callback_ns *ns, zend
PHP_DOM_EXPORT void php_dom_xpath_callbacks_get_gc(php_dom_xpath_callbacks *registry, zend_get_gc_buffer *gc_buffer)
{
+ if (registry->node_list) {
+ zval *entry;
+ ZEND_HASH_FOREACH_VAL(registry->node_list, entry) {
+ zend_get_gc_buffer_add_zval(gc_buffer, entry);
+ } ZEND_HASH_FOREACH_END();
+ }
if (registry->php_ns) {
php_dom_xpath_callback_ns_get_gc(registry->php_ns, gc_buffer);
}
@@ -117,7 +124,7 @@ PHP_DOM_EXPORT void php_dom_xpath_callbacks_get_gc(php_dom_xpath_callbacks *regi
PHP_DOM_EXPORT HashTable *php_dom_xpath_callbacks_get_gc_for_whole_object(php_dom_xpath_callbacks *registry, zend_object *object, zval **table, int *n)
{
- if (registry->php_ns || registry->namespaces) {
+ if (registry->php_ns || registry->namespaces || registry->node_list) {
zend_get_gc_buffer *gc_buffer = zend_get_gc_buffer_create();
php_dom_xpath_callbacks_get_gc(registry, gc_buffer);
zend_get_gc_buffer_use(gc_buffer, table, n);
diff --git a/ext/xsl/tests/xsltprocessor_callback_node_list_gc.phpt b/ext/xsl/tests/xsltprocessor_callback_node_list_gc.phpt
new file mode 100644
index 00000000000..1b24332d8de
--- /dev/null
+++ b/ext/xsl/tests/xsltprocessor_callback_node_list_gc.phpt
@@ -0,0 +1,50 @@
+--TEST--
+XSLTProcessor callback node list is reported to the cycle collector
+--EXTENSIONS--
+dom
+xsl
+--FILE--
+<?php
+$xml = new DOMDocument();
+$xml->loadXML('<root><a/></root>');
+
+$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:value-of select="php:function('hold', /root/a)"/><xsl:value-of select="php:function('pause')"/></out></xsl:template>
+</xsl:stylesheet>
+XSL);
+
+function hold(array $nodes): string {
+ @$nodes[0]->fiber = $GLOBALS['fiber'];
+ return 'h';
+}
+
+function pause(): string {
+ Fiber::suspend();
+ return 'p';
+}
+
+$proc = new XSLTProcessor();
+$proc->registerPHPFunctions();
+$proc->importStylesheet($xsl);
+$wr = WeakReference::create($proc);
+
+/* Suspending inside a callback leaves the transform without reaching the
+ node list cleanup, so the list still holds the node that owns the fiber. */
+$fiber = new Fiber(static function () use ($proc, $xml) {
+ $proc->transformToXml($xml);
+});
+$GLOBALS['fiber'] = $fiber;
+$fiber->start();
+
+var_dump($fiber->isSuspended());
+
+unset($proc, $fiber, $GLOBALS['fiber'], $xml, $xsl);
+gc_collect_cycles();
+
+var_dump($wr->get() === null);
+?>
+--EXPECT--
+bool(true)
+bool(true)
diff --git a/ext/xsl/tests/xsltprocessor_callback_node_list_gc_teardown.phpt b/ext/xsl/tests/xsltprocessor_callback_node_list_gc_teardown.phpt
new file mode 100644
index 00000000000..e8ecb3f3c0d
--- /dev/null
+++ b/ext/xsl/tests/xsltprocessor_callback_node_list_gc_teardown.phpt
@@ -0,0 +1,50 @@
+--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