Layering: Refine execution context requirements for Jobs

[syg]

Closes #1930

This PR intentionally leaves the actual steps to ensure there's a usable execution context implementation/host-defined.

My philosophy here is to be as loose as we can be without tripping assertions.

Some nuances:

• The choice of Realm is loosely constrained, just that it's non-null. The particular Realm only matters when a Job schedules a non-user function to run. If the Job runs a user function, calling that function will push a new execution context with that function's Realm.
• The active script or module could be null. The requirement is that Jobs that evaluate user code should propagate the active script at the queueing time to the time when the Job is run. I don't know any real world examples where we must propagate forward a null active script, but I don't see any reasons in allowing that possibility to be conformant.

[devsnek]

I believe _realm_ is separate from the realm we need to worry about here. The current realm record when HostEnqueuePromiseJob is called needs to be the current realm record when the job is executed.

[syg]

The current realm record when HostEnqueuePromiseJob is called needs to be the current realm record when the job is executed.

There is no normative requirement for this.

[devsnek]

The job uses the current realm to create objects that are exposed to user code. How is that not a normative requirement?

[syg]

You're referring specifically to PromiseResolveThenableJob's use of CreateResolvingFunctions, yes?

There is a normative requirement that the resolving functions are created in some Realm, but not in the same Realm. I'd want the requirements on hosts to be as lax as we can make it without breaking things. Does creating the resolving functions in a different realm break anything?

(I do see that there is an ambiguity in the current PR that won't prepare to evaluate code if the thenable is a revoked proxy where realm is null. That case still needs to create the resolving functions, even though they become unobservable in that case. If you're operating on the assumption that all steps are observable all the time, the [[Realm]] assert in CreateBuiltinFunction will fail.)

[devsnek]

The specification, before the change that underspecified this, was very clear about using the current realm. We shouldn't change that, and I see zero benefit to saying "hosts can create objects in whatever random realm they want"

[devsnek]

Actually with a revoked proxy, the exception would be passed as a rejection to rejection handlers or rejection tracking, right?

[syg]

Good point on the TypeError that gets created in the PromiseThenableJob case; this PR needs to say PromiseThenableJobs need a valid Realm at all times.

[bakkot]

I think this PR is a strict improvement on the current spec as-is.

It is not clear to me if it makes sense to normatively require that the Realm of the top of the execution context stack when the job executes is the same as the Realm of the top of the execution context stack when the Host is invoked. This part of the interaction between specs/hosts/implementations is complicated enough that it is hard for me to be sure whether or not anyone actually wants to violate that potential normative requirement. It may have been required prior to the layering PR, but the point of the layering PR was that that code was not usable for most hosts, so I don't really care what it said.

So, for now, I would propose that we add a NOTE to the line this conversation is attached to which reads

Note: typically this results in the execution context stack having as its topmost item an execution context whose Realm component was the Realm component of the topmost item on the execution stack when HostEnqueuePromiseJob was invoked, although this is not a normative requirement

and amend that later if we become confident it should be a normative requirement.

[devsnek]

In the case of that TypeError, it would at least matter for V8's Error.prepareStackTrace API, since it is scoped by realm.

[syg]

Whatever V8 does is definitely not normative. We do need to say that a Realm must be present for the error, but that it must be the Realm of the active script at the time of queuing is undesirable for me, see #1984 (comment).

[devsnek]

what is "an execution"?

[syg]

A particular evaluation of ECMAScript code: what evaluation steps were taken, and if SABs were used, stuff related to the memory model. I'm open to other things that "prepared to execute code" can be applied to, executions (more precisely, execution prefixes since we're talking about a moment in time) made the most sense to me.

[syg]

Unrequesting review for now. Layering here is pretty messy, and this PR is currently incorrect wrt to devsnek's exception point, but also needs to think through more carefully where the lax Realm requirements is needed to see if there are other bugs.

Part of my motivation for pushing for lax Realm requirements is because of the incumbent settings object stack mess in HTML. Incumbents, most of the time, but not always, matches up with the Realm of active user code. But because it doesn't always, it seemed less headache all around to make the requirements on the ecma262 lax: if HTML needs to use the incumbent Realm as the executing context's Realm for some jobs some time instead of the active script's Realm at the time of queuing a Job, I don't see why ecma262 needs to care.

As someone who will most likely be doing the bulk of layering editorial reviews and some authoring for future APIs, extra constraints here are onerous in having to re-page in incumbent machinery.

[littledan]

This seems like a really good improvement.

