Rephrase next-event-loop-turn requirement.

[domenic]

This fixes #100, by explicitly calling out the concept of an event loop. It also explicitly mentions the micro-turn vs. macro-turn distinction @erights discussed in #84 (comment).

See #70 for an earlier attempt, that used the concept of the function execution stack instead of explicitly talking about event loops.

---

I am not 100% sure this is the way to go, but it seems like the path of least resistance. I'd love if people threw out alternatives. Maybe the best I have is

onFulfilled or onRejected may only be called when the function execution stack is empty of user code [1].

with [1] explaining what "user code" means, e.g. promise implementation code and [native code] or Node.js platform code is not user code, but all other things are.

[bergus]

Actually I think the phrasing with " not called before then returns " was enough. In the resolvers-spec we should specify that fulfill/reject calls should not fire the callbacks before the resolver returns, and the behavior will be locked down.

No need for macro/micro event loops etc. The point should be that nothing happens during one user-defined function run, not before it returns control back to the trampolining mechanism (and if there is no active trampolining, it gets deferred to the next tick). The call stack must unwind far enough to have only promise-implementation-specific code on it, but multiple callbacks might be fired in the same event loop turn (and sometimes even in the same as then was called, as in brian's nested-then example or mine with the ugly syncResolve).

Specifying the " function execution stack to be empty of user code " is imho equivalent, but more complicated…

[juandopazo]

I came to say what @briancavalier already said: MutationObserver runs before the next tick of the event loop and after the rest of the code. The "execution stack" phrasing is what we're looking for. It's already in the EcmaScript spec in 10.3 Execution Contexts. It's obscure, but it's always a good time to learn something new, isn't it?

[briancavalier]

@bergus and @juandopazo Yeah, I think the challenge is finding a way to define what a "clear execution stack" is in the context of a promise system. E.g. an empty promise system execution stack === ES execution stack containing either or both of: 1) platform scheduler code (e.g. node's own process.nextTick code), 2) promise system code.

It seems like there are two routes we could go:

1. @domenic's original suggestion of trying to define "user code", which_excludes_ native, platform scheduler, and promise implementation code, or
2. the opposite approach of trying to define "promise system code", which includes native, platform scheduler, and promise implementation code.

[domenic]

A thought: is the clean-stack invariant really what we want? Inspired by @juandopazo on es-discuss, consider a hypothetical resolver API:

var p = new Promise(resolve => {
    setTimeout(=> {
      executeArbitraryUserCode();
      resolve.synchronously();
    }, 100); 
});

p.then(() => console.log("I am executed on a non-clean stack."));

Here resolve.synchronously() immediately executes the fulfillment callback(s) for p, of course wrapping them in a try/catch and transforming any errors. This should prevent multiple callbacks from interfering with each other. It also checks that at least one tick has indeed passed, and either refuses to execute or automatically schedules in the next tick if not.

This breaks the clean-stack invariant, but I am not sure what practical consequences it has. @erights?

[erights]

An attacker could call resolve.sync from the midst of a plan, disrupting clean stack assumptions by the callback

[domenic]

I don't think I quite understand. What plan and which callback? Example code would be much appreciated :)

[erights]

Not tonight. But in any case, what's motivating this?

[domenic]

Just want to make sure we have the right invariant before we codify it.

[ForbesLindesay]

We have to be carefull not to restrict our options for optimisation unnecessarily, and we want to make sure we include the important invariant, so it's esseential we understand what we're trying to encode.

[juandopazo]

@domenic good catch! I only skimmed through that part of the ES5 spec. Shouldn't this be enough: "callbacks must be called in a different/future execution context"?

[juandopazo]

Alex Russel just commented on es-discuss/public-script-coord:

This is far too glib. The spec may very well be wrong on this point. The design goal isn't to require a full yeild of the event loop, but instead to force async code flow -- that means that resolving and calling back should be able to happen at "end of microtask"; the same timing as Object.observe() callbacks.

http://esdiscuss.org/topic/promisefutuasynchronyinthen#content-5

[ForbesLindesay]

We could just appeal to pragmatism and say "onFulfilled or onRejected must always be called asynchronously [4.1]."

We could then leave it to individual implementations and the tests to establish exactly what that means (plus something to clarify in the notes)

(P.S. I love that people are now linking to esdiscuss.org rather than pipermail 😄)

[domenic]

Indeed. That kind of was the intent of the wording in this pull request, as discussed in ab6e22f#readme-md-P5. But I think that was kind of a mistake; we need to figure out exactly which invariants we mean to preserve.

Some code samples would be helpful. To get us started, I'll adapt the ones from #100 (comment). We want the sequence of calls to be a then b then c always:

// (1)
var p = new Promise(resolve => resolve())
a();
p.then(c);
b();

// (2)
var resolveP;
var p = new Promise(resolve => resolveP = resolve);

a();
resolveP();
p.then(c);
b();

// (3)
var resolveP;
var p = new Promise(resolve => resolveP = resolve);

a();
p.then(c);
resolveP();
b();

What else?

Note that (3) means our current 1.0 wording ("then must return before onFulfilled or onRejected is called") is insufficient. But we don't have any code samples that guide us toward the stack-empty-of-user-code requirement.

[briancavalier]

Today (which way is the wind blowing? haha), I'm back to feeling like our current "must not be called before then returns" is sufficient to preserve the real invariant, which is some form of "called in an execution context that is free of user code", which has proven to be very tricky to express/define clearly (e.g. we have to define "user code", which is tricky) without being overly limiting. I also feel like the current language leaves a lot of headroom for clever optimizations, which, imho, is a good thing.

The problem is that it doesn't cover case 3 in @domenic's examples.

Coming at this from another angle, is there a way to augment the current language to cover case 3? Seems tricky since this spec doesn't cover the resolve() API. I don't have any ideas right now, but I'll think about it.

[erights]

end of microtask

Note that end of microtask is an empty stack situation

[briancavalier]

Here's a quick attempt at trying to use execution context to extend the current language to cover case 3 above.

onFulfilled and onRejected must not be called in the execution context (or child thereof??)

• of the corresponding call to then
• in which promise transitions from pending to fulfilled or rejected (this wording may also require changing 'If/when' in 3.2.5.1/2 to "After")

I don't think either bullet is quite right, but I do feel like it could be a good direction. Thoughts?

[bergus]

I think that is exactly what we want. The first bullet point actually is what we already have (" not before then returns "), and the second bullet point should be part of the resolvers-spec (since we here only specify then).

This wording meets all those requirements to have the callbacks in the next micro/macro turn, and it allows trampolining either in native or in (promise) library code.

[domenic]

@briancavalier Interesting. Your first bullet point takes care of (1) and (2), whereas the second takes care of (3). I tried creating counterexamples using IIFEs to create new execution contexts, but I think it still holds. Here they are, for reference:

// (4)
var resolveP;
var p = new Promise(resolve => resolveP = resolve);

a();
(() => p.then(c))();
resolveP();
b();

// (5)
var resolveP;
var p = new Promise(resolve => resolveP = resolve);

a();
p.then(c);
(() => resolveP())();
b();

---

@bergus I feel somewhat strongly that we can't punt this to the resolvers spec; we are specifying an important aspect of when onFulfilled and onRejected are called, in terms of concepts like "if/when state transitions occur" that are well-defined in the base spec.

[bergus]

@domenic:

we can't punt this to the resolvers spec; we are specifying an important aspect of when onFulfilled and onRejected are called

…but only in reference to the then call, don't we? And we have covered that already with "not during the call".

in terms of concepts like "if/when state transitions occur" that are well-defined in the base spec

I don't think it would make much sense to specify any chronological/execution-logical relation between the callbacks and that abstract state transition operation - other than "each callback at most once, and not before the transition". There are enough cases where they can happen synchronously together.

The only thing we might want to specify how manual resolve calls should resolve calls should relate to the callback execution. But we need to define that explicitly together with every resolution procedure, and not for all those who trigger the abstract state-change operation. That's why I think we needed this in the resolution spec.

Yet, we already have some kind of resolution procedure in this spec here: [[Resolve]]. The case where a Promise is returned from a callback (p3 = p1.then(() => p2)) is already well-defined: p3 can be resolved (and its handlers be called) as soon as p2 resolves. "as soon as" here means no restrictions, this is one of the cases where we don't want to specify anything.

But what if a thenable is returned? Have a look at

var p3 = aResolvedPromise.then(function() {
    a2();
    return {
        then: function(res, rej) {
            a3();
            // setTimeout(function() {
                a4();
                res();
                b();
            // }, 100);
        }
    };
});
p3.then(c);
a1();

We do know that a1 to a4 are executed first, but is it b or c then? If res() is called synchronously from the thenable's then, it might be easy to defer c after the then returned, but what if we wrap it in the timeout (new execution stack)? Do we really want to specify that? If we do, then it might be useful to specify another abstract [[ResolveButCallBackAsynchronously]] procedure that can be referenced from places where such behaviour is needed.

[briancavalier]

I think I agree with @domenic that we should try to provide some minimal specification of the relationship between state transitions and execution of handlers. One practical reason I feel it's important is that if we don't, then someone can choose to implement Promises/A+, but not the resolvers spec, creating a valid promise implementation that allows case 3.

@bergus, a few questions about what you wrote:

…but only in reference to the then call, don't we? And we have covered that already with "not during the call".

There are enough cases where they can happen synchronously together.

Hmmm, not sure I understand what you're saying here, could you clarify what you mean by "synchronously together", and in what context we might want to allow that?

The only thing we might want to specify how manual resolve calls should resolve calls should relate to the callback execution

I think I see what you're getting at. Are you saying that by specifying a relationship between state transitions and handler execution, we're being too restrictive for some state transitions which clever implementations might be able to optimize by running handlers within the execution context where the state transition occurs? ("occurs" is a pretty loose term, see below)

We do know that a1 to a4 are executed first, but is it b or c then?

I'm not sure what you're getting at, but it seems like we can't do anything to control the order of b and c, since c is under the control of a foreign thenable. I believe that this is similar (but not identical) to an example we've seen before:

p.then(f).then(g);
p.then(h);

We can't specify an exact order for f, g, and h here because the temporal relationship between g and h is effectively determined by f's return value. Both fgh and fhg are valid orderings.

---

I think my bullet 2 may not be sufficient because the exact point at which a promise has transitioned is implementation dependent.

onFulfilled and onRejected must not be called in the execution context (or child thereof??) in which promise transitions from pending to fulfilled or rejected

For example, I think it leaves open the possibility of doing:

function Promise(resolver) {
    function resolve(x) {
        if(!isPromiseOrThenable(x)) {
            _transitionToFulfilled(x);
            // This breaks case 3
            runHandlers();
        } else {
            // ...
        }
    }

    function _transitionToFulfilled(x) {
        // This is the execution context where the
        // promise transitions to fulfilled
    }

    resolver(resolve, ...);
}

In that case, the implementer could make the case that the handlers are being execution outside the execution context where the state transition happens.

Seems like it may need a bit more thought.

[bergus]

if we don't [specify that], then someone can choose to implement Promises/A+, but not the resolvers spec, creating a valid promise implementation that allows case 3

Yes. And I don't see what's wrong with that - resolveP (from case 3) is unspecified, and if the implementor properly documents that it directly calls the handlers everything is fine. He might want that as a feature.

But let's get to your questions.

There are enough cases where they can happen synchronously together.

Hmmm, not sure I understand what you're saying here, could you clarify what you mean by "synchronously together"

Uh, I already feared that this wording is unclear. I meant that they are happening in the same execution context, the handlers getting called immediately after the state flag transitioned. Just like in your example code, where the resolve function triggers both together. resolve, and with it the resolver which invoked resolve, will not have returned before the callbacks are executed.

…and in what context we might want to allow that?

Wherever the resolver does not execute anything after the resolve invocation: a tail call, basically. Let's call this "tail resolution", just like in "tail recursion".

An example for this in user-code could be domenic's wait-implementation from above - it would not make sense to put another tick in between the resolve call and the callbacks here. And there should be a dedicated method like the resolve.synchronously() he used - or at least, it should be possible to implement that while being A+ compliant.

Other examples will include library-space code, see the next answer.

I think I see what you're getting at. Are you saying that by specifying a relationship between state transitions and handler execution, we're being too restrictive for some state transitions which clever implementations might be able to optimize by running handlers within the execution context where the state transition occurs?

Exactly. Just think of the very simple

p2 = p1.then(function a(){ return 5; });
p2.then(b);

When p1 fulfills, a and b can be executed in the same turn of the event loop ("synchronously"). Actually I would even expect that :-) Anyway, the state transition of p2 and the callback execution can happen together, right after a returns.

I even would propose that we can do this whenever a "promise state is adopted" ([[resolve]] step 2). Having

p3 = p1.then(function a(){
    var p2 = getSomePromise();
    p2.then(b);
    return p2;
});
p3.then(c);

As soon as p2 is fulfilled (at the same time when b is called back), p3 can be transitioned and c be called - everything in the same event loop turn, and especially the state transition of p3 and its callbacks even in the same execution context.

We do know that a1 to a4 are executed first, but is it b or c then?

I'm not sure what you're getting at, but it seems like we can't do anything to control the order of b and c, since c is under the control of a foreign thenable.

Yes, maybe we should not specify that at all.

I believe that this is similar to p.then(f).then(g); p.then(h); where no relationship between g and h is specified.

No. I tried to create a case 3, yet not with some hypothetical Promise constructor (as domenic used) or Deferred object (as in my original example) but only with the tools which we are specifying here (the thenable assimilation). I hope you can spot the similarity.

To rephrase my question: Are the resolvePromise() and rejectPromise() arguments which we pass to the thenable allowed to synchronously transition the state and invoke callbacks, or do we want to enforce the callbacks to happen asynchronously? Currently the spec only states " If/when resolvePromise is called with a value x, […] resolve promise with x ".

---

As you noticed yourself, specifying against the execution context (or child thereof??) in which promise transitions [happen] (bullet 2) is not sufficient. Instead, we do need to specify asynchronity against the function which is exposed to the user code.

That's the actual point I'm trying to make (sorry for the long reading :-). It is why I think we need to specify that - at least parts of it - in the resolvers spec. It could as well be a rule that is specified here, and only needs to be referenced from the resolvers spec. And it might should/need be referenced from the [[Resolve]] procedure for thenables, which is the only part of this spec where a resolution feature is exposed.

[domenic]

@bergus thanks very much for diving into this with us. Tricky business. I like the idea of re-doing (3) in terms of what we have here. Here's my attempt:

// (3')
var resolveP;
var p = aFulfilledPromise.then(() => { then(onFulfilled) { resolveP = onFulfilled; } });

a();
p.then(c);
resolveP();
b();

We still definitely want a-b-c for this. I think the more complicated questions you were asking are getting into other territory that shouldn't necessarily be specified, as @briancavalier said.

I also agree with your examples (e.g. a and b could both be executed in the same turn of the event loop).

Your examples actually sway me back toward something very like what we have in this pull request, because the pull request contains the idea of "whenever you want, in whatever combination you want, as long as it's after the turn in which then is called."

Yet, dealing with the subtleties of the "end phase" of the event loop (in which e.g. mutation observers and Node.js >= 0.10 process.nextTick and such run) is not much fun.

What about this??

onFulfilled and onRejected must not be called in the execution context of the corresponding call to then, or in any parent contexts of that execution context.

