[Question] Scheduling issue with node ?

[vicb]

Based on a comment from @jakearchibald.

Jack I had decided to look at the implementation for setImmediate() and took an other look at your comment:

setImmediate(_ => console.log(1));
Promise.resolve().then(_ => console.log(2));
setImmediate(_ => console.log(3));

The above will currently log 1,2,3, is the aim of this PR to make it 1,3,2 if zone.js is present? If so, I'm a little worried about it as it breaks es6.

It seems wrong when I looked at it again this morning:

• setImmediate() should enqueue on the macrotask queue,
• .then() should use the microtask queue,
• the expected result is then "2, 1, 3" - which it is when you executed this code with node 0.12

Now looking at the code again, asap() indeed uses setImmediate() on node 0.10. So when run with node 0.10, the result of

setImmediate(_ => console.log(1));
ES6Promise.Promise.resolve().then(_ => console.log(2));
setImmediate(_ => console.log(3));

will be "1, 2, 3" which is not expected.

I'm unsure if there is a better way to enqueue a microtask with Node 0.10 (as nextTick could not be used according to the comment).

If I'm right, this looks like an issue of ES6Promise ? if so it should probably be documented

edit: some background info on process.nextTick() - TL;DR: pre-0.9, it enqueues a macrotask.

[vicb]

confirmed via testing the last snippet from the previous comment:

• node 0.10.38 -> "1, 2, 3"
• node 0.12.2 -> "2, 1, 3"
• ES6 (for ref) -> "2, 1, 3"

/cc @stefanpenner

[stefanpenner]

nextTick on node 0.10 has a serious bug for long chains between two different producers.
see for more details: tildeio/rsvp.js#337

the trade-off is extremely crappy :(

[vicb]

Thanks for the reference.

Do we have any other choice than the current implementation ? If not we should at least document that

• with node <= 0.10, scheduling is different from ES6 Promise,
• with node >= 0.11, scheduling is the same as ES6 Promise (they've been introduced in node 0.12),
• if the browser fall-back to setTimeout(), scheduling will be different from ES6 Promise.

(Note that 1 is actually 2 folds, <=0.9 nextTick is used but act as latter setImmediate (enqueues a macrotask), 0.10 setImmediate is used which enqueues a macrotask)

Should I submit PRs to rsvp / es6-promise adding a "Known issues" section ?

[stefanpenner]

Do we have any other choice than the current implementation ?

We can release a version without the work-around for the node 0.10 problem. In all honesty though node 0.10 is quite old, this problem has literally be fixed upstream for years.

Seems like the bugfix is for people to upgrade to node 0.12 or later

Ultimately, I can be convinced of taking either direction.

[vicb]

If we can override the scheduler, we can pick a default behavior and make it easy to override it.

[vicb]

We can resolve this one by:

• document the "issue",
• document how to revert to a proper scheduling once Expose asap() and a way to override the scheduling mechanism on Promise #116 is merged by forcing the use of nextTick (and warn user about potential issues that come along)

[stefanpenner]

We can resolve this one by:

document the "issue",
document how to revert to a proper scheduling once #116 is merged by forcing the use of nextTick (and warn user about potential issues that come along)

sounds good, i may have time later this week.

(thanks for the clear steps to resolve)

[vicb]

I could try to draft something (most probably on Friday) but I'm not sure how to be able describe the issue accurately (i.e. interaction between different schedulers).

Anyway, if you fell it could helpful for you to have something to start with/review let me know and I'll prepare this.

[stefanpenner]

If we can override the scheduler, we can pick a default behavior and make it easy to override it.

note: the scheduler is now overridable -> #116

the question really is, should we sacrifice correctness for an edge case that only affects some users on node 10? e.g. should we just revert back to the previous mode. Especially if the schedule is overridable, they can always "work around it".

[vicb]

My idea was to keep the current behavior and explain to node 10 users how they can update and the limitations

[stefanpenner]

My idea was to keep the current behavior and explain to node 10 users how they can update and the limitations

seeems reasonable.

[vvo]

@vicb @stefanpenner What's the workaround for node 0.10 users you are talking about?

[vicb]

@vvo with node 0.10 es6-promises uses setImmediate() because nextTick() has some issues

We should indicate to users of this lib how to switch back to using nextTick() but warn them about the limitations when doing so.

[stefanpenner]

@vicb thinking about this, i wonder if we should just move to nextTick always, and let the failure scenario happens. No other promise lib handles the failure case either.

[vicb]

Not sure it's worth breaking BC for older systems ?
Anyway let me know what you decide and I'll submit a PR.

[stefanpenner]

i believe reverting that would merely restore it to be no worse then any other promise lib on that version of node