Exiting during the WSoD shutdown handler prevents all other shutdown handlers from being called

[johnbillion]

When multiple shutdown handlers are registered, any shutdown handler that calls exit will immediately terminate processing and prevent subsequent shutdown handlers from being called.

Ref: ​https://secure.php.net/manual/en/function.register-shutdown-function.php

The newly introduced WSoD protection shutdown handler (WP_Shutdown_Handler::display_error_template()) calls wp_die() by default. If a wp-content/php-error.php drop-in is present, it loads that instead but still manually calls die().

The WSoD protection shutdown handler is registered very early on in wp-settings.php, which means that all subsequent shutdown handlers -- including the various other shutdown handlers in core, anything hooked onto the shutdown hook, and any shutdown handlers registered by plugins -- do not fire after a fatal error occurs, which they did prior to the introduction of the WSoD protection shutdown handler.

This can be demonstrated with the following code:

add_action( 'shutdown', function() {
    echo 'Goodbye, World!';
} );
function_that_does_not_exist();

Prior to the introduction of the WSoD protection shutdown handler, the callback attached to the shutdown action would be called and it would do its thing.

WordPress core has shutdown handlers registered for destructors for the wpdb and WP_Object_Cache classes, along with the shutdown_action_hook() function. This should all continue to fire after a fatal error.

The WSoD protection shutdown handler needs to be altered so it does not exit, either indirectly via wp_die() or directly via die().

[johnbillion]

It's pretty likely that this breaks plugins or libraries that log or otherwise process fatal errors. Sentry for PHP and the Query Monitor plugin have both broken.

[schlessera]

I'm currently investigating how to solve this. But first of all, I'd like to address the issue of the DB/Cache destructors mentioned above.

According to the PHP manual:

The destructor will be called even if script execution is stopped using exit(). Calling exit() in a destructor will prevent the remaining shutdown routines from executing.

So, while we do need to address the exit() we have in our WSOD protection, I don't understand why there are "shutdown handlers" to call the destructors in the first place. That seems like a PHP-4-era left-over. The entire point of having destructors is that you can rely on them being called.

Furthermore, the only thing the Core-provided destructors do is return true;. If you look at the signature from the PHP manual, one can see that this is a bug: destructors are not meant to return anything:

__destruct ( void ) : void

This makes perfect sense, as they are not meant to be directly called anyway.

So I suggest creating two follow-up bug tickets:

• Do not register shutdown handlers to call destructors explicitly
• Adapt wpdb::__destruct() and WP_Object_Cache::__destruct() signatures and code to have a return type of void

[schlessera]

I uploaded a patch that adds an 'exit' argument to wp_die() that defaults to true but that you can set to false. This allows us to reuse the complete response type handling (checking headers, response type, etc...) but just skip the die() at the very end. This way, all other shutdown handlers can still run in sequence after the WSOD protection.

As this allows additional random output to be generated after the wp_die() output, I also added an open div tag to display: none whatever comes next. This is of course debatable, not sure whether this is needed, or whether this is even a good approach to hide the subsequent dump.

@johnbillion Can you test whether this solves the issues with the two plugins you mentioned?

[schlessera]

(replaced with an updated version)

[flixos90]

46038.2.diff​ applies against the latest trunk changes and fixes a few issues with undefined variables or array keys. It furthermore simply calls return after the php-error.php drop-in inclusion, rather than die() or wp_die().

[flixos90]

In 44671:

Bootstrap/Load: Ensure that the fatal error shutdown handler does not prevent other shutdown handlers from being called.

This changeset adds support for a new wp_die() argument exit, which defaults to true and determines whether wp_die() should actually terminate the request. The new fatal error handler then calls wp_die() with that argument set to false, as calling die() or exit from a PHP shutdown function prevents other shutdown functions from being called.

Props schlessera, johnbillion.
Fixes #46038. See #44458.

[johnbillion]

In 44682:

Bootstrap/Load: Update the @since entry for wp_die().

See #46038

[flixos90]

In 44717:

Bootstrap/Load: Revert fatal error recovery mechanism from 5.1 to polish for 5.2.

Due to the high number of follow-up tickets and associated security concerns, it was decided to reschedule the fatal error recovery feature for WordPress 5.2, in order to address these issues properly. The feature will continue to be developed, with iterations being merged into trunk early in the 5.2 release cycle.

Fixes #46141. See #44458, #45932, #45940, #46038, #46047, #46068.