I think this works! It's saying that by the time you call onFulfilled or onRejected, you must have reset your stack. You can be as deep into a clever trampoline or chained fulfillment sequence as you want, but you can't be in the same call stack in which then even exists.

Thoughts!??

[bergus]

@domenic:

thanks very much for diving into this with us. Tricky business.

Just trying to prove that what we currently have is sufficient, to keep the desired simplicity :-)

I like the idea of re-doing (3) in terms of what we have here. Here's my attempt […]

That doesn't work because the .then method is called asynchronously, long after you tried to resolveP. The b() of course is called before c in any case. I'll try to simplify my example by using coffescript sugar, removing all those a calls and the optional timeout:

somePromise.then(() => {
    then: function(res, rej) {
        res() // allowed to be sync or not?
              // proposed solution: yes - don't specify
        b()
    }
}).then(a)

the pull request contains the idea of "[…] after the turn in which then is called." [different highlighting]

Yeah, and that is too strict. It doesn't account for brians example p1.then(function a(){ p1.then(b) }) where b can be executed immediately after a.

What about this??

onFulfilled and onRejected must not be called in the execution context of the corresponding call to then, or in any parent contexts of that execution context.

I do not understand why you include the parent execution contexts? It sounds to me like the stack reset which you are talking of prevents trampolining in library code (as in brians example), maybe I did misunderstood this?

I still think if some of the parent execution contexts have access to a resolver which they intend to call and nevertheless attach a handler via .then, either something is horribly wrong or they know what they are doing. If we really want to specify the behaviour for case 3, we need to specify that the exposed resolveP method which they have access to is not synchronous (i.e. calls the handlers before it returns).

[briancavalier]

onFulfilled and onRejected must not be called in the execution context of the corresponding call to then, or in any parent contexts of that execution context.

My reading of this is the same as @bergus: the "or in any parent contexts" bit prevents clever trampolines. If we remove that bit, it works:

"onFulfilled and onRejected must not be called in the execution context of the corresponding call to then"

Which is mostly the same as our current "not before then returns" language. This part does seem sufficient to me. I'm still not sure about the case 3, tho.

[domenic]

My reading of this is the same as @bergus: the "or in any parent contexts" bit prevents clever trampolines.

Disagree! The trampoline parent context would not contain the call to then as a child. The trampoline generally batches together multiple onFulfilled calls inside a new stack introduced by e.g. setImmediate.

[bergus]

The trampoline parent context would not contain the call to then as a child.

But then it's neither a parent, nor what I would call a trampoline?

The trampoline generally batches together multiple onFulfilled calls inside a new stack introduced by e.g. setImmediate

Does it? That would not be capable of trampolining p1.then(function a(){ p1.then(b) }), but it should be possible to construct such one.

[briancavalier]

The trampoline parent context would not contain the call to then as a child

Hmmm, I need to think more closely about your wording.

That would not be capable of trampolining p1.then(function a(){ p1.then(b) }), but it should be possible to construct such one.

Yes, I agree this is important. I feel we must allow a and b to be executed in the same trampoline. In fact, when.js 2.x does exactly this whenever possible. Similarly:

p1.then(function a(){ p2.then(b) })

Should also allow a and b in the same trampoline under the right circumstances, for example, if both p1 and p2 already fulfilled before p1.then is called.

[domenic]

Argh, you got me.

That would not be capable of trampolining p1.then(function a(){ p1.then(b) })

Yeah. Example:

p1.then(function a() { p1.then(c); b(); });

We want the execution order to be a then b then c. The trampolining code probably does something like:

• p1.then queues a;
• new stack (*)
  • run a
    • p1.then queues c (**)
    • run b
  • a is done, but queue is not empty, so
  • run c (***)

As we can see, c is run () inside an ancestor () of the context where then is called ().

Damn!

[briancavalier]

I'll have time to comment on Monday.

[bergus]

thoughts on my latest proposal?

Yeah @domenic I totally forgot to post them :-)

The "platform context" seems to forbid synchronous resolving as in your example above - technically there is user code on the execution stack. It would disallow exposing platform code for explicit requests to resolve a promise immediately.

Gonna try a proposal myself now (still basically what we currently have, but clarifying some aspects):

onFulfilled or onRejected must not be called in a descendant execution context of the call to then [4.1].

note 1: This does enforce asynchronity. A user can rely upon the callback not to have been invoked before then returns.
In practical terms, an implementation will use a "macro-" or "micro-turn" scheduling mechanism such as setTimeout, setImmediate, Object.observe, or process.nextTick to ensure that onFulfilled and onRejected are invoked on a new execution stack. However, a trampoline mechanism that invokes them in the same turn of the event loop, from a parent execution context of the call to then, is also permitted.

[erights]

Why do you want to permit that? It would enable plan interference attacks.

[bergus]

@erights

