Skip to content

Fix GH-21699: callable resolution must fail if error handler threw during self/parent/static deprecations#21712

Open
macoaure wants to merge 2 commits intophp:PHP-8.4from
macoaure:001-fix-shutdown-trampoline
Open

Fix GH-21699: callable resolution must fail if error handler threw during self/parent/static deprecations#21712
macoaure wants to merge 2 commits intophp:PHP-8.4from
macoaure:001-fix-shutdown-trampoline

Conversation

@macoaure
Copy link
Copy Markdown

This pull request addresses a bug (GH-21699) where an assertion failure could occur in shutdown_executor if an error handler throws an exception during the resolution of self::, parent::, or static:: callables. The fix ensures that if an exception is thrown by a user-defined error handler during callable resolution, the callable is not incorrectly reported as valid. The PR also adds comprehensive tests to cover these scenarios.

Bug fix for callable resolution:

  • Updated zend_is_callable_check_class in Zend/zend_API.c to check for exceptions after error handlers run, preventing invalid callables from being reported as valid if an exception was thrown.

Tests for error handler behavior:

  • Added Zend/tests/gh_21699.phpt to test self:: callable resolution when the error handler throws.
  • Added Zend/tests/gh_21699_parent.phpt to test parent:: callable resolution with throwing error handler.
  • Added Zend/tests/gh_21699_static.phpt to test static:: callable resolution with throwing error handler.

Documentation:

  • Updated NEWS to document the bug fix for assertion failures in shutdown_executor during callable resolution with throwing error handlers.

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.

As this is a fix please target the 8.4 branch.

To prevent requesting a review from everybody please first rebase onto 8.4, force push, and then change the target branch.

Comment on lines +3773 to +3776
/* User error handlers may throw from deprecations above; do not report callable as valid. */
if (UNEXPECTED(EG(exception))) {
return false;
}
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.

I think it makes more sense to have this be where the E_DEPRECATED are emitted. But then others may disagree so this is fine.

…after deprecations

When resolving string callables using self::, parent::, or static::,
zend_is_callable_check_class() emits E_DEPRECATED. If the user error
handler throws, EG(exception) is set but the function could still
return true, leading to trampoline allocation and a failed assertion
in shutdown_executor().

Return false from zend_is_callable_check_class() when EG(exception)
is set after handling.

Add regression tests for self::, parent::, and static:: forms.
@macoaure macoaure force-pushed the 001-fix-shutdown-trampoline branch from 4b775dd to 94efc8f Compare April 10, 2026 18:31
@macoaure macoaure changed the base branch from master to PHP-8.4 April 10, 2026 18:34
@macoaure
Copy link
Copy Markdown
Author

As this is a fix please target the 8.4 branch.

To prevent requesting a review from everybody please first rebase onto 8.4, force push, and then change the target branch.

Ok, sorry about that.

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