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

window.hashchange events are unthrottled #32118

Open
shanehandley opened this issue Apr 20, 2024 · 1 comment
Open

window.hashchange events are unthrottled #32118

shanehandley opened this issue Apr 20, 2024 · 1 comment
Labels
C-untriaged New issues that haven't been triaged yet

Comments

@shanehandley
Copy link
Contributor

shanehandley commented Apr 20, 2024 •
edited

Describe the bug:
It's possible to prevent Servo from shutting down cleanly by defining an infinite loop in javascript based on event listeners.

Example:

<!DOCTYPE html>
<meta charset=utf-8>
<title>Servo HashChange loop</title>
<script>
window.onhashchange = function(e) {
    window.location.hash = Math.random().toString();
    console.log('hashchange', window.location.hash);
};

window.location.hash = ""
</script>

When executing this in Servo, the events are processed until attempting to exit, then there is a period where notify_history_changed errors are logged (with warnings on).

Chromium doesn't appear to log anything when this file is opened.

Firefox presents a throttle warning, via rate limiting:
Screenshot 2024-04-20 at 5 56 20 pm

Servo example:

output.mp4

To Reproduce:
Copy the above code to a html file and execute it with Servo with LOG_LEVEL warn.

Platform:
OSX/Sonoma 13.4.1

WPT Test
As an aside, this technique is used in a WPT test,.
@shanehandley shanehandley added the C-untriaged New issues that haven't been triaged yet label Apr 20, 2024
@gterzian
Copy link
Member

gterzian commented Apr 24, 2024 •
edited

a period where notify_history_changed errors are logged (with warnings on)

This originates at

servo/components/constellation/constellation.rs

Line 4532 in bef6c29

"notify_history_changed error after top-level browsing context closed."

Which I think happens in response to handling

servo/components/constellation/constellation.rs

Line 3676 in bef6c29

fn handle_navigated_to_fragment(

Interesting to note: we still have the pipeline, but the browsing context is already gone.

The NavigatedToFragment is sent from:

servo/components/script/dom/window.rs

Line 2232 in bef6c29

self.send_to_constellation(ScriptMsg::NavigatedToFragment(

Which is called into from

servo/components/script/dom/location.rs

Line 128 in bef6c29

.load_url(replacement_flag, reload_triggered, load_data);

load_url, after sending the message, queues a task to fire the hashchange event, and from that event load_url will be called again if the hash is changed.

Besides the non-existent rate-limiting, the question is why do we not handle the exit message from the constellation, apparently handling the task messages first?

The constellation exit workflow starts at

servo/components/constellation/constellation.rs

Line 2592 in bef6c29

fn handle_exit(&mut self) {
, not that this sends the exit pipeline message, but also first shuts down the background hang monitor, which should result in skipping task messages which should break the infinite loop(and I guess it does, but why so late?).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
C-untriaged New issues that haven't been triaged yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@shanehandley @gterzian