Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set_exception_handler must catch exceptions also when in shutdown #10695

Closed
mvorisek opened this issue Feb 24, 2023 · 3 comments
Closed

set_exception_handler must catch exceptions also when in shutdown #10695

mvorisek opened this issue Feb 24, 2023 · 3 comments

Comments

@mvorisek
Copy link
Contributor

mvorisek commented Feb 24, 2023

Description

The following code:

click to expand
<?php

set_exception_handler(function (\Throwable $exception) {
    echo 'Caught: ' . $exception->getMessage() . "\n";
});

set_error_handler(static function (int $severity, string $msg, string $file, int $line): bool {
    echo 'Caught error: ' . $msg . "\n";
    return true;
});

register_shutdown_function(function () {
    trigger_error('warn_shutdown_1', \E_USER_WARNING);
});

register_shutdown_function(function () {
    trigger_error('warn_shutdown_2', \E_USER_WARNING);
});

register_shutdown_function(function () {
    throw new \Exception('ex_shutdown_1');
});

register_shutdown_function(function () {
    // expected to be never executed as after 1 uncaught exception php invokes shutdown,
    // after 1 uncaught exception in shutdown, php terminates
    throw new \Exception('ex_shutdown_2');
});

trigger_error('warn_regular_1', \E_USER_WARNING);
trigger_error('warn_regular_2', \E_USER_WARNING);

throw new \Exception('ex_regular');

https://3v4l.org/U5sF8

Resulted in this output:

Caught error: warn_regular_1
Caught error: warn_regular_2
Caught: ex_regular
Caught error: warn_shutdown_1
Caught error: warn_shutdown_2

Fatal error: Uncaught Exception: ex_shutdown_1 in /in/lXRcY:24
Stack trace:
#0 [internal function]: {closure}()
#1 {main}
  thrown in /in/lXRcY on line 24

But I expected this output instead:

Caught error: warn_regular_1
Caught error: warn_regular_2
Caught: ex_regular
Caught error: warn_shutdown_1
Caught error: warn_shutdown_2
Caught: ex_shutdown_1

restore_error_handler is working in shutdown, set_exception_handler must work too

PHP Version

any

Operating System

any

@Girgias
Copy link
Member

Girgias commented Feb 25, 2023

I don't think this makes any sense whatsoever.

It like you are expecting:

try {
    function_1_that_throw();
    function_2_that_throw();
} catch (\Throwable $e) {
    echo "Caught exception";
}

To execute both function_1_that_throw() and function_2_that_throw(). Which is clearly nonsensical.

But wondering what other people think about this @iluuu1994 @arnaud-lb @kocsismate .

@mvorisek
Copy link
Contributor Author

mvorisek commented Feb 25, 2023

@Girgias this issue is about set_exception_handler to work in register_shutdown_function - simpler demo https://3v4l.org/nHlKr

@Girgias
Copy link
Member

Girgias commented Feb 25, 2023

I still don't think this is particularly sensible, we are re-entering the VM, which may be in an unstable state. But I'll let other people judge as I don't deal with the shutdown part that much.

iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 21, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue Mar 23, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 5, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 25, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 25, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 25, 2023
iluuu1994 added a commit to iluuu1994/php-src that referenced this issue May 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants