evaera / roblox-lua-promise

Promise implementation for Roblox
https://eryn.io/roblox-lua-promise/
MIT License
275 stars 74 forks source link

Promise.all can wait infinitely if passed an array with holes #98

Open Corecii opened 3 months ago

Corecii commented 3 months ago

Issue

Promise.all counts and loops through the given items in a manner that may be inconsistent when the array has holes, potentially causing it to wait infinitely or not wait at all for some items.

Suggested Fix

Make Promise.all ignore holes.

We already loop through all items in the table once to ensure all items are promises. We can use this as a chance to collect all integer-keyed items then loop through those. This is the fix with the closest match to today's behavior.

An alternative is to loop through all items, including non-integer-keyed entries. This is too big of a divergence from today's behavior, and could cause significantly different behavior if anyone is presently giving Promise.all mixed tables.

Version Bump

I'm planning to publish the "ignore holes" fix as a minor or patch version bump. This is technically a change in behavior, but it's really fixing a bug in a case that was never guaranteed to work as it does. This is in the realm of "if you're relying on this, you're relying on undefined behavior".

I'll leave some time to receive any comments before going forward with this fix. I suspect no one is knowingly relying on the behavior as it is today.

(fun fact: I'm writing this down because it caused an active problem in Adopt Me which took a while for a few engineers to diagnose! I wish that nobody else has to deal with such a problem. I wish the type system could have caught this, but it's not ready for Promise just yet.)