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.

Sign up for GitHub

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

Jump to bottom

feature(engine): prevent shutdown events from delaying next page load #8061

Closed
wants to merge 1 commit into from
Closed

feature(engine): prevent shutdown events from delaying next page load #8061

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
27 changes: 24 additions & 3 deletions engine/lib/elgglib.php
View file
Open in desktop
Expand Up @@ -1426,7 +1426,31 @@ function _elgg_normalize_plural_options_array($options, $singulars) {
*/
function _elgg_shutdown_hook() {
global $START_MICROTIME;

elgg_set_config('shutdown_flag', true);
Copy link
Member

Choose a reason for hiding this comment

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

I'd prefer adding a new function elgg_has_shutdown(). Better would be to move this logic to just before shutdown is triggered. That way, if you're handling shutdown, you know no more output is allowed.

Copy link
Member

Choose a reason for hiding this comment

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

Ah I see this already does that. Then the CONFIG flag isn't needed

Copy link
Member Author

Choose a reason for hiding this comment

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

the config flag is useful when you have something triggering on an event.

eg. when someone joins a group -> do something
if the 'do something' is time consuming you might want to defer it to after shutdown, but if they joined the group as part of a shutdown script then you can't defer it.

pseudocode is

if (elgg_get_config('shutdown_flag')) {
    // do something now
}
else {
    // defer to after shutdown
}


$headers = headers_list();
$connection_sent = false;
foreach ($headers as $hdr) {
if (stripos($hdr, "Connection") === 0) {
$connection_sent = true;
}
}

if (!headers_sent() && !$connection_sent) {
// Ignore user aborts and allow the script to run forever
ignore_user_abort(true);
// Prevent an APC session bug: https://bugs.php.net/bug.php?id=60657
session_write_close();
Copy link
Member

Choose a reason for hiding this comment

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

We need docs that devs shouldn't write to session during shutdown. Better if there were a way to catch it.

set_time_limit(0);
// Tell the browser that we are done
header("Connection: close");
$size = ob_get_length();
header("Content-Length: $size");
ob_end_flush();
flush();
Copy link
Member

Choose a reason for hiding this comment

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

It would be nice if we could truly prevent more output. Like open a buffer that gets emptied before script end.

}

try {
elgg_trigger_event('shutdown', 'system');

Expand All @@ -1440,9 +1464,6 @@ function _elgg_shutdown_hook() {
error_log($message);
error_log("Exception trace stack: {$e->getTraceAsString()}");
}

// Prevent an APC session bug: https://bugs.php.net/bug.php?id=60657
session_write_close();
}

/**
Expand Down