Repository navigation
Memory leaks in loops with Promise #6673
Description
Activity
/cc @nodejs/v8
Is this because each promise keeps a reference to the next?
@Fishrock123
Doesn't look like that, looks like mark-sweep isn't called for whatever reason.Could be a duplicate of #6180.
Perhaps it's even fixed by #6398 in the current master, but I can't check atm.Update: it's not, #6180 isn't fixed yet.filed https://bugs.chromium.org/p/v8/issues/detail?id=5002
Firefox also runs out of memory, so maybe it's a spec thing
according to the crash, MS is executed.
@jeisinger
It's not a spec thing, adding a manualgcevery e.g. 1e7 iterations fixes it.
Looks very similar to #6180 — scavege is triggered instead of mark-sweep, but the memory is reclaimed only by mark-sweep, not by scavenge.Update: I made a mistake in the test, shame on me =).
@jeisinger If it's a spec thing it's an issue because I don't know a better way to do a loop with promises :/
@petkaantonov What's your opinion on this?
@jeisinger No idea why the mark-sweep at the end doesn't reclaim most of that, though.@Fishrock123 @jeisinger I made a mistake in the test, ignore my previous comments. Sorry for the inconvenience.
If it's a spec thing it's an issue because I don't know a better way to do a loop with promises :/
Unbounded recursive loops are typically not safe in JavaScript at the current time, promise or not apparently.
Yes, it looks like all Promise objects are retained in the heap. There are twice more of them than the number of iterations, btw.
twice more of them than the number of iterations, btw.
makes sense, 2 new promises are created on each iteration
Unbounded recursive loops are typically not safe in JavaScript at the current time, promise or not apparently.
Yes but there are no other way to do async loops than using recursion (without tricks likes
setImmediate()to break the stack).makes sense, 2 new promises are created on each iteration
Ah, yes, that is correct.
28 remaining items
@bnoordhuis update to which version ? With v.7 I mean 7.10. If you mean that I should update to node v.8 that may be a problem
@danielo515 either v6.x or v8.x, both are LTS release lines.
Thanks @bnoordhuis , I'll give a try to v8.x
Can anyone explain the result of this conversation? I'm using "Node v8.9.2" but there is still a leak for cyclic use Promise or async / await functions.
@fenixphp do you mean that you experiment the leak just with promises or that you experiment it both with promises and async functions ?
Theoretically this only happens on promises because they keep a chain that grows wild, which is not the case of async functions.Thanks @danielo515, Experimentally, I came to the conclusion that promises begin to leak as the call chain looks like a tree. What can I replace the promises in a pair of async / await functions?
You can use async await function calls inside an infinite loop without any problem. But if you use recursion you will end with the same stack size problem.
- added a commit that references this issue
on Apr 21, 2018 Reading this thread, I am getting a little worried and confused.
The following code will make me able to pile tasks wherever in my codebase, and make sure that each task is executed only after all the previous ones are done.
function task() { //dummy async treatment return delay(100); } let stack = Promise.resolve('the stack is empty'); function addTask() { return stack = stack.then(task); }- When repeatedly calling addTask over the time, like every second, will I have a memory leak?
- If I do, how should I work around this?
Thanks for your help
Reacted by Abdel Atencio, kolya182 and Mikis WoodwinterFrom what I tested, the pattern for promise memory leaking must include a tail-recursive setup, the promise creating function always recursively appear in itself's
promise.thencode.
The recursive code cause the promise implementation need to keep extra reference/stack info so the GC can not claim those already-resolved promise from the chain.If we build a long/infinite promise chain gradually without the recursive code pattern, and only keep reference to the tail of the chain, the GC can took those un-referenced promise from the chain, whether the promise is pending or not.
So I think chaining promise up is not always unsafe, it depends on the actual code.
And if the promise version ofwhile (true) { /* async code */ }is needed, better use aasyncfunction instead of the tail-recursive promise pattern.
First part of test is 2 different OOM sample code and a fix attempt:
- [OOM] tail-recursive promise setup, GH sample, edit & formatted.
- [OOM] tail-recursive promise setup, edit & formatted.
- [SAFE] no-recursive promise setup
Then some common promise chain use pattern is tested:
- [OOM] holding the head & tail promise of a chain of pending promise
- [OOM] holding the head-resolve & tail promise of a chain of pending promise
- [SAFE] holding the tail promise of a chain of pending promise
- [SAFE] holding the head & tail promise of a chain of resolving promise
- [SAFE] holding the tail promise of a chain of resolving promise
The test code I used: (and an sample output from
v13.11.0 x64 win32)console.log('Testing Env:', process.version, process.arch, process.platform) const spawnFuncWithExposeGC = (...funcList) => { const { status } = require('child_process').spawnSync( process.argv0, [ '--expose-gc', // allow `global.gc()` call '--max-old-space-size=64', // limit max memory usage for faster OOM '--eval', `(${funcList.reduce((o, func) => `(${func})(global.gc, ${o})`, 'undefined')})` ], { stdio: 'inherit' } ) console.log(`process exit with status ${status} `.padEnd(64, '=')) console.log('\n') } const commonFunc = (triggerGC) => { const setTimeoutAsync = (wait = 0) => new Promise((resolve) => setTimeout(resolve, wait)) const formatMemory = (value) => `${String(value).padStart(10, ' ')}B` const markMemory = async () => { triggerGC() await setTimeoutAsync(10) triggerGC() const { heapUsed, heapTotal, rss, external } = process.memoryUsage() console.log([ `heapUsed: ${formatMemory(heapUsed)}`, `heapTotal: ${formatMemory(heapTotal)}`, `rss: ${formatMemory(rss)}`, `external: ${formatMemory(external)}` ].join(' ')) } const appendPromiseAdder = (promise, count = 0) => { let index = 0 while (index++ !== count) promise = promise.then((result) => (result + 1)) return promise } return { setTimeoutAsync, markMemory, appendPromiseAdder } } spawnFuncWithExposeGC(async () => { console.log('[OOM] tail-recursive promise setup, GH sample, edit & formatted. https://github.com/promises-aplus/promises-spec/issues/179#issuecomment-93453094') const run = (i) => new Promise((resolve) => setImmediate(resolve)) .then(() => { if (i % 1e5 === 0) console.log({ i }) return i < 99999999 ? run(i + 1) : i }) await run(0).then((result) => console.log(result)) }) spawnFuncWithExposeGC(async () => { console.log('[OOM] tail-recursive promise setup, edit & formatted. https://alexn.org/blog/2017/10/11/javascript-promise-leaks-memory.html') const signal = (i) => new Promise((resolve) => setImmediate(() => resolve(i))) const loop = (n) => signal(n).then(i => { if (i % 1e5 === 0) console.log({ i }) return loop(n + 1) }) await loop(0).catch(console.error) }) spawnFuncWithExposeGC(commonFunc, async (triggerGC, { markMemory }) => { console.log('[SAFE] no-recursive promise setup') let i = 0 let promiseTail = Promise.resolve() const token = setInterval(() => { // simulate user input or other outer timer adding batch of task to the queue if (i >= 1e8) return clearInterval(token) // check finish let n = 0 while (n++ !== 1e5) { promiseTail = promiseTail.then(() => { i = i + 1 if (i % 1e6 !== 0) return // check log console.log({ i }) markMemory() }) } }, 0) }) spawnFuncWithExposeGC(commonFunc, async (triggerGC, { setTimeoutAsync, markMemory, appendPromiseAdder }) => { console.log('[OOM] holding the head & tail promise of a chain of pending promise') const promiseHead = new Promise((resolve) => {}) let promiseTail = promiseHead let loop = 0 while (loop++ !== 128) { await markMemory() promiseTail = appendPromiseAdder(promiseTail, 64 * 1024) await setTimeoutAsync(10) } console.log({ promiseHead, promiseTail }) }) spawnFuncWithExposeGC(commonFunc, async (triggerGC, { setTimeoutAsync, markMemory, appendPromiseAdder }) => { console.log('[OOM] holding the head-resolve & tail promise of a chain of pending promise') let pendingResolve let promiseTail = new Promise((resolve) => { pendingResolve = resolve }) let loop = 0 while (loop++ !== 128) { await markMemory() promiseTail = appendPromiseAdder(promiseTail, 64 * 1024) await setTimeoutAsync(10) } console.log({ pendingResolve, promiseTail }) }) spawnFuncWithExposeGC(commonFunc, async (triggerGC, { setTimeoutAsync, markMemory, appendPromiseAdder }) => { { console.log('[SAFE] holding the tail promise of a chain of pending promise') let promiseTail = new Promise((resolve) => {}) let loop = 0 while (loop++ !== 128) { await markMemory() promiseTail = appendPromiseAdder(promiseTail, 64 * 1024) await setTimeoutAsync(10) } console.log({ promiseTail }) console.log('\n') } { console.log('[SAFE] holding the head & tail promise of a chain of resolving promise') const promiseHead = new Promise((resolve) => { resolve(0) }) let promiseTail = promiseHead let loop = 0 while (loop++ !== 128) { await markMemory() promiseTail = appendPromiseAdder(promiseTail, 64 * 1024).then((result) => { console.log({ result }) return result }) await setTimeoutAsync(10) } console.log({ promiseHead, promiseTail }) console.log('\n') } { console.log('[SAFE] holding the tail promise of a chain of resolving promise') let promiseTail = new Promise((resolve) => { resolve(0) }) let loop = 0 while (loop++ !== 128) { await markMemory() promiseTail = appendPromiseAdder(promiseTail, 64 * 1024) promiseTail = promiseTail.then((result) => { console.log({ result }) return result }) await setTimeoutAsync(10) } console.log({ promiseTail }) console.log('\n') } })
Reacted by Perry, Rolf Lear and munepom- added 2 commits that reference this issue
on Aug 26, 2020
The code above increasingly consumes memory until it crashes with:
With Bluebird, the used memory never goes above 30MB and the program does not crash: