Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Corecii opened this issue May 31, 2024 · 0 comments
Open

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

Corecii opened this issue May 31, 2024 · 0 comments

Comments

@Corecii
Copy link

Corecii commented May 31, 2024

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.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant