diff --git a/NEWS b/NEWS
index 98c8d3b3106b..f6a77614861e 100644
--- a/NEWS
+++ b/NEWS
@@ -21,6 +21,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 000000000000..7e68224e390e
--- /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--
+loadXML('');
+$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 349c304c9f25..a9c4c194f3b7 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);
}
}
@@ -100,6 +101,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);
}
@@ -113,7 +120,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 000000000000..1b24332d8dea
--- /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--
+loadXML('');
+
+$xsl = new DOMDocument();
+$xsl->loadXML(<<
+
+
+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 000000000000..e8ecb3f3c0de
--- /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--
+loadXML('');
+$xml->registerNodeClass(DOMElement::class, GcElement::class);
+
+$xsl = new DOMDocument();
+$xsl->loadXML(<<
+
+
+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--
+
+abcdefgh
+done