Skip to content
This repository has been archived by the owner on Aug 29, 2021. It is now read-only.

[Spec] figure out how to return/propagate promises properly #18

Closed
domenic opened this issue Apr 23, 2018 · 11 comments
Closed

[Spec] figure out how to return/propagate promises properly #18

domenic opened this issue Apr 23, 2018 · 11 comments

Comments

@domenic
Copy link
Member

domenic commented Apr 23, 2018

#15 has a problem; I'm not sure how easy it is to fix. Namely, it does stuff like

  1. Let promiseCapability be NewPromiseCapability(%Promise%).
  2. Do foo.
  3. Do Await(Bar(), promiseCapability).
  4. Do baz.
  5. Return promiseCapability.[[Promise]]

This doesn't really work, because all of the steps after step 3---including the return step---get sucked into the "await". So we never really return anything to the caller.

That is, as currently written, abstract ops that use Await() never return anything. Kinda? Except sometimes they do, in the current spec, in cases where it's OK for the caller to suspend itself?

This has worked OK for Await() usages so far, but here we really need a series of promise-returning abstract ops which call each other. I'm not sure what the right fix is. There might be something quick. Or we might need to do tc39/ecma262#933. Or maybe we need to mutate Await() to cause its caller to return a promise? But that seems to clash a bit with other uses, hmm.

One issue here is that we would really benefit from "await-like" technology, instead of "then-like" technology, because we're inside a loop. I can see how to solve this general class of problem by converting to "then-like" technology, but then my loop will be extraordinarily ugly (and also "then-like" technology is very verbose currently; that's tc39/ecma262#933).

@domenic
Copy link
Member Author

domenic commented Apr 23, 2018

I think I figured out what would work here. First, a minimal example of what we're doing.

Assume the environment provides some abstract op, e.g. PromiseReturningSetTimeoutFromEnvironment. It returns a promise, via magic, that fulfills after the given amount of time.

We want to write an abstract op that returns a promise that fulfills with 42 after one second. How would we attempt this? Here's what #15 does:

UsesAwait()

  1. Let promiseCapability = NewPromiseCapability(%Promise%).
  2. Perform ! Await(PromiseReturningSetTimeoutFromEnvironment(1 second)).
  3. Perform ! Call (promiseCapability.[[Resolve]], undefined, «42»).
  4. Return promiseCapability.[[Promise]].

This seems a bit fishy. Let's expand it out by inlining Await and PromiseReturningSetTimeoutFromEnvironment:

UsesAwaitUnrolled()

  1. Let promiseCapability = NewPromiseCapability(%Promise%).
  2. Let asyncContext be the running execution context.
  3. Ask the environment to perform the following steps in 1 second:
    1. Let prevContext be the running execution context.
    2. Suspend prevContext.
    3. Push asyncContext onto the execution context stack.
    4. Resume suspended evaluation of asyncContext.
    5. Assert: when we've reached this step, we're back to prevContext being the running execution context.
  4. Remove asyncContext from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
  5. Set the code evaluation state of asyncContext such that when evaluation is resumed, we perform the following steps:
    1. Perform ! Call (promiseCapability.[[Resolve]], undefined, «42»).
    2. Return promiseCapability.[[Promise]].

The problem is pretty clear now. We only return to the caller after a suspend-resume cycle. So if we have a caller, like

Caller()

  1. Let x = UsesAwaitUnrolled().

What is x? Well, it is indeed a promise, as desired. BUT, to get that promise, we might have had our execution context suspended out from under us.

Instead we really wanted something like this:

UsesAwaitUnrolled2()

  1. Let promiseCapability = NewPromiseCapability(%Promise%).
  2. Let asyncContext be the running execution context.
  3. Ask the environment to perform the following steps in 1 second:
    1. Let prevContext be the running execution context.
    2. Suspend prevContext.
    3. Push asyncContext onto the execution context stack.
    4. Resume suspended evaluation of asyncContext.
    5. Assert: when we've reached this step, we're back to prevContext being the running execution context.
  4. Remove asyncContext from the execution context stack and restore the execution context that is at the top of the execution context stack as the running execution context.
  5. Set the code evaluation state of asyncContext such that when evaluation is resumed, we perform the following steps:
    1. Perform ! Call (promiseCapability.[[Resolve]], undefined, «42»).
  6. Return promiseCapability.[[Promise]].

This is a tiny change: it just says that we return the promise once all the setup is done, instead of returning it after the evaluation state has resumed. But it does what we want.

That's as far as I've gotten so far. The next big question is, can we figure out some modification to Await(), or some other abstraction, which works "as we want" in this case? Thinking...

@jmdyck
Copy link

jmdyck commented Apr 23, 2018

I don't think there's a difference between UsesAwaitUnrolled() and UsesAwaitUnrolled2().

@domenic
Copy link
Member Author

domenic commented Apr 23, 2018

@jmdyck so sorry about that. Apparently two-space indents are not good enough for GitHub. Fixed.

One more long post coming...

@domenic
Copy link
Member Author

domenic commented Apr 23, 2018

Here's something that could work.

Introduce a new abstract op, AwaitWithReturn:

AwaitWithReturn(promise)

Algorithm steps that say

  1. Let value be AwaitWithReturn(promise).

Mean the same thing as

  1. Let resolveCapability be ! NewPromiseCapability(%Promise%).
  2. Perform ! Call(resolveCapability.[[Resolve]], undefined, « promise »).
  3. Let resolvedPromise be resolveCapability.[[Promise]].
  4. Let stepsFulfilled be the following steps of the algorithm that invoked Await, with argument value.
  5. Let onFulfilled be CreateBuiltinFunction(stepsFulfilled).
  6. Let resultCapability be ! NewPromiseCapability(%Promise%).
  7. Perform ! PerformPromiseThen(resolvedPromise, onFulfilled, undefined, resultCapability).
  8. Return resultCapability.

Similarly, you can write

  1. Perform AwaitWithReturn(promise).

to mean the same thing, except that your steps don't take an argument value.

Note that you never use ! and ? with AwaitWithReturn.


The difference is that AwaitWithReturn(p) behaves more like the await keyword; it automatically causes your abstract op to return a promise, and it just stuffs everything after it into a onFulfilled callback to p.then(), effectively.

In contrast, Await() adds a handler to p that will actually resume the caller execution context, and then it suspends the caller execution context in preparation for that later resumption. This is very powerful. It allows us to say in the middle of various syntactic forms' runtime semantics, "let's stop evaluating this syntax for a bit and come back to it when a promise is ready". It does this with suspension and evaluation, not then()-like chaining.

I am pretty sure we need both still...

With this in hand, we can then write things like:

UsesAwait2()

  1. Perform AwaitWithReturn(PromiseReturningSetTimeoutFromEnvironment(1 second)).
  2. Return 42.

Here the "Return 42" got stuffed into the "onFulfilled callback" of AwaitWithReturn, so this works as expected.

We could also do

UsesAwait3()

  1. Perform AwaitWithReturn(PromiseReturningSetTimeoutFromEnvironment(500 ms)).
  2. Perform AwaitWithReturn(PromiseReturningSetTimeoutFromEnvironment(500 ms)).
  3. Return 42.

and this works as expected: it's basically setTimeout(500).then(() => setTimeout(500).then(() => 42)).

(Better names for AwaitWithReturn welcome.)

@guybedford
Copy link
Collaborator

@domenic just catching up on this here - I don't have any knowledge of the Promises spec, but would this AwaitWithReturn still cause the EnqueueJob call of promise resolve? Just to be sure that if this spec were to use that it would still retain sync execution of synchronous subgraphs of the module tree.

@domenic
Copy link
Member Author

domenic commented Aug 30, 2018

I don't have any knowledge of the Promises spec, but would this AwaitWithReturn still cause the EnqueueJob call of promise resolve?

That probably depends on how tc39/ecma262#1250 goes. It would definitely still cause a job enqueued for the PerformPromiseThen, but if we are able to land 1250 (and I hope we can), then there wouldn't be another job for promise resolution.

Just to be sure that if this spec were to use that it would still retain sync execution of synchronous subgraphs of the module tree.

This depends on what you mean. EnqueueJob, at least in browsers, is still "synchronous"; it just gets pushed to the end of the task. A serious of enqueued jobs (as you would see, for example, in a series of modules that immediately resolve their evaluation promise by virtue of not having any awaits) execute one after the other inside the microtask loop. So they'll "synchronously" execute, although always at the end of the event loop.

@guybedford
Copy link
Collaborator

@domenic thanks for the clarification, in that case I can look into a spec PR here if it would help.

One question I had - what does error handling look like in this model? Is it just the usual abrupt completion check?

@littledan
Copy link
Member

These abstractions seem nice, but I am not sure if they are needed for top-level await. All we have to do is Promise.all the dependencies, then execute the body, and let that body execution settle the surrounding capability. I think we can do this just fine with then-based notation, as in guybedford#1 .

@littledan
Copy link
Member

The current specification text waits for things without introducing any new notation.

@domenic
Copy link
Member Author

domenic commented Apr 26, 2019

I managed to get myself pretty royally confused when I was last in this area, tentatively concluding that everything was broken and we needed a lot more work. But am happy to trust you, if you feel un-confused.

@littledan
Copy link
Member

@domenic To be honest, I still don't understand what you were trying to get at there; I'm pretty confident in the mechanics of the current spec text, though (but I have some editorial changes coming based on your feedback).

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants