Closed Bug 1569445 Opened 6 years ago Closed 6 years ago

Promise chains keep promises alive unnecessarily

Categories

(Core :: JavaScript: Standard Library, task)

task
Not set
normal

Tracking

()

RESOLVED WONTFIX

People

(Reporter: jorendorff, Unassigned)

Details

https://twitter.com/ForbesLindesay/status/1152251673756295169

function recurse() { return Promise.resolve().then(() => recurse()) };
const x = recurse();

The memory usage grows without bound in the SM shell.

It doesn't need to; at any moment, at most the first promise (x) and the two most recently created promises are live.

The Promise state machine goes

pending --> pending but resolved --> fulfilled

with the middle state often being skipped. But this example is all about the "pending but resolved" state, which means "my resolve hook was called but they passed me another unfulfilled promise so I am still waiting".

Pending-but-resolved promises are always tied together in sets; in each set, one promise is pending but not yet resolved, and the rest are pending but resolved. (Each promise can only be resolved once.)

The example creates an ever-larger set of pending-but-resolved promises. The newest promise is the unresolved one.

All but two or three of the promises in this set are dead. They can't become accessible to JS ever again. But because our implementation stores this set as a long chain of strong references, we don't GC the dead objects.

This is hard to fix because the spec is very complex, with observable interactions everywhere. We implement it line-by-line because otherwise we would surely have bugs.

The only fix I see is like this:

  1. In PerformPromiseThen, detect if the arguments indicate we're putting onFulfilled.[[Promise]] in the pending-but-resolved state (i.e. we've been called from PromiseResolveThenableJob).

  2. If so, store the PromiseReactions created by PerformPromiseThen differently.

  3. During GC mark phase, use a trace hook (?) to collapse chains of these reactions.

The best part: suppose we do this. A program creates a chain of ten million promises; we flatten it to a very broad shallow tree, to avoid keeping all the promises alive unnecessarily. We now have a bug, because the spec requires the chain to become resolved in a particular order, as though the optimization wasn't there. So in addition to all of the above, we would have to store an integer indicating how many links were optimized away, and on fulfillment, deoptimize accordingly.

I don't think this is realistically fixable given comment 2.

Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → WONTFIX
You need to log in before you can comment on or make changes to this bug.