Sequencing of promise resolution not being tested

[getify]

I have run across a bug (thanks to @smikes) in my native-promise-only promise polyfill that is not caught by the current promises/a+ test suite (which NPO already passes).

I just checked the open issues here, and the issue I've found may indeed be the same as, or related to, that reported in #59. However, my test case is expressed a little differently, so documenting here separately just in case they are two different things needing to be tested.

In particular, I have a latent bug in the internal scheduler of NPO (which I've already diagnosed and understand) that affects how the sequencing of two different promise then(..) handlers are fired compared to what would be expected.

Here's the test case:

T1

var resolveP1, rejectP2;

(new Promise(function(resolve, reject) {
    resolveP1 = resolve;
}))
.then(function(msg){
    console.log(msg);
});

(new Promise(function(resolve, reject) {
    rejectP2 = reject;
}))
.catch(function(msg){
    console.log(msg);
});

rejectP2("A");
resolveP1("B");

According to my understanding of the spec, and according to v8, the print out should be "A B".

My unfixed library prints out "B A". When I correct my scheduler bug, I get "A B" as I would expect.

The problem, as stated above and in #59, is that NPO is already passing all 872 tests for its then(..) behavior, even with this sequencing bug, because the test suite is apparently not asserting anything about this expected sequencing (though it is clearly implied by the spec).

What concerns me is that it seems like maybe this is more than just one test that's missing, but potentially a whole class of tests that make assertions about then-sequencing semantics between independent promises.

OTOH, perhaps the promises/a+ spec isn't specific enough about such inter-promise semantics, and maybe that accounts for the shortfall of tests.

[getify]

And another test case (which looks like it should be the same, but definitely isn't):

T2a

var resolveP1, rejectP2, p1, p2;

p1 = new Promise(function(resolve, reject) {
    resolveP1 = resolve;
});
p2 = new Promise(function(resolve, reject) {
    rejectP2 = reject;
});

rejectP2("B");
resolveP1("A");

p1.then(function(msg){
    console.log(msg);
});

p2.catch(function(msg){
    console.log(msg);
});

Again, should print "A B". In mine, it prints "B A" at the moment. [update: actually, not convinced what it should print, see messages below]

[getify]

And a variation of the second test with different behavior.

T2b:

var resolveP1, rejectP2, p1, p2;

p1 = new Promise(function(resolve, reject) {
    resolveP1 = resolve;
});
p2 = new Promise(function(resolve, reject) {
    rejectP2 = reject;
});

rejectP2("B");
resolveP1("A");

setTimeout(function(){
    p1.then(function(msg){
        console.log(msg);
    });

    p2.catch(function(msg){
        console.log(msg);
    });
},0);

Pretty clear this should print "A B", right?

[getify]

Actually, the more I think about it, the more I think T2a (without the timeout) should be "B A" and T2b (with the timeout) should be "A B".

Here's my reasoning:

rejectP2("B") schedules p2 to reject with "B" on the next cycle. Then resolveP1("A") schedules p1 to resolve with "A" on that same next cycle.

So the queue looks like: "rejectP2 -> resolveP1".

For T2a, before that scheduling queue completes, both p1 and p2 get handlers registered, but the order in which those are registered is irrelevant because neither has been dispositioned yet (waiting on the next cycle).

When that next cycle runs and processes the queue, p2's rejection is handled first, which should print "B", then p1's resolution is handled next, which then prints "A".

With T2b (where the timeout defers the then/catch registration until the next cycle, sorta), the order of registering the p1.then and p2.catch definitely does matter, because both of them have already been dispositioned earlier on that same cycle.

Is that analysis correct for test cases T2a and T2b?

[smikes]

The reference implementation prints "A B" for all three cases.

I think what happens in To2A is:

1. the rejectP2 and resolveP1 happen synchronously, leaving both P1 and P2 in a 'settled' state
2. then on a settled promise queues either the resolve or reject handler immediately, so what winds up controlling the order is the sequence that the then functions are executed.

The tests are rough right now, but I will polish them a bit and add them to the sequencer PR.

[smikes]

The tests are now polished and can be run under mocha in the promises-unwrapping project, branch smikes:sequencer-test

Link to the PR - domenic/promises-unwrapping#111

[briancavalier]

The two promises in the original post are completely independent. Promises/A+ doesn't make any attempt to lock down the order of that situation, and imho, it shouldn't. Developers shouldn't make any assumptions about invisible relationships between independent promises. If they did, then any nondeterminism or timing change due to refactoring upstream could break their code or cause subtle bugs. The point of then is to make strict ordering guarantees, so not using then means all bets are off.

The discussion over in promises-aplus/promises-spec#77 might be worth a read, even though the original case is slightly different.

[getify]

Brian, the problem with that perspective is that the order is observable with Promise.race

[domenic]

Promise.race is not part of Promises/A+...

[getify]

well, the discussion of what expected semantics (for promise.race) is important even if a+ isn't the place where the test ends up.

moreover, Promise.race isn't really exposing its own sequencing semantics, it's exposing a fundamental characteristic of the promise itself (how it puts stuff on the event queue).

[briancavalier]

the problem with that perspective is that the order is observable with Promise.race

Ah, interesting point. It's outside of A+, as @domenic said, but you could raise the discussion on an ES6 Promise forum.

[getify]

IOW: we have to define the sequencing semantic somewhere, because diffs in implementation affecting how promise.race behaves are intolerable. But we can't define only what promise.race semantics are, because the side effects are observable even without race. Therefore, some body (either a+ or tc39) has to define how two promises sequence.

[domenic]

It's already defined by TC39, in the ES6 spec.

[stefanpenner]

it should be a -> b and as such race([a,b]) -> a and race([b,a]) -> b. Settled order is irrelevant, we only care about stable external observation. If we race settled promises causal ordering is irrelevant rather the order of observation is relevant.

[getify]

I am here with this conversation because I already went to the ES6 spec and I did not find (it's quite possible I didn't know how to find what I was looking for) much of anything that suggests the proper sequencing semantic between independent promises (plenty said about sequencing within a promise), other than the language about "Tasks" and "Task queues".

If my reasoning below is somehow wrong by what the spec already says, I would certainly appreciate being pointed to the specific sections.

My understanding is that the calls to the rejectP2(..) and resolveP1(..) each insert Tasks into the Task queue, in the order they are called. To me, that means that the rejectP2(..) call inserts a Task into the queue, and then resolveP1(..) inserts another Task into the queue strictly after it. On the next event loop cycle, it process that Task queue in order, right?

If so, that means that the first Task in the queue is to disposition P2 (with rejection). If there are any handlers attached to P2 at that moment, it must call those handlers right then, correct?. In T2a, there is already a catch(..) handler registered on P2, so it must call its error callback right then, correct? Therefore, it would have to print "B" first.

The next Task in the queue is to disposition P1 (with success). If P1 has any handlers registered at that moment, it must call those handlers right then, correct? In T2a, a then(..) handler has already been registered on P1, so it must call that callback right then, right? Therefore, it would next print "A".

Setting aside the discussion of race(..) for a moment, I do not see how the spec gives any indication that T2a should result in "A B". It all seems to point to "B A".

Please help me understand what I'm missing that insists on "A B" as the proper result for T2a.

[domenic]

If there are any handlers attached to P2 at that moment, it must call those handlers right then, correct?

Incorrect, it queues a task to call those handlers. See TriggerPromiseReactions in the spec.

[getify]

OK, but it still queues them in the "B A" order. Or... not?

[domenic]

No; the handler for p2 is queued to run after the handler for p1, so it should be in A B order.

The reference implementation is made for exploring these kinds of things, BTW. Stepping through it in a debugger can give the clearest answers.

[getify]

For posterity and anyone who cares, I have tracked down (the hard way) what my misunderstanding was.

The problem was I was thinking in T2a that rejectP2(..) and resolveP1(..) were enqueuing tasks, which would mean their respective ordering was the important part. They don't, because their respective promises do not have any "PromiseReactions" yet registered. So all they essentially do is disposition their respective promises.

Thus, the tasks enqueued (with respect to this situation) come only from the .then(..) and .catch(..), which means that its their respective ordering that matters.

Hence, "A B".

[briancavalier]

@domenic and @getify thanks for pointing that out in the es6 spec and explaining!