Memory leak in async/await

[iliakan]

Here's the code. Heap usage grows:

async function run() {
  let idx = 0
  const thresh = 1e5
  const limit = 1e6
  while (idx++ < limit) {
    const record = await get()
    if (idx % thresh == 0) {
      console.log('heap-used:', process.memoryUsage().heapUsed)
    }
  }
}

async function get() {
  return {
    foo: 'foo',
    bar: 'bar'
  }
}

run();

Run with node --harmony-async-await, for Node 7.0.0.

P.S. If you remove async/await from function get, everything is fine. Without async/await it's also so much faster!

P.P.S. Original issue babel/babel#4382

[Fishrock123]

@nodejs/v8

[vkurchatkin]

I think it might be the same problem as: #6673

[jeisinger]

could you please file a bug on crbug.com/v8/new

[iliakan]

@jeisinger filed https://bugs.chromium.org/p/v8/issues/detail?id=5582

[iliakan]

Okay, seems fixed in a more recent V8. May I know the schedule for 5.5? :)

[iliakan]

Guess this patch is required to fix the leak: https://bugs.chromium.org/p/v8/issues/detail?id=5390

[iliakan]

FYI, the fix for https://bugs.chromium.org/p/v8/issues/detail?id=5390 made into V8 version -- 5.5.232.0

[ChALkeR]

Okay, seems fixed in a more recent V8. May I know the schedule for 5.5? :)

Per https://www.chromium.org/developers/calendar, V8 5.5 release is expected around 2016-12-06.
It will then need either a semver-minor or a semver-major Node.js release to introduce it in Node.js, depending on whether it would be an API/ABI breaking update or not.

[vkurchatkin]

I think it should be possible to fix it in babel as well

[gsathya]

Possibly interesting for babel -- leebyron/async-to-gen#26

[navinSing]

do we really need babel,, when nodejs 7.0.0 is out ?

[benjamingr]

This is a known issue with v8. It has been verified and fixed in 5.5 which should be integrated later on.

Quoting @littledan

I'm glad to hear so much excitement about async/await. However, Node v7 is based on V8 54, whose async/await implementation has some bugs in it (including a nasty memory leak). I would not recommend using --harmony-async-await in production until upgrading to V8 55; the easiest way to achieve that would be to wait for Node 8. A backport of everything to Node v6 would be non-trivial; there would be a lot of merge conflicts. Until Node 8 comes out, I'd recommend using a transpiler.

nodejs/promises#4 (comment)

[benjamingr]

As there is no workaround from the Node side I am aware of and we don't plan to realistically backport this v8 patch without a v8 upgrade I think the issue should be closed.

If anyone has anything else to add feel free to reopen.

[jeffijoe]

Too bad, was looking forward to ditching Babel in favor of native async-await soon.

[littledan]

Sorry about this historical bug!

[ChALkeR]

@jeffijoe

Too bad, was looking forward to ditching Babel in favor of native async-await soon.

You shouldn't use harmony flags in production, when a feature is ready and stable enough — it's enabled by default. If something is disabled — it is either not finished, has issues, or was not tested enough.

Specifically async/await is enabled in V8 5.5, which should be released in December, and will get into some Node.js release after that, either 7.x.0 or 8.0.0, depending on whether there would be API/ABI incompatibilities in the V8 update.

[jeffijoe]

@ChALkeR right, my too bad was aimed at the fact that it might remain behind a flag until Node 8.

[mgamsjager]

So this seems to be closed a long time ago.
Running the example still results in a memory leak:

 node -v: v11.15.0

With Async:
> run();
Promise { <pending> }
> heap-used: 337587480
heap-used: 67905672
heap-used: 57640192
heap-used: 100826320
heap-used: 98119280
heap-used: 94505664
heap-used: 122870192
heap-used: 192377968
heap-used: 148503872
heap-used: 211715008

Wihtout Async on the get():
> run();
heap-used: 5114248
heap-used: 5511560
heap-used: 5513640
heap-used: 5515192
heap-used: 5517008
heap-used: 5518560
heap-used: 5520112
heap-used: 5521664
heap-used: 5523704
heap-used: 5525256

[dr-js]

@mgamsjager it's not actual leaking, but small sample size with random lazy GC

Try the following edited code with bigger loop size and manual GC trigger:

const testFunc = () => {
  const run = async () => {
    let idx = 0
    const thresh = 1e6
    const limit = 1e9 // increased loop count
    while (idx++ < limit) {
      const record = await get()
      if (idx % thresh === 0) {
        global.gc() // trigger GC
        console.log('heap-used:', process.memoryUsage().heapUsed, record)
      }
    }
  }
  const get = async () => ({ foo: 'foo', bar: 'bar' })
  run()
}

process.exit(require('child_process').spawnSync( // run with a GC-enabled node process
  process.argv0,
  [
    '--expose-gc', // allow `global.gc()` call
    '--max-old-space-size=4', // limit max memory usage
    '--eval', `(${testFunc})()`
  ],
  { stdio: 'inherit' }
).status)

Or the shell command if you do not want a file:

node --expose-gc --max-old-space-size=4 --eval "$(cat <<- 'EOM'
  const run = async () => {
    let idx = 0
    const thresh = 1e6
    const limit = 1e9 // increased loop count
    while (idx++ < limit) {
      const record = await get()
      if (idx % thresh === 0) {
        global.gc() // trigger GC
        console.log('heap-used:', process.memoryUsage().heapUsed, record)
      }
    }
  }
  const get = async () => ({ foo: 'foo', bar: 'bar' })
  run()
EOM
)"

My test result end up stays the same with nodejs@12.14.1 on WSL:

...
heap-used: 1695936 { foo: 'foo', bar: 'bar' }
heap-used: 1695936 { foo: 'foo', bar: 'bar' }
heap-used: 1695936 { foo: 'foo', bar: 'bar' }

and with nodejs@13.11.0 on Win10 x64

...
heap-used: 1801880 { foo: 'foo', bar: 'bar' }
heap-used: 1801880 { foo: 'foo', bar: 'bar' }
heap-used: 1801880 { foo: 'foo', bar: 'bar' }