Restore exception handler after running it

[iluuu1994]

Symfony relies on finding the exception handler in the handler stack. There's currently no clean API to find it, so they pop all the handlers, and push them again once the stack is empty. This PR attempts to minimize the BC break by pushing the current handler onto the stack and clearing the current handler, and restoring it once it has finished. This is essentially equivalent to set_exception_handler(null) and restore_exception_handler().

restore_exception_handler() however is only called if the exception handler is still unset. If the handler has pushed a new handler in the meantime, we assume it knows what it's doing.

Fixes GH-13446

/cc @nicolas-grekas Does this solve your problem?

[nicolas-grekas]

👍 makes sense to me thanks.

[dstogov]

I didn't understand the test (it behaves as expected without the patch) and its relation to the bug report.

[iluuu1994]

Right, this still needs a better test. I'll add one later today. This test was added to make sure an executing closure doesn't get released prematurely (I wasn't sure about this behavior when implementing this, so I made sure it didn't break).

[lavarou]

This change is causing this code to recurse infinitely in PHP 8.3.5+: https://3v4l.org/Hct7c. Similar construct is used in Yii's exception handler here. Is that a desired behavior? On the other hand, this code https://3v4l.org/ARJ9A works like I would expect it to on 8.3.0-8.3.4, doesn't work like I would expect on PHPs < 8.3, but causes infinite recursion on 8.3.5+. Only this code https://3v4l.org/NNnEA seems to work consistently for all PHPs 8.3.x.

[iluuu1994]

@lavarou Yes. Since PHP 8.3, calling the error handler involves replacing the error handler with null, and calling the error handler. This ensures it won't be called recursively when the error handler is itself faulty. If the error handler itself restores the previous handler or pushes the new one, you can run into recursion.

What's the reason for calling restore_exception_handler()? It had no effect previously, in <PHP-8.3.

Looking at Yiis comment:

disable error capturing to avoid recursive errors while handling exceptions

I don't think that was ever a concern. Previously, PHP just didn't call error handlers at all after a fatal error has already occurred. Edit: I see it also disables the error handler, which might be the more relevant part.

[lavarou]

@iluuu1994 Thank you for your prompt response and providing more details.

What's the reason for calling restore_exception_handler()? It had no effect previously, in <PHP-8.3.

I cannot answer this question as I'm neither Yii user nor Yii developer. I just wanted to provide some examples of the PHP code where the changes around exception handling in PHP 8.3 demonstrate different behaviors between versions in case any future PHP developer will be curious how things work and what to expect.

[iluuu1994]

Thanks. The change to call the exception handler again (when re-registered) was made in PHP 8.3. However, it did not actually store the called error handler on the stack, it was just discarded. This was later improved in PHP 8.3.5, so that restoring the error handler would pop the current error handler in-place again.

I did not anticipate this breaking any code. However, I think adjusting this again would just further risk breaking more code, so I would prefer keeping things as-is.