2.2.2.2 fails when resolving synchronously outside of .then's event-loop-turn

[avih]

This is the test for 2.2.2.2:

        var d = deferred();
        var isFulfilled = false;
        d.promise.then(function onFulfilled() {
            assert.strictEqual(isFulfilled, true);
            done();
        });
        setTimeout(function () {
            d.resolve(dummy);
            isFulfilled = true;
        }, 50);

I think the spec says that onFulfilled should be called with a clean stack with regards to the then, but not necessarily WRT resolve. Spec 2.2.4 says:

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

Where note 3.1 clarifies:

In practice, this requirement ensures that onFulfilled and onRejected execute asynchronously, after the event loop turn in which then is called, and with a fresh stack.

Depending on interpretation, specifically of "and with a fresh stack" but mostly because then's context is mentioned explicitly while resolve/reject are not, if d.resolve(dummy) executes onFulfilled synchronously, which at this test is not at the same execution context where then was called, then I think the spec says it's OK, but this test will fail because it expects onFulfilled to be at a different context to resolve (i.e. that isFulfilled = true; runs before onFulfilled)

Personally I think that the spec should enforce fully async of resolve/reject as well (like the test expects), and even further that two onFulfilled/onRejected should also not be called at the same execution context, but if an implementer tries to squeeze every ounce of speed by using synchronous executions where the spec allows, then I think this test should pass if resolve executes onFulfilled synchronously (same for the equivalent reject and possibly other tests).

[domenic]

The spec seems pretty clear that no matter what causes onFulfilled to trigger, any onFulfilled that is passed to then must never be called with a dirty stack.

[avih]

That's the interpretation issue I was referring to. I think it would have been clearer if the spec said "after the event loop turn which triggered the call, be it then or resolve or reject"

[domenic]

The spec doesn't real with resolve or reject though. But it does deal with every onFulfilled passed to then.

[avih]

True that...

[sterpe]

Hi all,

This is also related to the interpretation of "after the event loop turn in which then is called, and with a fresh stack" as well so thought I'd pop it in here rather than new issue.

My promise implementation uses an internal trampoline queue (to cover the fresh stack/platform code aspect) & passes all current compliance tests but I'm concerned that I did not correctly implement the after event loop turn requirement...there doesn't seem to be any actual coverage of that requirement when dealing with adding a fulfilment handler to the same promise within a fulfillment handler to that promise.

I've modified a test from 2.2.4 to illustrate:

  describe("Clean-stack execution ordering tests (fulfillment case)", function () {
       ....
        specify("when one `onFulfilled` is added inside another `onFulfilled`", function (done) {
            var promise = resolved();
            var firstOnFulfilledFinished = false;

            promise.then(function () {
        var x
        ;
        process.nextTick(function () {
            x = 1;
        });
                promise.then(function () {
                    assert.strictEqual(firstOnFulfilledFinished, true);
            assert.strictEqual(x, 1);
                    done();
                });
                firstOnFulfilledFinished = true;
            });
        });

Am I correct in assuming that A+ implementation would pass this test: I.e. then called in a fulfillment handler is fulfilled in the next event loop turn?

[stefanpenner]

@sterpe I believe this being left unspecified is intentional

[domenic]

@stefanpenner nope.

@sterpe that test is a bit tricky because it's interleaving two "async queues": process.nextTick and the promise queue. The test of firstOnFulfilledFinished is more useful than the test of x, because it says literally whether the promise queue has unwound. The test of x also tests that the nextTick queue has unwound. But that is not necessary for conformance because the stack will still be clean.

You can picture it this way:

promise queue: [], nextTick queue: []
promise queue: [<set x to undefined>], nextTick queue: [<set x to 1>]
promise queue: [<set x to undefined>, <assert x === 1>], nextTick queue: [<set x to 1>]

At this point a bunch of stuff has been enqueued, but no other code is ready to run, so our stack is empty. Now it's time to start dequeing stuff. Let's say that things have worked out so that the promise queue comes before the nextTick queue. Then we have:

run <set x to undefined>
promise queue: [<assert x === 1>], nextTick queue: [<set x to 1>]

stack is again empty.

run <assert x === 1> (it will fail)
promise queue: [], nextTick queue: [<set x to 1>]

stack is again empty

run <set x to 1>
promise queue: [], nextTick queue: []

done; no more queued event loop work items. Node can now exit; browser can now GC; etc.

So this shows how an implementation can be conforming but still fail the x === 1 test.

[stefanpenner]

@stefanpenner nope.

I must be missing something because you followed this up with an explanation as to why this is left unspecified.

[domenic]

Well, I guess it depends on perspective. But this is not an unspecified loophole. This is a very well-specified issue.

[stefanpenner]

@domenic ah ok. I then believe my wording was incorrect. unspecified -> untested

[sterpe]

@domenic, thanks for the clarification. I suppose then there's an unnecessary resolution speed penalty that an implementation would have to take in this situation waiting for the next tick queue to unwind in order to pass the 'x' test.

[sterpe]

Or rather potential performance impact since you are ceding the thread back between the on fulfillment handler resolution.