diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b367a99c..47886c62b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,10 @@ project adheres to [Semantic Versioning](http://semver.org/). (Unreleased) ================== ### Changed +* Upgrade node-addon-api from 7.x to 8.x ### Added ### Fixed +* Fix memory leak caused by N-API weak reference callbacks being deferred to SetImmediate instead of running during GC. Enabled `NAPI_EXPERIMENTAL` to use `node_api_nogc_finalize` for ObjectWrap destructor. (#2436) 3.2.3 ================== diff --git a/binding.gyp b/binding.gyp index fe99f998a..20568e913 100644 --- a/binding.gyp +++ b/binding.gyp @@ -58,7 +58,7 @@ { 'target_name': 'canvas', 'include_dirs': ["(info), en return; } _pattern = cairo_pattern_create_for_surface(surface); + _source = Napi::Persistent(obj); if (info[1].IsString()) { if ("no-repeat" == info[1].As().Utf8Value()) { @@ -127,3 +128,7 @@ repeat_type_t Pattern::get_repeat_type_for_cairo_pattern(cairo_pattern_t *patter Pattern::~Pattern() { if (_pattern) cairo_pattern_destroy(_pattern); } + +void Pattern::Finalize(Napi::Env env) { + _source.Reset(); +} diff --git a/src/CanvasPattern.h b/src/CanvasPattern.h index 1f768e03b..55a0119b9 100644 --- a/src/CanvasPattern.h +++ b/src/CanvasPattern.h @@ -26,8 +26,10 @@ class Pattern : public Napi::ObjectWrap { static repeat_type_t get_repeat_type_for_cairo_pattern(cairo_pattern_t *pattern); inline cairo_pattern_t *pattern(){ return _pattern; } ~Pattern(); + void Finalize(Napi::Env env); Napi::Env env; private: cairo_pattern_t *_pattern; + Napi::Reference _source; repeat_type_t _repeat = REPEAT; }; diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc index 9c5482074..56ad99000 100644 --- a/src/CanvasRenderingContext2d.cc +++ b/src/CanvasRenderingContext2d.cc @@ -244,6 +244,9 @@ Context2d::Context2d(const Napi::CallbackInfo& info) : Napi::ObjectWrap { void resetState(); inline PangoLayout *layout(){ return _layout; } ~Context2d(); + void Finalize(Napi::Env env); Napi::Env env; private: diff --git a/test/memory.test.js b/test/memory.test.js new file mode 100644 index 000000000..2c2e633ef --- /dev/null +++ b/test/memory.test.js @@ -0,0 +1,36 @@ +/* eslint-env mocha */ + +'use strict' + +// These tests require --expose-gc. Skip gracefully if not available. +const gcAvailable = typeof gc === 'function' + +const assert = require('assert') +const { createCanvas } = require('../') + +describe('Memory management', function () { + before(function () { + if (!gcAvailable) this.skip() + }) + + it('Canvas objects are freed by GC', function () { + const ITERATIONS = 100 + const SIZE = 1024 // 1024x1024 ARGB = 4 MiB per canvas + + for (let i = 0; i < ITERATIONS; i++) { + const canvas = createCanvas(SIZE, SIZE) + const ctx = canvas.getContext('2d') + ctx.fillStyle = 'red' + ctx.fillRect(0, 0, SIZE, SIZE) + gc() + } + + gc() + gc() + + // 100 canvases x 4 MiB = 400 MiB if leaked. + // With proper GC, RSS should stay well under 150 MiB. + const rssMiB = process.memoryUsage().rss / 1024 / 1024 + assert(rssMiB < 150, `RSS is ${rssMiB.toFixed(0)} MiB, expected < 150 MiB`) + }) +})