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

Need to be clearer about how aborted/terminated script evaluations are handled #715

Closed
bzbarsky opened this issue Feb 19, 2016 · 4 comments
Assignees
Labels
clarification Standard could be clearer

Comments

@bzbarsky
Copy link
Contributor

In https://html.spec.whatwg.org/multipage/webappapis.html#run-a-classic-script we have:

Let evaluationStatus be ScriptEvaluation(result).

and then the assertion is that if evaluationStatus is an abrupt completion then its [[value]] is "an error object". Ignoring for the moment that it might not be an object at all (c.f. throw 5), the problem is what happens if ScriptEvaluation gets aborted by something like worker termination or the slow script detection in browsers. In that situation we do NOT want to end up in step 10 at all, I believe. We should instead end up in step 11, but callers should be able to detect the abort condition somehow.

As a data point, the way this is implemented in Gecko right now is that ScriptEvaluation returns an abrupt completion with no [[value]] when script is aborted. Callers then know that this situation means an abort happened. Of course that state needs to be propagated back out as needed, but our ErrorResult struct has an explicit representation for this case (which we call an "uncatchable exception").

@domenic
Copy link
Member

domenic commented Feb 19, 2016

These have always been done in a very hand-wavey way by the spec. E.g. in #terminate-a-worker, "Abort the script currently running in the worker" is the best we get, and in #run-a-worker, we have the note

In addition to the usual possibilities of returning a value or failing due to an exception, this could be prematurely aborted by the "kill a worker" or "terminate a worker" algorithms defined below.

(this note was there before my recent changes).

But yeah, let's try to clean this up a bit. I think what would work is:

  • Define somewhere the term "abort a running script", and talk about how the JS spec doesn't really account for this, but it's a thing anyway. (I think there is already text to this effect somewhere that I just want to slap a dfn onto.)
  • Change "Let evaluationStatus be ScriptEvaluation(result)." To "Let evaluationStatus be ScriptEvaluation(result). If ScriptEvaluation does not complete because the user agent has [aborted the running script], skip to the step labeled cleanup." And similar for the run a module script section.
  • Make sure all the appropriate places in the spec (kill a worker, terminate a worker, also the slow script dialog clause?) link to "abort a running script".

An alternate approach would be to try to make aborting a ScriptEvaluation a more first-class thing in ES, similar to your uncatchable abrupt completion. But that could be done later, IMO.

@bzbarsky
Copy link
Contributor Author

That plan sounds fine to me.

@domenic domenic added the clarification Standard could be clearer label Feb 19, 2016
@domenic
Copy link
Member

domenic commented Feb 19, 2016

@bzbarsky do these uncatchable exceptions trigger finally blocks?

@bzbarsky
Copy link
Contributor Author

Good question. They do not.

domenic added a commit that referenced this issue Feb 19, 2016
Previously it wasn't clear exactly which parts of the processing model
the vague phrase "abort the script" impacted. This gives us a specific
term that at least outlines the concept, and makes all other places
that are impacted by script abortion reference that term.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clarification Standard could be clearer
Development

No branches or pull requests

2 participants