Memory leak when used with JSDOM (and fabricJS)

[asturur]

Hello,

I received the following issue on the fabricJS issue tracker.
fabricjs/fabric.js#5102

If you spend 2 minutes reading it, it talks about a memory consumption increase.

A way to solve it, partially, is to reach the node-canvas object behind a JSDOM image or canvas and manually dereference it.

Is there a proper way to dispose a node-canvas object?

[Hakerh400]

Can you provide a minimal test case without any external modules like fabric, jsdom or xmldom? From the linked issue, it seems that the problem is in HTMLImageElementImpl not being GC-ed properly, but it belongs to jsdom, not node-canvas.

However, I succeeded to reproduce a small leak using the following code:

const {Canvas} = require('canvas');

while(1){
  new Canvas(640, 480);
  global.gc();
}

It grows from 12MB to 50MB in ~15min. Here are the results (obtained using ProcessExplorer). Not sure if this is related to the issue you're experiencing.

The leak that happens with this code doesn't seem to be coming from node-canvas directly, but rather from the cairo library. My guess is that cairo internally uses some data structures or cache tables of limited size, which are part of their implementation. I didn't perform long enough test, but I hope it will not grow unlimitedly.

[asturur]

I understand the leak can come from multiple sources, i started here to ask if there is a dispose/clear method that you may know of. I ll post on jsdom issue tracker too.

[Hakerh400]

I left the test I posted in comment 1 to run over night. Afetr about 10 hours, the Node.js process crashed with OOM error. However, the error was not fatal and was properly caught

new Canvas(640, 480);
^

Error: out of memory
    at Object.<anonymous> (C:\Projects\JavaScript\canvas\main.js:4:1)
    at Module._compile (internal/modules/cjs/loader.js:689:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:700:10)
    at Module.load (internal/modules/cjs/loader.js:599:32)
    at tryModuleLoad (internal/modules/cjs/loader.js:538:12)
    at Function.Module._load (internal/modules/cjs/loader.js:530:3)
    at Function.Module.runMain (internal/modules/cjs/loader.js:742:12)
    at startup (internal/bootstrap/node.js:266:19)
    at bootstrapNodeJSCore (internal/bootstrap/node.js:596:3)

Made heapdumps several times during the test. The heap is not growing at all, but the rss is growing constantly. Also I made Node.js process memory dump. In the heap (not v8 heap) there are a lot of 32-byte sequences allocated, but not freed. That is the only thing that takes up more than 2GB of memory.

I investigated Canvas::~Canvas destructor and ImageBacked destructor, but couldn't find any place where the leak may occur. It almost definitely comes from C++ land. Probably there is either a leak in the cairo library, or we are not properly cleaning up the space after cairo_surface_t deallocation.

[asturur]

Can i ask a question?
Does a while(1) loop give chance to the GC to actually do work?

I always asked myself this question.

Anyway the memory leaks i get are fare bigger but there are more things involved

[Hakerh400]

Does a while(1) loop give chance to the GC to actually do work?

It should.
If new Canvas(640, 480) is replaced with Buffer.alloc(1e6) no leak occurs at all, even after 14 hours.
Also, GC may be implicitly called synchronously when there is no more free space on the v8 heap.
GC is supposed to be called synchronously.

the memory leaks i get are fare bigger

Probably these two leaks are not the same.

[Hakerh400]

Update 13 Aug 2018

I think there are no leaks in node-canvas. The leak that was brought by my script from comment 1 is probably related to Node.js bug with global.gc function
(seems to be already reported and fixed in nodejs/node#͏22241)

With the fix from 22241, no leak seems to occur. I tried different tests: creating canvases in a loop, registering fonts, creating different contexts, image data, buffers from canvas, etc. All the tests were running for 120 hours and the memory stays at the same level.

The original leak is, as it seems, related to jsdom's objects HTMLImageElementImpl not being deallocated. The reason is probably that they use some permanent reference or something like that. Also, from the logical point of view, if HTMLImageElementImpl instance contains a reference to the Canvas object, even if Canvas continues to live in the memory, it should not prevent HTMLImageElementImpl to be GC-ed, since Canvas itself doesn't contain reference to their object.

As the question was "Is there a proper way to dispose a node-canvas object?", the answer is no, every Canvas object (and it's context) should be GC-ed when there are no more references to it. Memory cleanup happens implicitly.

IMO, since the issue is missing dependency-free repro steps, I don't see what we can do about it. Probably the next step would be reporting the issue to the jsdom repo and closing this one.

[asturur]

Thank you, i will prepare a jsdom only leaking script and i ll open an issue there, referencing this one.