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

Send end trace on early shutdown #60

Merged
merged 5 commits into from
Mar 5, 2020
Merged

Send end trace on early shutdown #60

merged 5 commits into from
Mar 5, 2020

Conversation

joehoyle
Copy link
Member

@joehoyle joehoyle commented Mar 5, 2020

Currently if the script execution ends before the WordPress shutdown handler has been set up, we don't sent the end trace. This is a problem because Batcache (or any advanced-cache.php) HIT will be before the shutdown handler for WordPress has been set up.

We have to add our own shutdown handler in all cases, and only use it where appropriate.

Currently if the script execution ends before the WordPress shutdown handler has been set up, we don't sent the end trace. This is a problem because Batcache (or any advanced-cache.php) HIT will be before the shutdown handler for WordPress has been set up.

We have to add our own shutdown handler in all cases, and only use it where appropriate.
@joehoyle joehoyle requested review from rmccue and roborourke March 5, 2020 10:56
Copy link

@hm-linter hm-linter bot left a comment

Choose a reason for hiding this comment

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

Linting failed (1 error).

inc/namespace.php Outdated Show resolved Hide resolved
// It's possible the script is shutting down before register_shutdown_function( 'shutdown_action_hook' )
// has been called by WordPress. There's no way to check if this callback has been registered, so we use a heuristic that
// default-filters.php has not been loaded, which happens just before WordPress calls register_shutdown_function.
if ( has_action( 'wp_scheduled_delete', 'wp_scheduled_delete' ) === false ) {
Copy link
Member

Choose a reason for hiding this comment

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

function_exists( 'get_locale' ) would be a better heuristic; l10n.php is loaded immediately after the shutdown function.

As an alternative idea, could we call register_shutdown_function( 'shutdown_action_hook' ) ourselves? What happens when the same function is added twice?

Copy link
Member Author

@joehoyle joehoyle Mar 5, 2020

Choose a reason for hiding this comment

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

Yeah I didn't want to do that, as I think there might be a lot of functionality hooked into shutdown that cached-responses might not want.

Copy link
Member

@rmccue rmccue left a comment

Choose a reason for hiding this comment

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

YOLO I guess.

@joehoyle joehoyle merged commit f0939d2 into master Mar 5, 2020
@joehoyle joehoyle deleted the fix-early-shutdown branch March 5, 2020 11:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants