Memory leak with infinite loop built on long stacktraces and promisified function

[Rush]

Running Node 4.4.2 and latest bluebird.3.3.5.

'use strict';
let Promise = require('bluebird');

Promise.longStackTraces();

function sendPing(cb) {
  cb();
}

let promisifiedSendPing = Promise.promisify(sendPing);

function pingLoop() {
  return promisifiedSendPing()
  .then(pingLoop)
  .catch(err => {
    console.log('Caught error', err);
    process.exit();
  });
}

pingLoop();

// this is to demonstrate that the memory is growing despite the GC working
if(global.gc) setInterval(global.gc, 1);

Run with:

node --expose_gc test.js

Then observe memory usage rising:

while [ 1 ];do ps aux|grep test.js|grep -v grep ; sleep 1;done

If promisifiedSendPing is changed to Promise.delay(1) there is no memory leak.

[benjamingr]

Does it leak with stack traces disabled?

[Rush]

No. 4 and 5.

[Rush]

@benjamingr any ideas?

[benjamingr]

I think this happens on the version without long stack traces as well - but it's much slower.

[bergus]

That code is buggy and there's hardly a way to avoid growing memory - you are adding more and more catch handlers to the chain with every pingLoop call. You should either be using

function pingLoop() {
  return promisifiedSendPing()
  .then(pingLoop);
}
pingLoop()
.catch(err => {
  console.log('Caught error', err);
  process.exit();
});

or

function pingLoop() {
  return promisifiedSendPing()
  .catch(err => {
    console.log('Caught error', err);
    process.exit();
  })
  .then(pingLoop);
}
pingLoop();

[benjamingr]

@bergus oh wait you're absolutely right! The catch handlers are added after the current promise and not before it - there is nothing bluebird can do about this.

Great catch!

[benjamingr]

Both versions you posted don't leak so that's extra proof.

[Rush]

@benjamingr Unfortunately this is leaking memory. Can you re-open the ticket?

let Promise = require('bluebird');
Promise.longStackTraces();

setInterval(global.gc, 100);

function promisifiedSendPing() {
  return Promise.resolve();
}

function pingLoop() {
  return promisifiedSendPing()
  .then(pingLoop);
}

pingLoop()
.catch(err => {
  console.log('Caught error', err);
  process.exit();
});

Run with node --expose_gc test.js

[spion]

Confirmed. Also confirming that it doesn't leak without LST with node --max-old-space-size=10 for at least 90s

With LST time BLUEBIRD_DEBUG=1 node --max-old-space-size=20 leak.js dies after 2 seconds.

@petkaantonov if this is impossible to be both performant (because stacks are not stringified) and non-leaky, maybe we should limit the stack trace size to N async events...

[zorji]

I can also reproduce the memory leak with longStackTraces enabled.

// npm install bluebird faker memwatch-next

var Promise = require('bluebird');
Promise.config({
    longStackTraces: true
});
var faker = require('faker');
var memwatch = require('memwatch-next');
function memUsage() {
    var hd = new memwatch.HeapDiff();
    var diff = hd.end();
    console.log(diff.after.size);
}

// a simple database.query() mock
function query(criteria, callback) {
    var data = faker.lorem.paragraphs(5000, '');
    memUsage();
    setTimeout(function () {
        callback(null, data);
    }, 600);
}

function queryPromisified(criteria) {
    return new Promise(function (resolve, reject) {
        query(criteria, function (ex, response) {
            ex ? reject(ex) : resolve(response);
        });
    });
}
function getItems(criteria) {
    return queryPromisified(criteria).then(function (data) {
        return data ? getItems(data) : null;
    });
}
getItems({});

If I comment out the Promise to use native it doesn't have the problem.

[atdksp]

has it been fixed? or still an issue?