Why do you want to permit that?

There are examples above.

It would enable plan interference attacks.

I don't really understand what you mean, and have never seen that term. Could you please link to a reference, or elaborate? I've found your post on esdiscuss, but I don't see how that applies here. Can you make an example of how a promise implementation conforming to the proposal would support malicious code?

[erights]

http://erights.org/talks/promises/paper/tgc05.pdf
http://erights.org/talks/thesis/markm-thesis.pdf especially chapter 4 and part 3

http://www.infoq.com/presentations/Secure-Mashups-in-ECMAScript-5 with slides at
https://es-lab.googlecode.com/files/RemainingHazardsAndMitigatingPatterns.pdf
Plan interference attacks starting on slide 44

[bergus]

@erights: Right, I've already read them once but forgot that term. And I should've use GoogleScholar to search :-/

However, I don't see how my proposal would change anything in regards to that attack vector. What exactly are you referring to? Nested publication bugs cannot happen due the nature of promises - they are only resolved once. Nested subscription (with out-of-order propagation) is prohibited by point 6 of the spec, and "aborting the wrong plan" is not dealed with explicity, yet implied by " must be called".

[erights]

Let's take nested publication. Under your proposal, the callback might still happen after the .then returns but before the for-loop is finished.

[briancavalier]

@erights I also am having trouble seeing the problem.

the callback might still happen after the .then returns but before the for-loop is finished

By "for-loop" here, I'm assuming you mean the for loop that is running queued callbacks. Sorry if I've misunderstood.

It seems like this is a problem only if the callback is allowed to run out of order, or is allowed to abort the for loop by throwing. Both of those seem easy to deal with. Maintaining all callbacks in a trampoline queue will preserve the order--that is, any callbacks passed to some then from within another callback can simply be added to the end of the queue to preserve order. Always calling handlers within a try/catch will prevent aborting the loop.

@bergus' latest wording, which is a clarification on our current "not before then returns" and my first attempt, seems pretty good to me. The note mentioned micro and macro tasks seems clear. Some folks may scratch their heads if they haven't heard those terms before, but we can certainly link to some useful information to help them understand.

[bergus]

@briancavalier 👍

The for-loop thing is implementation-specific, and whether you are preventing bugs by wrapping in try-catch and queuing (which you have to do anyway) or by scheduling each single callback on a new event loop turn should be left open.

[erights]

Perhaps I am misunderstanding. I am fine with a trampoline-based implementation if there are no user stack frames below it. Does this wording somehow imply this?

The for-loop I'm referring to is in the example code being attacked, not in the implementation.

[bergus]

if there are no user stack frames below it

Yeah, but I think the spec should be open enough to allow even that. Synchronous resolve calls won't be the standard, but they can be useful in some situations (I've called them "tail resolution" above). A conforming library won't need, but should be allowed to offer such functionality.

The for-loop I'm referring to is in the example code being attacked, not in the implementation.

OK, it seems we misunderstood you then - sorry. Could you post that example code, please, to ensure that we talk about the same things?

[domenic]

To clarify the case for "synchronous resolve calls": if you are already at least one turn past the original call to then, because e.g. you went off and did some I/O, it would be faster if you could immediately invoke all fulfillment or rejection callbacks right then and there, instead of queuing up at the end of the current turn. Notably, this does not break any of the example invariants we've produced in this thread, i.e. the ones labeled (1), (2), (3), etc. Here are some examples:

const fs = require("fs");

// This is part of a library packaged with the promise implementation,
// so it has access to internal underscored properties. As such, it
// bypasses the usual way of creating and manipulating promises in
// favor of assumedly-faster internal state manipulation.
export function promisifiedReadFile(fileName) {
   const promise = new Promise((resolve, reject) => {});

   fs.readFile(fileName, (err, data) => {
     if (err) {
       promise._reason = err;
       promise._state = "rejected";
     } else {
       promise._value = data;
       promise._state = "fulfilled";
     }

     // This *synchronously* fires any `onFulfilled` or `onRejected` callbacks
     // that have been added via `promise.then`. Since we know that file I/O
     // always takes at least one tick, we know that we are past the tick in which
     // `promise.then` was called. But, we're not in a clean-stack situation.
     promise._fireSettledCallbacks();
   });

   return promise;
}

// This is an example of a promise implementation which allows any consumer to violate
// the clean stack invariant, but does *not* allow generic synchronous resolution.
// It does this by tracking ticks, ensuring that `onFulfilled` and `onRejected` are never
// called in the same tick as `then`.
//
// Apologies for all the missing details, but hopefully you see the idea...
export function Promise(resolver) {
  try {
    resolver(resolve, reject);
  } catch (e) {
    reject(e);
  }

  function reject(r) {
    this._rejectionHandlers.forEach(function (onRejected) {
      if (onRejected._isNextTick) {
        // synchronously call and deal with `onRejected`
      } else {
        // do it asynchronously
      }
    });
  }

  this._rejectionHandlers = [];

  this.then = function (onFulfilled, onRejected) {
    onRejected._isNextTick = false;
    process.nextTick(() => onRejected._isNextTick = true);
    this._rejectionHandlers.push(onRejected);

    // ...
  };
}

[domenic]

@erights, would love your comments on the "synchronous resolve calls" use case, or some example code that helps us all understand the plan interference attack idea as it applies to this question.

[domenic]

I am becoming increasingly convinced that accommodating the cases I give above are important for callback-performance parity, since they avoid doing an "extra" nextTick if you're already doing something asychronous.

[briancavalier]

Hmmm, so you're saying that an implementation could essentially provide some API (e.g. the setting of onRejected._isNextTick in your example) that allows someone to break the "no user space execution contexts" guarantees. Although, I think in your example, the consumer could also break callback ordering guarantees since each onRejected is allowed to have it's own _isNextTick (e.g. set an earlier-then'ed one to true and a later-then'd one to false).

Similarly, in some discussions about IndexDB compatibility in when/issues#148, it was suggested that a library could expose a runAllHandlersNow() type API that would synchronously execute (i.e. before runAllHandlersNow() returns) all handlers whose promise has settled. While I'm not crazy about that idea (it can be abused, obviously, and lead to hazards), it seems like a lib could do that, as long as, outside that API call, it obeys Promises/A+. Allowing a pluggable scheduler is another approach, and one that I like better.

That said, tho, doesn't our current wording "not before then returns" allow your example? I.e. since it runs the handlers synchronously inside the resolver's public API and not inside then. Seems like a variant of the now-familiar "case 3" discussed way up thread.

[domenic]

Hmmm, so you're saying that an implementation could essentially provide some API (e.g. the setting of onRejected._isNextTick in your example) that allows someone to break the "no user space execution contexts" guarantees.

It could go further than that. It could be that resolve() or reject() always tries to synchronously call the appropriate callbacks, assuming at least one tick has been passed.

That said, tho, doesn't our current wording "not before then returns" allow your example?

Yes indeed. Our current wording does allow this, but it also allows the not-so-desirable case 3.

---

In practice, here's the difference:

/// (6)
function example6() {
    return new Promise(function (resolve) {
      console.log("6A");
      resolve();
      console.log("6B");
    });
}

example6().then(function onFulfilled() {
  console.log("6C");
});

This must give 6A, then 6B, then 6C. But, in contrast:

(7)
function example7() {
    return new Promise(function (resolve) {
      setImmediate(function () {
        console.log("7A");
        resolve();
        console.log("7B");
      });
    });
}

example7().then(function onFulfilled() {
  console.log("7C");
});

This could give 7A, 7B, 7C or it could give 7A, 7C, 7B. That is, resolve() could synchronously call onFulfilled, since a tick has already passed, in order to get a speed-up. Note that this is a separate case from case (3) above.

[briancavalier]

It could go further than that. It could be that resolve() or reject() always tries to synchronously call the appropriate callbacks, assuming at least one tick has been passed.

That is interesting. So the assumption here is that if you are using promises to represent operation that will complete asynchronously, then resolve/reject must have been called from a tick other than the one in which all calls to then on the related promise have occurred? (and possibly additionally, after all then calls in the same tick as resolve/reject).