I wonder if we could build on this concept of "prepared to evaluate code" to specify, once and for all, that hosts/implementations have to preserve the single-threaded nature of JS and not call in "re-entrantly". But I'm not sure how exactly to differentiate that from a fair-game synchronous callback; I don't have any particular wording in mind.

[syg]

I wonder if we could build on this concept of "prepared to evaluate code" to specify, once and for all, that hosts/implementations have to preserve the single-threaded nature of JS and not call in "re-entrantly". But I'm not sure how exactly to differentiate that from a fair-game synchronous callback; I don't have any particular wording in mind.

That's an interesting idea I haven't given much thought to yet. Specifying reentrancy in a more robust manner would be good all around. What's an example of a fair-game synchronous callback? You mean ones that don't "run script", whatever that might mean more formally?

[syg]

Motivations and explanations for the changes in ef0495a:

1. When the handler is not undefined in a PromiseReactionJob, or when the then function is not undefined in PromiseResolveThenableJob, the realm passed to HostEnqueuePromiseJob is never null. This used to be null for revoked Proxies, but it was pointed out that error creation for throwing the TypeError on revoked Proxies still needs to happen in some Realm. In that corner case, this PR uses the current Realm. Unfortunately implementations disagree here:
   1. Using the current Realm for revoked proxies matches what V8 does.
   2. SpiderMonkey uses the Realm of the then callable regardless whether it is a revoked Proxy. SpiderMonkey's behavior isn't speccable currently since the only objects the spec track creation Realms on are functions. cc @codehag for visibility.
   3. I don't know what Safari does.
2. The prepared to evaluate code property is now specified on Job Abstract Closures to be a precondition that must hold on every invocation.
3. The Realm requirement for the prepared to evaluate code property remains lax: it doesn't call for a specific Realm to be the current Realm at the time some Job Abstract Closure runs. This is web reality: HTML uses different Realms depending on if there's an active script to propagate, for instance.
4. Propagating the active script or module is split out to its own property on Job Abstract Closures. This is to leave the freedom for hosts to null out the active script on a job-by-job basis. No Jobs do this now but e.g. HTML nulls it out for some of its own callbacks, so this is a bit of futureproofing.

cc @domenic for visibility.

[syg]

Point 1 above is a spec bug in that it's been underspecified AFAICT both in 262 and in HTML and the current state is incoherent.

However, though we don't have interop, this PR isn't a normative change, since what Realm the job initially runs in has ultimately been at host discretion. So the V8, SM, and possibly Safari disagreement above is a web thing imo. The fact that SpiderMonkey uses some Realm that's unutterable in ecma262-ese doesn't really matter, since we don't have any normative requirement on the entry Realm for Promise Jobs anyway.

[syg]

@bakkot That note about the Realm being the same would've been good, but after poring over the settings object stacks in HTML again today, it's not even typically true. :(

To repeat some of what I commented above, the realm in the HostEnqueuePromiseJob is used by HTML for its entry settings object concept. By the time the abstract closure gets called, the entry settings object's Realm is the topmost on the stack iff there isn't an active script. If there's an active script to propagate for import.meta, then the active script's Realm is the topmost. More typically, this realm is never used since the handler function's [[Call]] will push a new execution context with the function's own Realm.

[bakkot]

It's not clear to me whether "whenever job is called" means here. A Job is just an abstract closure. It looks like this is trying to make an assertion about the ways in which the Job is invoked, but that's not really a property of the Job itself (as currently described).

Perhaps it would be better to speak of "an invocation of a Job" here? (If necessary, we could define that term above, by appending to "Such operations accept a Job Abstract Closure as the parameter and schedule it to be performed at some future time" the phrase "; this is called an invocation of the job".)

[syg]

Invocation sgtm.

[syg]

Ended up rephrasing step 2 as "Invoke the Job Abstract Closure" from "Call the Abstract Closure". The definition of Abstract Closure already talks about invocations, so I don't think we need to further define what "invocation of a job" means.

[syg]

Note that this PR is not designed to be gated on #1951 and uses "implementation-defined" to be more consistent with the current spec language. #1951 or this will be revised, whichever lands first.

[bakkot]

[devsnek]

This seems to work, though its a bit prose-y. I guess there isn't a clean way to specify it with more concrete algorithm steps?

[syg]

I guess there isn't a clean way to specify it with more concrete algorithm steps?

I don't think so, since we're trying to describe constraints instead of prescribing exact steps. Even if it were formatted as concrete steps, it'd still look like "Perform steps such that <constraint> holds", which seems only more verbose to me.

[syg]

Rebased

[ljharb]

This has the requisite consensus and reviews; I'll plan to land it later tonight or tomorrow unless there's any remaining feedback (cc @tc39/ecma262-editors)