Skip to content

Fix GH-22118: Compare equivalent fake closures in FCCs#22145

Open
prateekbhujel wants to merge 1 commit into
php:PHP-8.4from
prateekbhujel:prateekbhujel/fix-gh-22118-tick-fcc-php85
Open

Fix GH-22118: Compare equivalent fake closures in FCCs#22145
prateekbhujel wants to merge 1 commit into
php:PHP-8.4from
prateekbhujel:prateekbhujel/fix-gh-22118-tick-fcc-php85

Conversation

@prateekbhujel
Copy link
Copy Markdown
Contributor

@prateekbhujel prateekbhujel commented May 25, 2026

First-class method callable syntax creates a fresh fake Closure object for each expression. Some callback comparisons were still treating Closure-backed callables by object identity, so equivalent fake Closures could fail to unregister.

Move fake-Closure equivalence into zend_fcc_equals() so FCC users get the same comparison behavior, and reuse the same helper for SPL autoload's stored callback comparison. The regression coverage includes both a normal first-class method callable and a trampoline callable produced through __call().

Fixes GH-22118.

Tests:

  • sapi/cli/php run-tests.php -q -p sapi/cli/php ext/standard/tests/general_functions/gh22118.phpt ext/spl/tests/gh22118.phpt ext/spl/tests/gh10011.phpt ext/spl/tests/spl_autoload_013.phpt ext/spl/tests/spl_autoload_014.phpt ext/spl/tests/bug75049.phpt ext/spl/tests/spl_autoload_unregister_without_registrations.phpt ext/standard/tests/general_functions/bug66094.phpt

Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO this should be fixed in zend_fcc_equals() as this same issue would crop up if trying to register and unregister an autoloader using a FCC.

@prateekbhujel
Copy link
Copy Markdown
Contributor Author

Sorry, you’re right, this was too narrowly placed before. I moved the fake-Closure comparison into zend_fcc_equals() and reused the same helper for SPL autoload’s stored callback comparison, with a new spl_autoload_unregister() regression test for the FCC case you mentioned.

I pushed the updated branch and ran the focused tick + SPL autoload tests locally.

Copy link
Copy Markdown
Member

@Girgias Girgias left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Trampoline test and this needs to be backported to PHP 8.4 rather than 8.5 as this issue has existed for longer.

Comment thread ext/standard/tests/general_functions/gh22118.phpt
@prateekbhujel prateekbhujel force-pushed the prateekbhujel/fix-gh-22118-tick-fcc-php85 branch from 7c2e35c to b49ce7c Compare May 25, 2026 09:26
@prateekbhujel prateekbhujel changed the title Fix GH-22118: Unregister equivalent tick closures Fix GH-22118: Compare equivalent fake closures in FCCs May 25, 2026
@prateekbhujel prateekbhujel changed the base branch from PHP-8.5 to PHP-8.4 May 25, 2026 09:26
@prateekbhujel prateekbhujel requested a review from Girgias May 25, 2026 09:29
Comment thread Zend/zend_API.h
Comment on lines +765 to +768
ZVAL_OBJ(&closure_zv1, closure1);
ZVAL_OBJ(&closure_zv2, closure2);

return zend_compare_objects(&closure_zv1, &closure_zv2) == 0;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@TimWolla is this the best way to compare two closures objects?

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