Unexpected execution order with multiple parents

[sokra]

While trying to ensure consistency between webpack's and v8 implementation I stumbled over this case:

// async.mjs
console.log("async 1");
await 0;
console.log("async 2");

// a.mjs
import "./async.mjs";
console.log("a");

// b.mjs
import "./async.mjs";
console.log("b");

// x.mjs
import "./a.mjs";
console.log("x");

// index.mjs
import "./a.mjs";
import "./b.mjs";
import "./x.mjs";
console.log("index");

index -> a -> async
      -> b -> async
      -> x -> a -> async

Without the TLA this will behave as:

async 1
async 2
a
b
x
index

and since every module depends on the completion of the TLA, I would expect it to have the same output. The remaining graph would just execute sync after the await has finished. And I implemented it this way in webpack.

But to my surprise v8 doesn't behave this way. Instead it behaves like this:

async 1
async 2
a
x  <-- diff
b  <-- diff
index

So the question is who is right? I tried to read the spec and to me it seem like the spec also thinks that this is the spec'ed behavior. Am I reading the spec correctly? Is this is intended behavior? I also thought the remaining graph would execute in the same order as sync execution when there is no TLA in between. Is this a bug in the spec?

[guybedford]

Thanks for the very clear example here, this is effectively a "short-circuiting" behaviour of the execution ordering in the fulfillment handlers, and I can appreciate that it could seem an unintuitive deviation from the concept of the post-ordering.

It's unclear if there is scope to make such a change at this stage, but if there was enough implementer concensus it could still be possible.

I've posted an approach for applying this invariant in #159, which should achieve the behaviour you are looking for here.

[sokra]

I found another case where the behavior is questionable. This time I tend more towards this being a v8 bug, but maybe you could help to confirm this. It's related to circular dependencies between c1 and c2:

// async.mjs
console.log("async before");
await 0;
console.log("async after");

// c1.mjs
import "./c2.mjs";
import "./async.mjs";
console.log("c1");

// c2.mjs
import "./c1.mjs";
console.log("c2");

// x.mjs
import "./c2.mjs";
console.log("x");

// index.mjs
import "./c1.mjs";
import "./x.mjs";
console.log("index");

index --> c1 <--> c2
              --> async
      --> x --> c2 <--> c1 --> async

Intuitively expected output:

c2
async before
async after
c1
x
index

v8:

c2
async before
x
async after
c1
index

x is executed way too early. Since c1 is the cycle root, I would expect it to execute after c1 (and async and c2) has completed.

Note: This only happens when c2 don't contain an await. Adding a if(false) await 0; fixes the problem.

[sokra]

Also tested with on SpiderMonkey, which leads to yet another result:

c2
async before
async after
x
c1
index

[guybedford]

@sokra in the above, the v8 output is correct per the spec while the SpiderMonkey output is a bug. The reason the exeuction happens this way is because async executions do not stop parallel execution of further dependencies in order not to be blocking - the async exec is started, then the algorithm moves on to execute the next post-order module that it can.

[sokra]

hmm... I would see c1+c2+async as strongly connected component, so x will only be executed when the strongly connected component has finished. This would be consistent with sync execution flow.

x transitively depends on async and is not part of a cycle, so in my opinion it must execute after async.

There is also this note in the spec text:

NOTE 2
Any modules depending on a module of an async cycle when that cycle is not evaluating will instead depend on the execution of the root of the cycle via GetAsyncCycleRoot. This ensures that the cycle state can be treated as a single strongly connected component through its root module state.

c1 is the root of the cycle, so when x depends on c2 (which is part of the cycle) it should instead depend on the state of the root of the cycle: c1.

[guybedford]

Ah sorry I missed that c1 and c2 were in a cycle here, yes in that case, x should only execute after both c1 and c2 per the spec since attachment to async completions on the already-visited graph is based on GetCycleRoot.

(so yes this is a v8 bug!)

[guybedford]

I think it would be benefical to include these two test cases or very similar in test-262, and leave this issue open to track that, regardless of the outcome of #159.