Skip to content

Fix GH-21600: Remove xsltCleanupGlobals call in ext/xsl MSHUTDOWN.#21610

Open
devnexen wants to merge 1 commit intophp:PHP-8.4from
devnexen:gh21600
Open

Fix GH-21600: Remove xsltCleanupGlobals call in ext/xsl MSHUTDOWN.#21610
devnexen wants to merge 1 commit intophp:PHP-8.4from
devnexen:gh21600

Conversation

@devnexen
Copy link
Copy Markdown
Member

@devnexen devnexen commented Apr 2, 2026

The call to xsltCleanupGlobals() during module shutdown can cause a segfault in xmlHashFree() when freeing libxslt internal hash tables. This is the same class of shutdown cleanup issue that led to xmlCleanupParser() being removed from ext/libxml. The process is about to exit and the OS will reclaim all memory, making the explicit cleanup both unnecessary and harmful.

The call to xsltCleanupGlobals() during module shutdown can cause
a segfault in xmlHashFree() when freeing libxslt internal hash
tables. This is the same class of shutdown cleanup issue that led
to xmlCleanupParser() being removed from ext/libxml. The process
is about to exit and the OS will reclaim all memory, making the
explicit cleanup both unnecessary and harmful.
@devnexen devnexen marked this pull request as ready for review April 3, 2026 11:13
@devnexen devnexen requested a review from ndossche as a code owner April 3, 2026 11:13
@ndossche
Copy link
Copy Markdown
Member

ndossche commented Apr 3, 2026

It's a bit strange. I'd like to first better understand what is causing the segfault.
I'm assuming you were able to reproduce this. If so, can you run it under Valgrind (without this patch) and USE_ZEND_ALLOC=0 ?

@devnexen
Copy link
Copy Markdown
Member Author

devnexen commented Apr 3, 2026

I ran it under Valgrind with USE_ZEND_ALLOC=0 — valgrind is clean, but I wasn't able to reproduce the same circumstances as the original report (many shared
xml-related extensions loaded, Magento shutdown sequence).

That said, this is the same class of bug that was fixed in ext/libxml by 8742276 where xmlCleanupParser() was removed from
php_libxml_shutdown() for the same reason: "usage of xmlCleanupParser can affect the thread local storage, or even cause crashes in single threaded programs. On
shutdown the memory will be freed anyway."

xsltCleanupGlobals() is the xslt-side equivalent — it tears down global state (frees xsltFunctionsHash, xsltExtMutex, resets initialized = 0) that can be re-entered
by other extensions during shutdown, and the process is about to exit anyway.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants