Layering: Allow hosts to cleanly specify Promise Job semantics #1597

littledan · 2019-06-20T14:18:13Z

Replace the EnqueueJob abstract operation with a HostEnqueuePromiseJob operation, responsible for asking the host to schedule the PromiseJob, and run it in a way that preserves the run-to-completion nature of JavaScript. As it is a host hook and not an algorithm, the invariants are specified declaratively, rather than imperatively as previously.

This PR implements the conclusion of the discussion in the June 2019 TC39 discussion on Promise scheduling and host layering.

Replaces #735

cc @erights

domenic

LGTM with nits. Very excited about this, and how it will also help fix whatwg/html#1426. <3 <3 <3

domenic · 2019-06-21T01:04:41Z

spec.html

+
+      <p>HostEnqueuePromiseJob is a host-defined abstract operation that schedules the abstract operation indicated by _job_ to be performed, with the arguments _arguments_, at some future time. The Jobs used with this algorithm are intended to be related to the handling of Promises, or otherwise, to be scheduled with equal priority to Promise handling operations.</p>
+
+      <p>The implementation of HostEnqueuePromiseJob must conform to the requirements in <emu-xref href="#sec-jobs-and-job-queues"></emu-xref>. Additionally, Jobs must be scheduled in FIFO order, with Jobs running in the same order as the HostEnqueuePromiseJob invocations which scheduled them.</p>


"Jobs must be scheduled in FIFO order" seems like a general requirement that belongs in the previous section (and I think the previous section already contains it, in different words?).

The motivation for putting it in this section was that not all host job hooks will use it. For example, I don't think we want this to apply to WeakRef finalization callbacks. However, we will want it to apply to Atomics.waitAsync.

The above general requirements state

The evaluation of any Jobs enqueueed via this or other host job scheduling algorithms must complete before evaluation of the Job starts.

This seems like the same requirement, applied to all jobs?

I think the above requirement is more about run-to-completion (which applies to all host job hooks), and this requirement is about FIFO ordering (which only applies to some of them).

I think I can see that reading now. Any jobs that have not yet started, don't trigger that clause.

In that case I think that clause should be deleted, since it comes right before an IMO-clearer statement:

Once evaluation of a Job starts, it must run to completion before evaluation of any other Job starts.

OK, deleted; I agree that it's a bit redundant.

This still hasn’t been deleted; is a new diff expected?

What used to be at 6439 was deleted, following @domenic's recommendation.

Does the phrase “in FIFO order” need to appear? It seems redundant with the prose immediately following it.

It's slightly redundant for clarity. FIFO was present in the previous text, and the FIFO ordering was also clear from the RunJobs algorithm, so the wording here is analogous to that. If it's a big deal for you, I could move part of it to a note, but I think it's OK to have a single sentence with words that agree with each other.

spec.html

jmdyck · 2019-06-21T13:52:44Z

The first commit message says:

The algorithms in RunJobs, TopLevelModuleEvaluationJob, and ScriptEvaluationJob have been moved to an informative annex [...]. Similarly, the PendingJob framework and the algorithm previously in place in EnqueueJob have been moved to this annex [...]. The annex is then fleshed out by also defining possible algorithms for the remaining host-defined abstract operations (HostEnsureCanCompileStrings, HostResolveImportedModule, and HostPromiseRejectionTracker) [...].

Where's this annex?

littledan · 2019-06-21T15:43:34Z

The annex was removed; you can see it in #735. We can squash this and change the commit message to clarify.

jmdyck · 2019-06-21T17:28:10Z

Why was the annex removed? (I don't see any reasons given here or in #735. Will the June meeting notes have some discussion of it?)

Without the Annex, the spec has no invocation of:

InitializeHostDefinedRealm
ParseScript
ScriptEvaluation
ParseModule

and no guidance or restrictions on if/when an implementation should invoke them. There isn't even a sentence to reassure the reader that the lack of invocation isn't a spec bug.

I'm also wondering about the normative effect of not having any requirement that these operations ever 'run'; i.e., there's no requirement that the behavior they express ever happens.

devsnek · 2019-06-21T17:38:21Z

without the Script/Module jobs it isn't entirely clear how one actually runs a script or module. Maybe we should have RunScript and RunModule example algorithms or something?

ljharb

Existing comments imply another diff is impending?

ljharb · 2019-06-22T05:04:28Z

spec.html

+    <emu-clause id="sec-hostenqueuepromisejob" aoid="HostEnqueuePromiseJob">
+      <h1>HostEnqueuePromiseJob ( _job_, _arguments_ )</h1>
+
+      <p>HostEnqueuePromiseJob is a host-defined abstract operation that schedules the abstract operation indicated by _job_ to be performed, with the arguments _arguments_, at some future time. The Jobs used with this algorithm are intended to be related to the handling of Promises, or otherwise, to be scheduled with equal priority to Promise handling operations.</p>


are there no consistent steps needed here? I’d expect, at the least, an assertion about the arguments passed to the abstract operation.

This text follows the pattern of other host hooks in not having algorithm steps--the algorithm is provided by the host. The argument types are documented in this paragraph.

This paragraph does not define the types - there is zero text that explicitly says that job is a Job, and nothing that indicates that arguments is a List or an Array (i assume), nor of what (presumably “any ECMAScript value”)

The type of job is given ("the abstract operation indicated by job"). I suppose you could state "with the arguments given by arguments (a List)" if it's not clear enough from context.

The argument name doesn’t guarantee what type it is.

@ljharb: The phrase the abstract operation indicated by _job_ means that the parameter _job_ is (bound to) an abstract operation. Its type is 'abstract operation'. (In the world of this PR, there isn't really a distinct Job type.)

As for _arguments_, you have to observe that HostEnqueuePromiseJob is a job-scheduling operation as described in the parent clause, which gives the parameter types of such operations more precisely.

Thanks for the clarification - this seems like a really obscure way to specify things. For example, why not make a Job a record with an abstract operation in a slot, so it’s a reified “thing” with its own type? (not suggesting doing that; it’s just that this approach seems very vague and hand-wavey and hard to understand without the context of how the HTML spec works, which shouldn’t be required in order to understand this spec)

I don't think wrapper types is a good strategy for increasing clarity; it just creates an extra layer of abstraction that makes it less clear what is being operated on.

I'd also point out that the job setup is inherited from the current spec, not something particularly tied to HTML. I agree that the current spec is pretty clear and close to how programming languages operate. E.g., process.nextTick(job) in Node.js, and similarly function pointers in C/C++.

@ljharb:

why not make a Job a record with an abstract operation in a slot, so it’s a reified “thing” with its own type?

In the world of this PR, you can think of that record existing in the host environment, but it's not the spec's job to specify the host environment. (At least, not to that level of detail.)

I agree with @domenic that introducing such a record to this PR (e.g., reinstating PendingJob) would not increase clarity.

this approach seems very vague and hand-wavey

I think some of that is inevitable when the spec talks about the host environment and its interaction with the ES implementation. (On the other hand, I do think this PR could be clearer.)

ljharb · 2019-06-22T05:05:05Z

spec.html

+
+      <p>HostEnqueuePromiseJob is a host-defined abstract operation that schedules the abstract operation indicated by _job_ to be performed, with the arguments _arguments_, at some future time. The Jobs used with this algorithm are intended to be related to the handling of Promises, or otherwise, to be scheduled with equal priority to Promise handling operations.</p>
+
+      <p>The implementation of HostEnqueuePromiseJob must conform to the requirements in <emu-xref href="#sec-jobs-and-job-queues"></emu-xref>. Additionally, Jobs must be scheduled in FIFO order, with Jobs running in the same order as the HostEnqueuePromiseJob invocations which scheduled them.</p>


This still hasn’t been deleted; is a new diff expected?

ljharb · 2019-06-22T05:10:25Z

spec.html

@@ -37913,7 +37740,7 @@ <h1>Promise Resolve Functions</h1>
            1. Let _thenAction_ be _then_.[[Value]].
            1. If IsCallable(_thenAction_) is *false*, then
              1. Return FulfillPromise(_promise_, _resolution_).
-            1. Perform EnqueueJob(`"PromiseJobs"`, PromiseResolveThenableJob, &laquo; _promise_, _resolution_, _thenAction_ &raquo;).
+            1. Perform HostEnqueuePromiseJob(PromiseResolveThenableJob, &laquo; _promise_, _resolution_, _thenAction_ &raquo;).


Can this produce an abrupt completion? If so, let’s prefix it with ?; if not, ! (same throughout)

I would be happy to add ! here; any concerns?

Then we should also add a sentence to the specification of HostEnqueuePromiseJob, like "An implementation of HostEnqueuePromiseJob must complete normally in all cases." (copied from HostReportErrors)

domenic · 2019-06-22T09:06:11Z

@devsnek @jmdyck I think it's a strict improvement that that, instead of a called-by-nothing RunJobs that matches no implementations (and was never required to be called), the world after this CL instead has InitializeHostDefinedRealm/ParseScript/ScriptEvaluation/ParseModule as top-level. Those, at least, have implementation counterparts.

We could consider adding an annex (as my original PR did) that gives an example of all the host hooks, as well as an example of initial startup. But I don't think that's necessary for making this patch work, and I think it'd best be done as a follow-up so that it gets its own debate (as RunJobs never did) as to whether the spec is really in the business of providing example hosts.

littledan · 2019-06-22T09:50:27Z

Note, the reason I omitted the annex was that people seemed to be objecting to it before. I am fine to add it back in, but I'd like to come to a conclusion here soon. This patch has already been waiting for years due to editorial disagreements.

jmdyck · 2019-06-22T13:36:43Z

I think it's a strict improvement that that, instead of a called-by-nothing RunJobs that matches no implementations (and was never required to be called), the world after this CL instead has InitializeHostDefinedRealm/ParseScript/ScriptEvaluation/ParseModule as top-level.

To be clear, I'm not disagreeing with that. I'm just saying that I think the spec needs to describe/architect/rationalize that world somewhat better than this PR does.

We could consider adding an annex (as my original PR did) that gives an example of all the host hooks, as well as an example of initial startup. But I don't think that's necessary for making this patch work, and I think it'd best be done as a follow-up so that it gets its own debate (as RunJobs never did) as to whether the spec is really in the business of providing example hosts.

I'd be okay with that, assuming the spec-without-such-an-annex is understandable.

Re example hosts: given the existence of test262, I think it would be useful if TC39 at least outlined an example host that is capable of processing the test suite in the intended manner. (But whether that description appears in ecma262 or test262 is another matter.)

littledan · 2019-06-22T15:13:54Z

@jmdyck Interesting idea. Would you be interested in preparing a modification of this PR (e.g., as a PR against it) to draft out such a description?

jmdyck · 2019-06-22T15:43:51Z

@jmdyck Interesting idea. Would you be interested in preparing a modification of this PR (e.g., as a PR against it) to draft out such a description?

If you mean where I said the spec needs to better "describe/architect/rationalize" the world created by this PR, I'm not sure I know enough about implementations + hosts to do so. It feels like what's missing is a description of what a "host" is or does from the perspective of the ES spec (or an ES implementation?). And/or some constraints on when+how the host can invoke (the behavior represented by) the new 'top-level' abstract operations.

jmdyck · 2019-06-22T20:39:02Z

I think what I'm asking for is something roughly like this:

To a first approximation, the implementation provides functionality that the host can invoke.

The host can invoke InitializeHostDefinedRealm(), to create a realm.
The host must have a way to acquire or construct one or more "source texts", each of which is a sequence of Unicode code points.
The host can invoke ParseScript(), supplying one of these source texts and one of the realms created above. [Note: To make this work, I think InitializeHostDefinedRealm() needs to return the Realm Record that it creates, else how does the host have a realm record to pass to ParseScript?] ParseScript() will either return a non-empty list of errors, or a Script Record.
The host can invoke ScriptEvaluation(), supplying a Script Record obtained above. ScriptEvaluation() will return a Completion Record.

The host can invoke these operations as many times as it likes, in any order, subject only to the constraint that a call to ScriptEvaluation() (with a given Script Record) must necessarily come after the call to ParseScript() that returned that Script Record, which in turn must necessarily come after the call to InitializeHostDefinedRealm() that created the realm passed to ParseScript().

And then all that similarly for the module side of things.

But it's not clear to me how Jobs and Job queues and Agents fit into this description.

Of course, it's actually far more complicated, because the host can do lots more than just invoke those top-level operations.

jmdyck · 2019-06-22T20:49:36Z

spec.html

-        1. Add _pending_ at the back of the Job Queue named by _queueName_.
-        1. Return NormalCompletion(~empty~).
-      </emu-alg>
+    <p>Jobs are scheduled for execution by ECMAScript host environments with abstract operations such as HostEnqueuePromiseJob. Such operations accept two parameters: _job_, an abstract operation, and _arguments_, a list of ECMAScript values. These abstract operations schedule _job_ to be performed with _arguments_ at some future time. Their implementations must conform to the following requirements:</p>


"These abstract operations" is initially ambiguous, because both the job-enqueuer and the job itself are abstract operations, and the previous sentence refers to both. Reading further in the sentence resolves the ambiguity, but it would be easier to read if the ambiguity wren't there at all. One way to avoid it would be to introduce a name for the class of job-enqueuing operations.

jmdyck · 2019-06-22T21:19:10Z

spec.html

+    <p>Jobs are scheduled for execution by ECMAScript host environments with abstract operations such as HostEnqueuePromiseJob. Such operations accept two parameters: _job_, an abstract operation, and _arguments_, a list of ECMAScript values. These abstract operations schedule _job_ to be performed with _arguments_ at some future time. Their implementations must conform to the following requirements:</p>
+
+    <ul>
+      <li>At some future point in time, when there is no running execution context and the execution context stack is empty, the implementation must:


Since the preamble to this <ul> ends with "Their implementations must conform to the following requirements:", it's tempting to think that "the implementation" here refers to the same thing, i.e. the (host's) implementation of a job-enqueing operation. But it could also mean what it usually means elsewhere in the spec (roughly, the ECMAScript engine).

But neither option entirely makes sense to me.

The implementation of a job-enqueuing operation is basically just "append this job to a queue". What happens when the jobs runs isn't the responsibility of the enqueuing operation.

If it's the ECMAScript engine doing the following steps (which, by itself, seems to make sense), how does the job get from a host-maintained queue to the engine? It seems like there needs to be a RunJob operation that the host can call to get the engine to do this.

Also, it's unclear how the the execution context stack would ever be empty. Specifically, InitializeHostDefinedRealm pushes an execution context onto the stack; what would ever pop that context from the stack?

The implementation of a job-enqueuing operation is basically just "append this job to a queue". What happens when the jobs runs isn't the responsibility of the enqueuing operation.

This isn't correct. See e.g. https://html.spec.whatwg.org/multipage/webappapis.html#enqueuejob(queuename,-job,-arguments) for what can be involved here. It is not simple at all, and the enqueuing operation is intimately tied to the event loop, which in turn works to give these guarantees.

If you haven't familiarized yourself with @littledan's presentation on this PR, that might be a good way of getting some shared context.

Also, it's unclear how the the execution context stack would ever be empty. Specifically, InitializeHostDefinedRealm pushes an execution context onto the stack; what would ever pop that context from the stack?

In HTML the stack is popped immediately upon creation. The stack-pushing is just a kind of weird way of getting a return value out of InitializeHostDefinedRealm(). See https://html.spec.whatwg.org/multipage/webappapis.html#creating-a-new-javascript-realm.

The implementation of a job-enqueuing operation is basically just "append this job to a queue". What happens when the jobs runs isn't the responsibility of the enqueuing operation.

This isn't correct. See e.g. https://html.spec.whatwg.org/multipage/webappapis.html#enqueuejob(queuename,-job,-arguments) for what can be involved here. It is not simple at all, and the enqueuing operation is intimately tied to the event loop, which in turn works to give these guarantees.

Sorry, yes, the host can make this as complicated as it likes, I was looking at it from the implementation's/engine's point of view. (I think.)

But even so, I'm still not sure which meaning of "the implementation" is intended.

If you haven't familiarized yourself with @littledan's presentation on this PR, that might be a good way of getting some shared context.

Sure. This one?

Also, it's unclear how the the execution context stack would ever be empty. Specifically, InitializeHostDefinedRealm pushes an execution context onto the stack; what would ever pop that context from the stack?

In HTML the stack is popped immediately upon creation.

Sneaky!

The stack-pushing is just a kind of weird way of getting a return value out of InitializeHostDefinedRealm(). See https://html.spec.whatwg.org/multipage/webappapis.html#creating-a-new-javascript-realm.

Ah, so that fits with my suggestion that InitializeHostDefinedRealm() needs to return the Realm Record that it creates.

I've read @littledan's presentation a few times now (assuming I got the right one). I don't think it answers any of the questions I've asked so far, but maybe answers some I haven't asked yet.

Thanks for the HTML links. One thing that hadn't quite clicked for me before was that an HTML user agent can manipulate the ES execution stack. (This feels a bit odd to me, since the ES spec doesn't mention that possibility when it describes the stack.)

I wouldn't mind looking into cleaning up this aspect of layering further in a follow-on PR.

"Clean up" as in refactor the spec so that the host doesn't have to directly manipulate the stack? That'd be nice. (In a follow-on PR, agreed.)

domenic · 2019-06-23T05:42:28Z

I think what I'm asking for is something roughly like this:

I'd like to echo the sentiments upthread that such additions be pursued as a separate PR after this one lands. We're just trying to implement the committee consensus from the June meeting here. Let's not scope-creep this into something bigger that will require another two-month (or two-year) roundtrip.

domenic · 2019-06-23T05:45:56Z

Nit: I wonder if "job" should be de-capitalized throughout? There used to be a Job Record type, and we capitalize record type names, but not the names of abstract concepts, which are now what jobs are.

I think "promise" definitely should be lowercased as well. (Example existing usages.)

jmdyck · 2019-06-23T15:46:53Z

I think what I'm asking for is something roughly like this:

I'd like to echo the sentiments upthread that such additions be pursued as a separate PR after this one lands.

I don't see any such sentiments. The closest I see is where you said:

We could consider adding an annex (as my original PR did) that gives an example of all the host hooks, as well as an example of initial startup. But I don't think that's necessary for making this patch work, and I think it'd best be done as a follow-up so that it gets its own debate.

And I said I'd be okay with that, but that was talking about an annex of examples, which is not what I was asking for. Moreover, I was responding to @littledan's invitation to prepare a modification of this PR, so that's counter to the sentiments you refer to.

We're just trying to implement the committee consensus from the June meeting here.

And I'm just trying to make sure that the resulting spec makes sense.

Let's not scope-creep this into something bigger that will require another two-month (or two-year) roundtrip.

I'll accept the editors' decision.

jmdyck · 2019-06-24T13:36:31Z

spec.html

+    </ul>
+
+    <emu-clause id="sec-hostenqueuepromisejob" aoid="HostEnqueuePromiseJob">
+      <h1>HostEnqueuePromiseJob ( _job_, _arguments_ )</h1>


So sometime between @littledan's presentation and this PR,
HostEnqueueJob("Foo", _job_, _args_)
got replaced with
HostEnqueueFooJob(_job_, _args_)
(roughly speaking).

Why's that?

It seems like defining a single job-enqueuing operation would be simpler than defining a 'class' of them.

We discussed the two alternatives that the slides presented in the TC39 meeting, and the consensus there (at least shared by me, @domenic, and @erights, if I understood correctly; I don't remember hearing opinions from others) was that hook-per-job is easier to understand and more flexible. This way, we can define different invariants for different hooks. For example, WeakRef finalizer callbacks should not be FIFO but rather considered unordered, whereas Promise-related job hooks should be ordered.

We discussed the two alternatives that the slides presented

(I'm not positive we're talking about the same thing, because I don't see the slides presenting what I'm suggesting. Are you thinking of the "One-off callbacks" alternative? I'm not.)

Anyhow, it seems to me the spec could as easily attach an invariant to a particular argument value as to a particular operation. E.g.: Jobs enqueued with _kind_ = "Promise" must be scheduled in FIFO order. No difference in flexibility there.

As for the "easier to understand" claim, I don't get it. I mean, I could see it if different kinds of jobs needed to supply different extra information when they were enqueued. But the PR says that every job-enqueuing operation accepts the same two parameters.

Yes, I was talking about PR #735 vs one-off callbacks (this PR). I am not sure what other option you are thinking of.

I agree, we could do that. We discussed these two options in TC39 and settled on one-off callbacks as subjectively simpler. The decision here is editorial, and we can iterate on it over time.

one-off callbacks (this PR)

Well, now I'm really confused, because one-off callbacks ("No use of Jobs") doesn't appear to be what this PR is doing.

So here's what I think you're saying: the consensus was that the presentation's second alternative (one-off callbacks, no use of jobs) was more flexible and easier to understand than the first (HostEnqueueJob), but rather than adopt one-off callbacks, you moved (for the purposes of this PR) part-way in that direction, from HostEnqueueJob to HostEnqueueFooJob. Have I got that right?

If I understand you correctly, yes.

Okay, good. But that still leaves me puzzled on 2 fronts:

If one-off callbacks were deemed better, why not just adopt one-off callbacks?

I still don't get why it's deemed more flexible and easier to understand.

You gave the example of specifying invariants, and I pointed out that invariants can be attached regardless. Or consider the example of a one-off callback from the presentation; I can imagine defining the operation:

HostCleanupFinalizationGroup(_fg_) 1. Return HostEnqueueJob("whatever", CleanupFinalizationGroup, << _fg_ >>).

If I can "implement" a one-off callback using HostEnqueueJob, then a one-off callback doesn't seem to be more flexible. (If it's deemed to have expositional benefit, we'd be free to retain it but define it as above. Of course, then it wouldn't be a "Host" operation, so name-change.)

In terms of flexibility, the three approaches look about the same to me.

But in other ways, HostEnqueueJob seems to me to be better: less to specify, less to understand, and less for downstream specs to have to deal with.

So I'm probably missing something. But what?

For Promises, there are several different algorithms that need to be run, scheduled in the same way. So there's still value in passing around an abstract algorithm, while having these callbacks per type of scheduling.

Yes, there are other ways of attaching invariants, and we could do it with HostEnqueueJob. We could have a string parameter, and specify the invariants to be related to what the string is. I see this as an extremely minor editorial difference compared to the patch under discussion.Tor me, it feels marginally cleaner to attach invariants to a whole algorithm rather than to the particular string. But we could go the other way; it just isn't a big deal. You probably aren't missing much.

I don't really understand why HostEnqueueJob would be less to specify or understand. There would probably be more lines of spec text, since we'd have to break things up by the queue name on both the JS and host environment side anyway. It would be sort of like, in a computer program, merging a few functions together with a switch statement at the top, so that we have fewer functions that you have to think about.

Re 1: I think I agree.

2. I see this as an extremely minor editorial difference

I agree that it's editorial. But given that it's an interface that downstream specs will probably reference, I don't think it's "extremely minor".

compared to the patch under discussion.To me, it feels marginally cleaner to attach invariants to a whole algorithm rather than to the particular string. But we could go the other way; it just isn't a big deal.

I don't really understand why HostEnqueueJob would be less to specify or understand.

Because it's one operation rather than many.

There would probably be more lines of spec text,

I doubt it. Each new HostEnqueueFooJob operation would have some boilerplate overhead.

since we'd have to break things up by the queue name on both the JS and host environment side anyway.

Not quite sure what you mean.

It would be sort of like, in a computer program, merging a few functions together with a switch statement at the top, so that we have fewer functions that you have to think about.

Sort of.

littledan · 2019-06-25T05:47:23Z

@jmdyck I would still welcome a PR against this branch for any clarifications you'd like to make. I don't quite understand what issues you see here, so such a PR would be helpful to drive this discussion forward.

jmdyck · 2019-06-25T14:48:28Z

I would still welcome a PR against this branch for any clarifications you'd like to make

Yeah, but that's tough when I can't (yet) see the June meeting notes. I could spend hours on a PR that (for all I know) goes counter to the committee's consensus. (Which is why I've asked for more information about why this PR is the way it is.)

I don't quite understand what issues you see here, so such a PR would be helpful to drive this discussion forward.

My main issue is the spec's lack of clarity about the interface/interaction between the ES implementation and the host environment. I know that that's a bigger problem than this PR, and it's not this PR's job to make it all better. But it would be nice if it at least didn't make things worse.

I'm not convinced that a PR against this branch would be the best way to move the discussion forward, but I can give it a shot.

jmdyck · 2019-06-25T22:01:55Z

@littledan, could you rebase this branch against master? The anachronisms are causing me some problems.

jmdyck · 2019-07-03T13:21:11Z

@littledan: I mean, I'd be okay with doing the rebase myself, but I don't know if delivering the resulting
changes via a PR would work.

littledan · 2019-09-02T14:23:10Z

Rebased. I think this is ready to land, but nevertheless, I'm looking forward to @jmdyck 's PR if he wants to propose an alternative.

littledan · 2019-09-10T08:54:43Z

@jmdyck Can we discuss your possible variant after this patch lands, or should we keep waiting for you to propose it?

jmdyck · 2019-09-10T13:50:03Z

Thanks for rebasing. I've been trying to get other things out of the way before getting back to this PR. I hope to start paging it back in today.

In the meantime, do you have any thoughts on my June 25th comment?

devsnek · 2019-09-10T14:09:58Z

