tc39 / proposal-cancelable-promises

Former home of the now-withdrawn cancelable promises proposal for JavaScript
Other
376 stars 29 forks source link

Race between downward settlement and upward cancelation #8

Closed domenic closed 8 years ago

domenic commented 8 years ago

https://github.com/domenic/cancelable-promise/commit/19b48e28d768d84cff8c2b69f61f710376eb9394 adds a test which illustrates an issue with the upward-propagating cancel signal. I wanted to get people's thoughts, especially @jakearchibald and @petkaantonov.

In short, here is the problem:

const root = Task.resolve(5);
const generation1 = root.then(() => delay(100));
const generation2 = generation1.then();

generation2.cancel("boo");

What would you expect the (eventual, after 150 ms) states of each promise to be?

For me, I would expect root to be fulfilled with 5, generation1 to be fulfilled with undefined, and generation2 to be canceled. Does this match your intuition?

However, this is not what happens. At the time generation2.cancel is called, generation1 is currently pending and unresolved. Only after a microtask passes, and the () => delay(100) function runs, would generation1 get resolved to the promise returned by delay(100). But by then it is too late, as the cancelation signal has reached it and it is instead canceled.

In other words, the cancelation signal propagates upward "faster" than promise settlement propagates downward, because the latter has to wait for a microtask checkpoint.

Note that if you modify the last line to be

delay(0).then(() => generation2.cancel("boo"));

it will behave according to my intuition.


What are peoples' thoughts? Is this a problem? I'm not entirely certain myself.

If it is, any suggestions on how best to solve it?

Is this actually really bad? E.g. do you think this kind of downward-vs.-upward race is fatal to the idea of upward-propagating cancelation signals, and we should just give up and go with downward-propagating cancel tokens?

domenic commented 8 years ago

Not doing tasks.

benjamingr commented 8 years ago

I think this is an interesting discussion regardless.

For me, I would expect root to be fulfilled with 5, generation1 to be fulfilled with undefined, and generation2 to be canceled. Does this match your intuition?

No, but I'm sort of biased towards what bluebird is doing. I would expect the delay to possibly run but both generation1 and generation2 to be in a cancelled state.

bergus commented 8 years ago

Does this match your intuition?

No. I believe that both generation2 and generation1 should be cancelled. I even believe that delay(100) should not be called at all, since everything is cancelled before the then callback would be called asynchronously. If it already had been called at the time of the cancellation, I'd expect the returned delay promise to be cancelled as well.

pfrazee commented 7 years ago

Note that if you modify the last line to be delay(0).then(() => generation2.cancel("boo")); it will behave according to my intuition.

Would it be reasonable to make all cancels automatically wait for a turn of the event loop, just like resolves do?

jakearchibald commented 7 years ago

@pfrazee this proposal has been withdrawn, so there's not a lot of point in discussing it further.

That said, resolves don't wait for a turn of the event loop. See https://jakearchibald.com/2015/tasks-microtasks-queues-and-schedules/

pfrazee commented 7 years ago

@jakearchibald Ok. Thanks for the post, that's good to know about.