Fabric.js: nodejs, memory leak with toDataURL

[peernohell]

Version

fabric: 2.2.2
node: v6.10.3

Test Case

https://gist.github.com/peernohell/db5513b45e015a5ed7f2b4b860211413

Steps to reproduce

copy the gist.
npm install fabric
node index.js --expose-gc

Expected Behavior

providing as first object an empty object or an object with a small image should have the same memory footprint.

Actual Behavior

If you execute the code as is you will see in the terminal that the RSS is growing (to ~1.2Go)
If you uncomment the lien obj = {} (line 55) then the RSS grow to ~500Mo then go down.

here is what I have in my terminal,
With obj uncomment

❯ node --expose-gc index.js
START  103
IMG0  506
IMG1  506
IMG2  506
END  506
RSS  104
RSS  104
RSS  77
RSS  76

with obj with the image

❯ node --expose-gc index.js
START  105
END  125
IMG0  526
RSS  526
IMG1  888
IMG2  1285
RSS  1284
RSS  1284
RSS  1284
RSS  1283

I know that the RSS is not a good metric to detect leak, but on our production server, node take so much memory that it hit all the free memory and crash.

[asturur]

Will try to run it tomorrow.
No idea if we are disposing the image loaded in anyway on node, if we should do it of jsdom do it in some other way

[asturur]

Ok found! we are clearing the canvas elements, but not the context, that hold a referent to the canvas.

a 2 line changes in canvas.dispose will solve.

[acemtp]

@asturur Would love to see those 2 lines we have to change so we can test in our production server

[asturur]

in canvas.dispose of StaticCanvas add this.contextContainer = null and this.cacheContext = null

in canvas.dispose of Canvas add this.upperContext = null

if you use text elements too there is another reference to take care of that is fabric._measuringContext with what i m unsure what to do.

[acemtp]

Thank you so much for your reactivity ❤️