Def seems worth thinking through this ... a few questions spring to mind immediately:

1. What about calls to then that occur after the resolve/reject?
2. What about the notion that assimilating in a future tick is the right thing? Hmm, perhaps since we're "already" in a future tick, the assimilation can happen synchronously too, as long as it doesn't break anything else?

[juandopazo]

Yes, that's what DOM Promise used to do with an optional sync flag in resolve. It's gone now for some reason.

[domenic]

Sorry for taking a break on this. I think it's the main thing holding us up on the 1.1 release.

What about calls to then that occur after the resolve/reject?

Those of course need to be nextTicked. The logic inside then looks something like: "if settled, nextTick to fire the appropriate callback just passed in. otherwise, push on to list of callbacks." And the logic inside resolve/reject looks something like "if it's been a turn, call immediately. Otherwise, nextTick this procedure."

What about the notion that assimilating in a future tick is the right thing? Hmm, perhaps since we're "already" in a future tick, the assimilation can happen synchronously too, as long as it doesn't break anything else?

Right, hrm. We need to go back to that original test case you wrote: kriskowal/q@3f32632

var fulfilled = adapter.fulfilled();

function when(x) {
    return fulfilled.then(() => x);
}

var i = 0;
var p = when({
    then(cb) {
        cb(i);
        // vs
        // setTimeout(() => cb(i));
    }
});

i++;

p.then(console.log);

(The desire is for this to always log 1, no matter if it's cb(i) or setTimeout(() => cb(i)).)

I believe this is unaffected. Following my above internal heuristics for how then/resolve/reject would behave, since fulfilledPromise is already settled, fulfilled.then knows to nextTick the () => x call and resulting assimilation, and so 1 is logged.

[domenic]

I would like to close this, this weekend. I think the correct step here is to strengthen our requirement very slightly from the current one, to something like @bergus's. It is still stronger than our current one, but weaker than the complete clean-stack requirement. If we ever get a more concrete reason why we need the completely-empty-stack requirement, we'll release another spec revision with that strengthening.

I will create a new pull request tonight-ish.

[domenic]

@bergus's wording:

onFulfilled or onRejected must not be called in a descendant execution context of the call to then

does not work:

// (3'')
var resolveP;
function x() {
  var p = aFulfilledPromise.then(() => { then(onFulfilled) { resolveP = onFulfilled; } });

  a();
  p.then(c);
}

x();
resolveP();
b();

An implementation could follow that wording and still do a-c-b. The execution context tree for such an implementation would be as follows:

• main execution context
  • x declaration
  • x call context
    • the call to then
  • resolveP call context
    • the call to c
  • the call to b

That is, "the call to c" does not happen in a child execution context of the call to then. (Nor does it happen in a child of "the same execution context in which then was called," which was my preferred choice.)

Argh.

[domenic]

Note also that @briancavalier's idea:

onFulfilled and onRejected must not be called in the execution context (or child thereof??)

• of the corresponding call to then
• in which promise transitions from pending to fulfilled or rejected (this wording may also require changing 'If/when' in 3.2.5.1/2 to "After")

prohibits the synchronous-resolve-if-already-async optimization I want to preserve. So that's out.

[domenic]

My latest try was:

onFulfilled or onRejected must not be called until the execution context stack contains only platform code.

Here "platform code" means engine, environment, and promise implementation code.

This seems to prohibit the following:

(7)
function example7() {
    return new Promise(function (resolve) {
      setImmediate(function () {
        console.log("7A");
        function x() {
            resolve(); // oops, execution context stack contains `x`
        }
        x();
        console.log("7B");
      });
    });
}

example7().then(function onFulfilled() {
  console.log("7C");
});

which I would like to work.

[domenic]

And I believe that, even if make "end" of event loop turn clearer, my original phrasing in this pull request

onFulfilled or onRejected must not be called before the end of the event loop turn in which then is called

breaks trampolining, as can be seen from this comment.

Getting discouraged here. Have to take a step back.

[domenic]

The only thing I can think of is along the lines: "Either one event loop turn must have passed, or only platform code must be in the execution context stack."

[domenic]

Starting over with a new statement of the problem in #139.