Memory leak in Image

[kkaefer]

When constructing an Image, memory for that image seems to never be released:

var fs = require('fs');
var Canvas = require('canvas');
var Image = Canvas.Image;

var filename = 'image.png';
var data = fs.readFileSync(filename);

function load() {
    var img = new Image;
    img.onload = function() {
        process.nextTick(load);
        global.gc();
    };
    img.src = data;
}

load();

This code uses global.gc() to force garbage collection, but even then, memory rises very quickly. When moving the var img = new Image; to outside the load function (and effectively reusing it) changes memory usage behavior to near constant.

[chearon]

https://github.com/Automattic/node-canvas/blob/master/src/Image.cc#L268

I think it's just cause it's not cleaning up the old onload handler

edit: never mind, you're creating a new image every time. hmmm

[kkaefer]

There is no previous onload handler in this example. I added cleanup locally, but it doesn't solve the issue. It seems to be related to img being referenced inside the callback; somehow the closure holds a reference back to the img object, so due to the circular reference it never gets cleaned up.

[kkaefer]

For reference, the most reliable way I've found (apart from fixing this issue by marking the onload/onerror callbacks as weak when no load actions is in progress) is this:

img.onload = function() {
    process.nextTick(function() {
        img.onload = null;
    });
    // ...
});

[chearon]

The only thing in load's closure is the data buffer though and the callback doesn't reference anything except load, which only references the data buffer. I can't see how there would be leftover references to img. You might have stumbled on a real issue with Image not cleaning up its memory in C when it gets destructed

[chearon]

Oh, I see now. But how is the callback referencing img...

[kkaefer]

Not in this particular example, but in most real-world code, you retain a reference to the Image object so you can access it. If you wouldn't retain a reference, loading the image doesn't have any point.

[kkaefer]

NaN::Callback creates a persistent handle that is retained. In most real-world code that uses one-time callbacks, that persistent reference is destroyed after the callback is called, but using onload is different in that the lifetime of the callback goes beyond its invocation (e.g. you can use the same Image object multiple times with the same callback)

[tonbz]

I got 2 gig of memory after loop created 150 image objects. even assign null to all of it after render process finish still gave me same result of memory leak. so I dig deep to image.cc and found that the solution is simple. Just re assign source of image to null, image object will clean itself then I got my memory back :) (it might trigger onerror event because image load with nothing)

var img = new Image;
    img.onload = function() {

      //do anything you want ...

      //clean it by change src to null
      img.src = null;
    };
    img.src = data;

[diwu1989]

we hit this issue too and had to explicitly null out the callbacks... 😢

[zbjornson]

I can't seem to reproduce this, with node.js 6.x, using your code @kkaefer

var fs = require('fs');
var Canvas = require('.');
var Image = Canvas.Image;

var filename = './test/fixtures/clock.png';
var data = fs.readFileSync(filename);

var max = 5000;

function load() {
var img = new Image;
    img.onload = function() {
        if (--max > 0) process.nextTick(load);
        global.gc();
        if (max % 50 === 0) {
            var mem = process.memoryUsage();
            console.log(mem.rss, mem.heapUsed, mem.external);
        }
    };
    img.src = data;
}

load();

From both the process.memoryUsage stats and my process monitor, memory stays pretty constant over 5k runs, with that 27 KB image from the test directory. Is this still happening for anyone?