31

Will awaiting a Promise which neither resolves nor rejects (never settle/unfulfilled) cause a memory leak?

I got curious about this while looking at React hooks with slorber/awesome-debounce-promise that creates new promises, but only settles the last one of them, thus leaving many/most unsettle/unfulfilled.

OliverRadini
  • 6,238
  • 1
  • 21
  • 46
Qtax
  • 33,241
  • 9
  • 83
  • 121
  • Interesting question. I think it theorically shouldn't, since it should keep checking whether the promise resolved and, if it didn't, wait and check again. Perhaps the result would differ according to the target, though, since it depends how the promise is natively handled and/or eventually polyfilled. – briosheje Aug 08 '19 at 12:22
  • 1
    Debouncing a promise shouldn't cause it to never be resolved / rejected. It's just your ignoring them instead. So it shouldn't effect memory. For example if you created a promise and never bothered awaiting or attaching to it's thenable, the garbage collector would just clean things up. – Keith Aug 08 '19 at 12:26
  • @Keith according to the description and the usage examples it seems to await every new promise created. *"each call will return a promise"*, `const result = await searchAPIDebounced(text);`, *"only the promise returned by the last call will resolve"* (and they are not rejected either). – Qtax Aug 08 '19 at 12:29
  • No, they all will resolve or reject. What the debouce will be doing is just returning the last promise. The description is miss-leading. A badly written promise might not resolve / reject, but that would be a different problem.. – Keith Aug 08 '19 at 12:32
  • @Keith but if they reject then the `await` will throw an exception, right? Thus breaking the examples. Looking at the code, it's using https://github.com/slorber/awesome-only-resolves-last-promise. Which does exactly what was described and I assumed, never resolves/rejects all promises but the last one created. – Qtax Aug 08 '19 at 12:34
  • 1
    He's using `awesome-imperative-promise`, all this does is create a wrapper promise, the real physical promise will still reject / resolve. – Keith Aug 08 '19 at 12:37
  • *Reopen reason:* The "duplicate" question does not try to `await` the promise. Thus that question & answer does not answer this question. (Plus that one is angular specific?) – Qtax Nov 12 '19 at 13:45

2 Answers2

14

Preface (you probably know this!):

await is syntactic sugar for using promise callbacks. (Really, really, really good sugar.) An async function is a function where the JavaScript engine builds the promise chains and such for you.

Answer:

The relevant thing isn't so much whether the promise is settled, but whether the promise callbacks (and the things they refer to / close over) are retained in memory. While the promise is in memory and unsettled, it has a reference to its callback functions, keeping them in memory. Two things make those references go away:

  1. Settling the promise, or
  2. Releasing all references to the promise, which makes it eligible for GC (probably, more below)

In the normal case, the consumer of a promise hooks up handlers to the promise and then either doesn't keep a reference to it at all, or only keeps a reference to it in a context that the handler functions close over and not elsewhere. (Rather than, for instance, keeping the promise reference in a long-lived object property.)

Assuming the debounce implementation releases its reference to the promise that it's never going to settle, and the consumer of the promise hasn't stored a reference somewhere outside this mutual-reference cycle, then the promise and the handlers registered to it (and anything that they hold the only reference for) can all be garbage collected once the reference to the promise is released.

That requires a fair bit of care on the part of the implementation. For instance (thanks Keith for flagging this up), if the promise uses a callback for some other API (for instance, addEventListener) and the callback closes over a reference to the promise, since the other API has a reference to the callback, that could prevent all references to the promise from being released, and thus keep anything the promise refers to (such as its callbacks) in memory.

So it'll depend on the implementation being careful, and a bit on the consumer. It would be possible to write code that would keep references to the promises, and thus cause a memory leak, but in the normal case I wouldn't expect the consumer to do that.

