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