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

It's unclear how well browsers line up with the "Killing scripts" section of the spec #6210

Open
domenic opened this issue Dec 10, 2020 · 3 comments

Comments

@domenic
Copy link
Member

domenic commented Dec 10, 2020

https://html.spec.whatwg.org/#killing-scripts says

When a script exceeds a limit, the user agent may either throw a "QuotaExceededError" DOMException, abort the script without an exception, prompt the user, or throttle script execution.

where "abort the script" basically means "throw an uncatchable exception":

This causes any ScriptEvaluation or Source Text Module Record Evaluate invocations to cease immediately, emptying the JavaScript execution context stack without triggering any of the normal mechanisms like finally blocks.

I'm not sure what browsers actually do in these cases. Chrome seems to kill the entire process if the user chooses to do so after a prompt. That isn't allowed by this reading, but seems like a valid architectural choice. Firefox, back in 2016 at least (per #715), used an uncatchable exception, so I think falls into the "abort the script without an exception" category after user prompting. What about Safari?

Do any browsers differ in their treatment of module and classic scripts? <script> and import()?

The first step is to get clarity on existing behaviors. Myself and @syg are investigating for Chrome. @codehag, could you help with Firefox? Maybe @cdumez or @rniwa for Safari?

After that, read on to find out the details of what's messed up with the current spec.


The idea of throwing a "QuotaExceededError" DOMException is particularly interesting as it'd be observable. E.g. using code like

<script>
window.onerror = event => {
  console.log(event.error); // could log a "QuotaExceededError" DOMException
};
</script>

<script>
while (true) { }
</script>

Furthermore, by a strict reading that exception would be catchable (i.e., it's "thrown", not "reported"): i.e. in theory this could be done:

<script>
try {
  while (true) { }
} catch (e) {
  // This line could be reached with e as a "QuotaExceedError" DOMException
}
</script>

I doubt any browser does the latter. And probably none do the former either? If that's the case, I'd like to remove this potential complexity from the spec.


Worse, as we've made script execution more rigorous, and integrated more with ECMAScript, this all seems to have gotten more observable to web developers. (Which is probably not what implementations want.) In particular, consider run a classic script and run a module script.

I think the classic script case is OK: aborting the running script itself does nothing special when the script is aborted. It returns a "QuotaExceedError" DOMException to its callers, but the only caller that cares about its return value is javascript: URL processing, which just treats any exception the same as returning undefined. So, probably should be cleaned up to be less confusing, but it isn't broken.

Module scripts are worse though. Any aborts are noted in step 6.2, and will be translated into promises rejected with a "QuotaExceedError" DOMException. (That translation is buggy if the module script contains any awaits, as discovered in #4352 (comment).) That promise will then either be reported to window.onerror in step 7, or in the case of import(), be used as the import() return value.

I suspect implementations do not want to go through the trouble of plumbing "QuotaExceedError" DOMExceptions around for module scripts that get killed. At least, Chrome probably doesn't, given its "kill the whole process" approach. So probably this needs to be re-thought. But what should replace it depends on what browsers do.

@annevk
Copy link
Member

annevk commented Dec 10, 2020

Does Chrome do that on Android as well? I guess that again calls for "agent cluster crash" to be somewhat defined.

@hiroshige-g
Copy link
Contributor

Another case of aborting scripts (not killing scripts though) is Worker.terminate.
Although I'm not sure the QuotaExceededError is observable in this case in the spec, Blink does nothing (e.g. does not reject promises for dynamic import()s, doesn't crash the renderer) after aborting scripts, and V8 seems disallow accessing V8 APIs after aborting.

WPT:

@codehag
Copy link

codehag commented Dec 11, 2020

Re: the request to look at how firefox handles this, I will try to find some time soon to do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

No branches or pull requests

4 participants