T.J. Crowder
  • 1,031,962
  • 187
  • 1,923
  • 1,875
  • Doesn't `await somePromise;` keep a ref to the promise? Even if we don't have any ref to the promise elsewhere. – Qtax Aug 08 '19 at 12:42
  • 2
    @Qtax - No. `await somePromise` is basically *(lots of handwaving here)* `somePromise.then(() => { /*...the code following the await through the end of the function */).catch(reject);` where `reject` in that is the `reject` of the implicit promise that the `async` function returned. If the `somePromise` there is really a variable (not a stand-in for a function returning a promise), then there's the question of how long the reference in `somePromise` endures, but if it's (for instance) a local variable the callbacks close over, that won't prevent GC. – T.J. Crowder Aug 08 '19 at 12:43
  • _Releasing all references to the promise, which makes it eligible for GC_ Oh. I didn't know that (I mean, I didn't know it would work against promises as well), that's a good news. – briosheje Aug 08 '19 at 12:45
  • 2
    @briosheje this is the case for anything, not just promises. Releasing a reference to anything makes it eligible for GC – nem035 Aug 08 '19 at 12:46
  • @nem035 all right, just thought it actually could be different on Promises due to their nature. Your point totally makes sense, though, you never stop learning, after all, right? :D – briosheje Aug 08 '19 at 12:49
  • `Releasing all references to the promise` This one of course all depends on what the promise does. For example, if we made a silly promise called `waitForDPressed`, that basically waited until the D key was pressed, this in turn will use `addEventListener`, this will then keep this promise in memory until the D key is pressed,.. IOW: If the promise references any global's it will stay in memory too. – Keith Aug 08 '19 at 12:52
  • 2
    @Keith - That's a very good point. That's sort of a sub-case of the implementation not releasing the references to the promises it will never settle, but a somewhat hidden one I should call out explicitly above. – T.J. Crowder Aug 08 '19 at 12:53
  • 1
    @briosheje Sure thing, we're all learning every day :). The gist of garbage collection comes down to distinguishing "used" from "unused" references, and cleaning up the "unused" ones, no matter what the references point to. GC treats the idea of reference as an abstraction around any particular data type (in any language for that matter). For example, this is why, as Keith pointed out above, anything referencing a global stays in memory (because globals are always "used"), be it a promise or something else. (of course, I'm surmising everything in a short comment but that's the gist :)) – nem035 Aug 08 '19 at 12:56
  • @nem035 I was aware about the globals being always used, reason why I barely use them (unless they are always needed). Thanks for the clear explanation, GC topics are usually quite confusing (and misleading) out there :p. – briosheje Aug 08 '19 at 13:01
  • @T.J.Crowder odd that my example keeps eating memory then? – Qtax Aug 08 '19 at 13:04
  • @Qtax - See comment on your answer. – T.J. Crowder Aug 08 '19 at 13:26
  • I always had a problem with that *`await` is just syntaxtic sugar around promises thing*, and it definetly breaks in this case. When an `async function` awaits, it's execution context gets removed, then later when the promise resolves the context gets restored. From a memory usage point that makes a difference – Jonas Wilms Aug 14 '19 at 09:27
  • @T.J.Crowder excellent answer! Just wanted to added clarification for posterity, it is the _resolve_ and _reject_ handlers that typically reference the promise, and it is these references that `awesome-imperative-promise` removes (by setting their values to `null` in the current implementation) – Adam Thompson Jun 07 '20 at 17:33
8

I did some testing using the following structure:

function doesntSettle() {
    return new Promise(function(resolve, reject) {
        // Never settle the promise
    });
}

let awaited = 0;
let resolved = 0;

async function test() {
    awaited++;
    await doesntSettle();
    resolved++;
}

setInterval(() => {
    for (let i = 0; i < 100; ++i) {
        test();
    }
}, 1);

Implemented here: https://codesandbox.io/s/unsetteled-awaited-promise-memory-usage-u44oc

Running just the result frame in Google Chrome showed continuously increasing memory usage in dev tools Memory tab (but not under the Performance/JS heap tab), indicating a leak. Running this but resolving the promises did not leak.

Running this increased memory usage for me increased by 1-4MB/second. Stopping it and running the GC did not free up any of it.

Google Chrome dev tools Memory tab showing increasing usage

Qtax
  • 33,241
  • 9
  • 83
  • 121
  • 1
    Interesting, and well done throwing a test at it. I think that shows an issue with V8's `async` function implementation (they are relatively new). I don't think I'm seeing it with [the near-equivalent non-`async` version](https://codesandbox.io/s/unsetteled-awaited-promise-memory-usage-j5t2y). I'm fairly sure that theoretically, it shouldn't be leaking (even when it's an `async` function). Sure looks like it does in V8 though. – T.J. Crowder Aug 08 '19 at 13:24
  • Doesn't happen with Firefox's SpiderMonkey. – T.J. Crowder Aug 08 '19 at 13:41
  • 1
    @T.J.Crowder I'm not seeing it with Firefox either, nor Edge. So like you say it's probably a bug/feature of the V8 implementation. Interestingly in Chrome the JS heap size traced on the Performance tab of dev tools doesn't grow and seems to be GCed nicely, not going above ~16MB. – Qtax Aug 08 '19 at 14:38
  • I wonder if it relates to async stack traces... Are you doing a bug report? – T.J. Crowder Aug 08 '19 at 14:44
  • @T.J.Crowder was not planning to do a bug report (so feel free if you want to). I have async stack traces enabled, so that could be related. – Qtax Aug 08 '19 at 15:29
  • 1
    I'll go ahead and do it. Looks and smells like a bug. :-D – T.J. Crowder Aug 08 '19 at 15:41
  • 2
    Just tested this in Chrome v89 and it now appears to be garbage collecting these unresolved promises like Firefox does, so no more memory leak. – TechPrime Apr 09 '21 at 15:28