It seems rather suboptimal to have Promise.race() hang forever when given an iterable with no items (likely leaking the returns promise and any promises chained to that, since they will always be 'pending').
If given an iterable with no items, it would be better to reject with a TypeError (as Array.prototype.reduce does in a similar situation). You might also resolve with 'undefined', I suppose. Either way the promise is resolved and the chained promises can be resolved and then disposed.
(If Promise.race() continues to take only an immediate array, not a promise for an array (see issue 2514), then the TypeError could be thrown synchronously. I think it would be more consistent to allow Promise.race() to accept a promise and to always throw the TypeError asynchronously.)
This is intentional and has been discussed before.
https://github.com/domenic/promises-unwrapping/issues/75
I've added my two cents over in https://github.com/domenic/promises-unwrapping/issues/75 but to briefly recap here:
I strongly disagree with writing a resource leak into the spec. I think Promise.race() should throw TypeError. But if you want to make it hang forever for reasons of "algebraic consistency" then I think the spec needs to be updated to allow promises chained after a Promise.race([]) to be disposed.
I think the simplest way of doing that is to add a new 'forever-pending' value for [[PromiseStatus]]. We can then observe this status and avoid creating references to uncallable handlers in the implementation of Promise.prototype.then. A rough draft of how this might be done:
25.4.4.4 Promise.race:
add between steps 5 and 6, "let seen be false"
step 6c of Promise.race, "if next is false, then: (1) if seen is false, return NewForeverPendingPromise(C). (2) return promiseCapability.[[Promise]]."
add step 6j, "set seen to true"
25.4.5.3 Promise.prototype.then ( onFulfilled , onRejected )
add between steps 19 and 20, "Else if the value of promise's [[PromiseStatus]] internal slot is "forever-pending", return NewForeverPendingPromise(C)"
25.4.6 Properties of Promise Instances
add "forever-pending" to list of acceptable values of [[PromiseStatus]]
add "or not 'forever-pending'" to the end of the description of [[PromiseResult]]
Add new helper, "NewForeverPendingPromise(C)" with the following definition:
1. Let promiseCapability be NewPromiseCapability(C).
2. ReturnIfAbrupt(promiseCapability).
3. Let promise be promiceCapability.[[Promise]]
3. If promise does not have a [[PromiseStatus]] internal slot, or its value is not undefined, then throw a TypeError exception.
4. Set promise's [[PromiseStatus]] internal slot to "forever-pending".
5. Return promise.
Reopening and retitling bug to describe how to avoid the resource leak if the infinite hang is actually desired.
The algorithm in comment 2 is not quite right. For example:
class TimeoutPromise extends Promise {
constructor(resolver) {
super((resolve, reject) => {
setTimeout(() => reject(new TimeoutError()), TIMEOUT);
return resolver(resolve, reject);
});
}
}
TimeoutPromise.race([]); // this should actually reject
The easiest solution is to only return a new NewForeverPendingPromise if C is %Promise%.
But a better solution is to probably use weak references carefully such that the only strong references to a promise P are held by its resolve and reject functions. For example, all references in a PromiseCapability (held by PromiseReaction, etc) would be weak. If done carefully, a "forever pending" promise would magically become gc'able when it is no longer possible to resolve or reject it, as in Promise.race([]), and disposing that promise would then eliminate the references to the resolution functions of other promises chained with `then`. That is, the goal would be that after:
var p = Promise.race([]).then(function(){...}).then(function(){...});
only 1 promise (and none of the anonymous onFulfill functions) should be live.
Hm. I think I'm about to eat my words.
I drew a little diagram of where all the strong references are, and I think
var p = Promise.race([]).then(function(){...}).then(function(){...});
already works as I wanted, according to the spec. The two cases are:
var p = new Promise(function(f, r) { ... }); // case 1
In case 1, f and r hold the only references to p, exactly how we'd like.
var p2 = p1.then(f, r); // case 2
In case 2 we create a number of objects:
1) pc, a PromiseCapability for p2
2) pr1, a rejection PromiseReaction, which holds pc and r
3) prh, a Promise Resolution Handler, which holds p1, f, and r
4) pr2, a resolution PromiseReaction, which holds pc and prh
And then p1 is made to hold pr1 and pr2.
So if p1 becomes unreachable (and its resolution functions are unreachable, as in the case of the result of Promise.race([])), then pr1 and pr2 are also unreachable, which makes pc, prh, f, and r unreachable. Assuming that no one has stolen copies of pc's [[Resolve]] and [[Reject]] functions, then p2 is unreachable as well, just as we want.
So p2 ultimately isn't holding any sort of reference to p1, and unresolvable promises ultimately become unreachable. QED.
I still think that Promise.race([]) is a debugging nightmare. But at least it's not a resource leak.
I would still caution you against taking the spec's descriptions of the data structures in play as having any actual impact on what strong references are present in the system. Only the observable semantics matter, not the exact algorithmic steps given in the spec.
I don't like the API as a developer, but I'm convinced there isn't an inherent resource leak here. Resolving as invalid.