spec.html

+    <ul>
+      <li>At some future point in time, when there is no running execution context and the execution context stack is empty, the implementation must:
+        <ol>
+          <li>Push an execution context onto the execution context stack.</li>


I think we should describe here how to set up the environment, setting [[ScriptOrModule]] and [[Function]] and such.

The wording here matches wording about execution contexts elsewhere in the specification. I wouldn't be opposed to the broader editorial change, but I think it's a separate issue.

@domenic

In tc39#1597 (comment) @domenic points out that: > The stack-pushing is just a kind of weird way of getting > a return value out of InitializeHostDefinedRealm(). This PR should simplify https://html.spec.whatwg.org/multipage/webappapis.html#creating-a-new-javascript-realm

littledan · 2020-03-05T18:05:00Z

I hope we can separate what HTML calls the "entry" realm semantics/layering (which has multiple implementer support and is part of the HTML living standard) from incumbent realm semantics (where I don't know if we have multiple implementer support). Let's focus on the entry realm for now.

syg · 2020-03-06T01:14:09Z

Should they be preserved via 'oldids'?

I'll preserve

job-queue
sec-enqueuejob
sec-jobs-and-job-queues
sec-runjobs

as oldids to sec-jobs.

I'll preserve

sec-promisereactionjob
sec-promiseresolvethenablejob

to the obvious renames.

I'm going to leave out

sec-host-report-errors
sec-scriptevaluationjob
sec-toplevelmoduleevaluationjob
table-25
table-26

Because it's probably clearer if they're obviously gone.

syg · 2020-03-06T01:17:00Z

I hope we can separate what HTML calls the "entry" realm semantics/layering (which has multiple implementer support and is part of the HTML living standard) from incumbent realm semantics (where I don't know if we have multiple implementer support). Let's focus on the entry realm for now.

Yeah, let's follow up with incumbents later. It's a more complicated thing and certainly shouldn't block this PR.

syg · 2020-03-06T02:04:32Z

@domenic We have an underspecified corner here. Getting the [[Realm]] of callables is fallible because of handlerless Proxy values. HTML currently doesn't specify what happens if we hit that case. Handlerless Proxy are kinda like null, so I'm going to pass null to the host hook in those cases. Sounds right?

spec.html

domenic · 2020-03-06T18:56:50Z

@domenic We have an underspecified corner here. Getting the [[Realm]] of callables is fallible because of handlerless Proxy values. HTML currently doesn't specify what happens if we hit that case. Handlerless Proxy are kinda like null, so I'm going to pass null to the host hook in those cases. Sounds right?

Amazing... So in this case the job would throw a TypeError, per proxy [[Call]]. I think null will work here, as no author code is run, just the ES-spec-throwing. So our current treatment of null (which is used when the handler is one of the built-in identity or thrower ones) will also work for this case.

This also effects Web IDL in general, I think. I will file a separate issue.

syg · 2020-03-06T22:07:20Z

If GetFunctionRealm throws on the handler for a PromiseReactionJob, should that promise be rejected at time of being enqueued (and thus not enqueue a microtask at all), or should a ReactionJob be enqueued as normal, which then rejects the promise when it actually runs?

What this PR currently does is reject the promise when the microtask runs, not when enqueued. It swallows the error at enqueues time and enqueue the job in the host with a null Realm. In HTML, when the microtask actually runs, it'll run the microtask, hit step 2 of the Proxy [[Call]], and then reject the promise.

FWIW Chrome implements this behavior. When V8 can't extract the context from the handler, it'll fall back to the currently entered context and enqueue a microtask on that context. When that microtask runs, the promise is rejected because the Proxy throws on being called.

domenic · 2020-03-06T22:09:49Z

That behavior makes sense to me. After all, you could revoke the proxy between the enqueuing and the running of the microtask, so we'll need this check anyway. Any earlier check would just be for catching the issue earlier, which for this kind of edge case, doesn't seem worth it.

bakkot · 2020-03-06T22:25:24Z

What this PR currently does is reject the promise when the microtask runs, not when enqueued.

I like this. It is consistent with the proxy still having a typeof of "function" but throwing when invoked.

(Plus that is the behavior normatively required by ECMA-262 prior to this PR, as far as I can tell.)

spec.html

…1597) This layering change retains the PendingJob concept but sends PendingJob records off to the host to be dispatched at the appropriate time, replacing the RunJobs algorithm. The change is expected to help specify the interaction between Promises, WeakRefs and the Web more precisely, while maintaining all current invariants. Co-authored-by: Domenic Denicola <d@domenic.me> Co-authored-by: Daniel Ehrenberg <littledan@chromium.org> Co-authored-by: Shu-yu Guo <syg@chromium.org>

ljharb · 2020-03-09T21:30:22Z

finally, then, this long-awaited PR's wish has been fulfilled and not rejected. <3 for everyone's efforts!

littledan · 2020-03-09T22:09:20Z

Are the editors planning on making any further editorial changes in follow-on patches, or is this the final version?

Do you plan to follow up with changes to whatwg/html#4722 to match the changes here? (Or do you prefer to leave that to me? Either way is fine with me.)

(re tc39#1597, see tc39#1237)

No other algorithm step creates a record value without some kind of name prefix before the open-brace. (re tc39#1597)

littledan mentioned this pull request Jun 20, 2019

Layering: Implement HostEnqueuePromiseJob whatwg/html#4722

Closed

domenic approved these changes Jun 21, 2019

View reviewed changes

ljharb reviewed Jun 22, 2019

View reviewed changes

jmdyck reviewed Jun 22, 2019

View reviewed changes

jmdyck reviewed Jun 24, 2019

View reviewed changes

littledan mentioned this pull request Jun 26, 2019

Add Atomics.waitAsync host hook whatwg/html#4613

Merged

3 tasks

littledan force-pushed the enqueue-promise-job branch from 5a0d850 to a832c38 Compare September 2, 2019 14:21

devsnek reviewed Sep 10, 2019

View reviewed changes

bakkot reviewed Mar 6, 2020

View reviewed changes

spec.html Show resolved Hide resolved

bakkot reviewed Mar 6, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

domenic mentioned this pull request Mar 6, 2020

Issues with callables, realms, and proxies whatwg/webidl#847

Open

TimothyGu reviewed Mar 6, 2020

View reviewed changes

spec.html Outdated Show resolved Hide resolved

ljharb force-pushed the enqueue-promise-job branch from 043669d to 3588286 Compare March 9, 2020 21:03

ljharb approved these changes Mar 9, 2020

View reviewed changes

ljharb force-pushed the enqueue-promise-job branch from 3588286 to c595020 Compare March 9, 2020 21:21

ljharb merged commit c595020 into tc39:master Mar 9, 2020

syg mentioned this pull request Mar 10, 2020

[editorial] Fix typo in NewPromiseReactionJob #1893

Merged

ljharb pushed a commit to syg/ecma262 that referenced this pull request Mar 10, 2020

[Editorial] Fix typo in NewPromiseReactionJob from tc39#1597 (tc39#1893)

9a9753b

ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Mar 21, 2020

Markup: delete 'es6num' comments (tc39#1904)

ab3593c

(re tc39#1597, see tc39#1237)

ljharb pushed a commit to jmdyck/ecma262 that referenced this pull request Mar 21, 2020

Editorial: insert "the Record" before record 'constructor' (tc39#1904)

2bded5a

No other algorithm step creates a record value without some kind of name prefix before the open-brace. (re tc39#1597)

syg mentioned this pull request Mar 23, 2020

Layering: fix the jobs infrastructure #735

Closed

jmdyck mentioned this pull request Sep 24, 2021

Note entrypoints #2534

Open

jridgewell mentioned this pull request Dec 15, 2021

Test Promise thenable's onResolve realm tc39/test262#3356

Open

mstoykov mentioned this pull request Feb 3, 2022

Promise API feedback dop251/goja#371

Closed

jmdyck mentioned this pull request May 1, 2023

Normative: Add Atomics.waitAsync #3049

Merged

3 tasks

jmdyck mentioned this pull request Sep 12, 2023

Audit host hook points in HTML #3147

Open


		<p>HostEnqueuePromiseJob is a host-defined abstract operation that schedules the abstract operation indicated by _job_ to be performed, with the arguments _arguments_, at some future time. The Jobs used with this algorithm are intended to be related to the handling of Promises, or otherwise, to be scheduled with equal priority to Promise handling operations.</p>

		<p>The implementation of HostEnqueuePromiseJob must conform to the requirements in <emu-xref href="#sec-jobs-and-job-queues"></emu-xref>. Additionally, Jobs must be scheduled in FIFO order, with Jobs running in the same order as the HostEnqueuePromiseJob invocations which scheduled them.</p>

Layering: Allow hosts to cleanly specify Promise Job semantics #1597

Layering: Allow hosts to cleanly specify Promise Job semantics #1597

Conversation

littledan commented Jun 20, 2019 • edited Loading

domenic left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmdyck commented Jun 21, 2019

littledan commented Jun 21, 2019

jmdyck commented Jun 21, 2019

devsnek commented Jun 21, 2019

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ljharb Jun 22, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Jun 22, 2019

littledan commented Jun 22, 2019

jmdyck commented Jun 22, 2019

littledan commented Jun 22, 2019

jmdyck commented Jun 22, 2019

jmdyck commented Jun 22, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic Jun 23, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

domenic commented Jun 23, 2019 • edited Loading

domenic commented Jun 23, 2019

jmdyck commented Jun 23, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented Jun 25, 2019

jmdyck commented Jun 25, 2019

jmdyck commented Jun 25, 2019

jmdyck commented Jul 3, 2019

littledan commented Sep 2, 2019

littledan commented Sep 10, 2019

jmdyck commented Sep 10, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

littledan commented Mar 5, 2020

syg commented Mar 6, 2020

syg commented Mar 6, 2020

syg commented Mar 6, 2020

domenic commented Mar 6, 2020

syg commented Mar 6, 2020 • edited Loading

littledan commented Jun 20, 2019 •

edited

Loading

ljharb Jun 22, 2019 •

edited

Loading

domenic Jun 23, 2019 •

edited

Loading

domenic commented Jun 23, 2019 •

edited

Loading

syg commented Mar 6, 2020 •

edited

Loading