feat: enable Wasm weak references for automatic garbage collection

[yisibl]

Enable Weak References in the Wasm bindings. This ensures that Wasm memory is freed automatically by the garbage collector, otherwise, it will continue to grow unless the developer explicitly calls .free() on every exported instance or the instance is passed back to a function that takes ownership of it, allowing Rust to free it.

Without weak references your JS integration may be susceptible to memory leaks in Rust, for example:

• You could forget to call .free() on a JS object, leaving the Rust memory allocated.
• Rust closures converted to JS values (the Closure type) may not be executed and cleaned up.
• Rust closures have Closure::{into_js_value,forget} methods which explicitly do not free the underlying memory.

Note: Weak References are enabled by default since wasm-bindgen v2.9.1, before that you need to add the --weak-refs option.

e.g. wasm-pack build --target web --out-name index --out-dir wasm/dist --release --weak-refs

Test

https://github.com/yisibl/repro-resvg-wasm-memory-leak

---

Resolved: #216 (comment)

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
resvg-js	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Mar 25, 2024 0:23am

[yisibl]

@mizdra Please check if this PR fixes the problem.

[mizdra]

Using WeakRef is a good idea! I'll try it and see if it solves the problem.

[yisibl]

@mizdra I'm looking forward to your test results, I'm ready to release them later.

[mizdra]

I just finished work. I'm testing now.

[mizdra]

FinalizationRegistry requires Node.js v14.6.0 and Safari 14.1. Is this okay? What versions of Node.js/Safari does @resvg/resvg-wasm support?

• https://developer.mozilla.org/ja/docs/Web/JavaScript/Reference/Global_Objects/FinalizationRegistry

[mizdra]

@yisibl
It seems that the memory leak has not been resolved. Could you please check this?

Test Codes

// wasm-memory-leak-test.mjs
import { readFile } from 'fs/promises'
import { join } from 'path'

import { Resvg, initWasm } from './wasm/index.mjs'

const wasmPath = join(import.meta.dirname, './wasm/index_bg.wasm')

if (!global.gc) {
  throw new Error('You must run this script with --expose-gc')
}

function logMemoryUsage(i, rss, heapTotal, heapUsed, external) {
  console.log(
    `${i.padStart(8, ' ')},` +
      `${rss.padStart(12, ' ')},` +
      `${heapTotal.padStart(12, ' ')},` +
      `${heapUsed.padStart(12, ' ')},` +
      `${external.padStart(12, ' ')},`,
  )
}

await initWasm(readFile(wasmPath))
const svg = await readFile(join(import.meta.dirname, './example/text.svg'))
const opts = {
  background: 'rgba(238, 235, 230, .9)',
  fitTo: {
    mode: 'width',
    value: 1200,
  },
  font: {
    fontFiles: ['./example/SourceHanSerifCN-Light-subset.ttf'],
    loadSystemFonts: false,
  },
}

logMemoryUsage('i', 'rss', 'heapTotal', 'heapUsed', 'external')

for (let i = 0; i < 1000000; i++) {
  const resvg = new Resvg(svg, opts)
  const pngData = resvg.render()
  const pngBuffer = pngData.asPng()
  if (i % 10 === 0) {
    // NOTE: Do GC before measuring memory usage because the garbage is not measured.
    global.gc()
    const usage = process.memoryUsage()

    logMemoryUsage(
      i.toString(),
      usage.rss.toString(),
      usage.heapTotal.toString(),
      usage.heapUsed.toString(),
      usage.external.toString(),
    )
  }
}

Test environment

• Node.js v21.4.0
• cargo 1.76.0-nightly (978722961 2023-12-06)

Before

$ git switch main
$ node --expose-gc wasm-memory-leak-test.mjs
       i,         rss,   heapTotal,    heapUsed,    external,
       0,   131956736,     9895936,     4704664,    26211296,
      10,   178569216,     9895936,     4714272,    55639141,
      20,   213336064,     6750208,     4714864,    87424101,
      30,   244957184,     6750208,     4453344,   116128869,
      40,   275021824,     6750208,     4452648,   149552229,
      50,   305004544,     6750208,     4469672,   176770687,
      60,   334856192,     6750208,     4456840,   209889867,
      70,   366247936,     6750208,     4471112,   237412505,
      80,   396165120,     6750208,     4471896,   268893285,
      90,   427704320,     6750208,     4458584,   302164555,
     100,   464650240,     6750208,     4473336,   329383013,
     110,   495779840,     6750208,     4464816,   362654283,
     120,   525713408,     6750208,     4464272,   391359051,
     130,   557056000,     6750208,     4479936,   423144011,
     140,   586940416,     6750208,     4480656,   454928971,
     150,   618479616,     6750208,     4481376,   482147429,
     160,   648380416,     6750208,     4468152,   510700107,
     170,   678297600,     6750208,     4471016,   544123467,
     180,   708263936,     6750208,     4485288,   571494015,
     190,   738131968,     6750208,     4471368,   604613195,
     200,   769474560,     7012352,     4486856,   636398155,

After

$ gh pr checkout 318
$ node --expose-gc wasm-memory-leak-test.mjs
       i,         rss,   heapTotal,    heapUsed,    external,
       0,   125566976,    10158080,     4705576,    26211309,
      10,   170131456,    10158080,     4714128,    55639141,
      20,   220971008,     7012352,     4714656,    87576191,
      30,   253853696,     7012352,     4453552,   116128869,
      40,   285720576,     7012352,     4455256,   149400139,
      50,   315670528,     7012352,     4469528,   176770687,
      60,   345538560,     7012352,     4456224,   210041957,
      70,   376897536,     7012352,     4470968,   237412505,
      80,   406814720,     7012352,     4471688,   269045375,
      90,   438550528,     7012352,     4458440,   302316645,
     100,   473513984,     7012352,     4473192,   329383013,
     110,   504578048,     7012352,     4464736,   362654283,
     120,   534511616,     7012352,     4465176,   391359051,
     130,   565837824,     7012352,     4479856,   423296101,
     140,   595771392,     7012352,     4480576,   454928971,
     150,   627310592,     7012352,     4481232,   482147429,
     160,   657145856,     7012352,     4468336,   510700107,
     170,   687161344,     7012352,     4469984,   544123467,
     180,   717111296,     7012352,     4485144,   571494015,
     190,   746995712,     7012352,     4471288,   604613195,
     200,   778305536,     7012352,     4486776,   636398155,
^C

[yisibl]

Thanks to your detailed testing, I confirm that the problem still exists, for which I have raised an issue: rustwasm/wasm-bindgen#3903

[mizdra]

I found that my method of measuring memory leaks was incorrect. In Node.js, when global.gc() is called, the callback registered with FinalizationRegistry is not called. (I don't know if this is a Node.js/V8's bug or a specification.) I'm not sure, but, the callback seems to be called at idle time. Calling await setTimeout(0) in a loop will call the callback and free the heap allocated by RenderedImage class.

--- a/wasm-memory-leak-test.mjs
+++ b/wasm-memory-leak-test.mjs
@@ -1,5 +1,6 @@
 import { readFile } from 'fs/promises'
 import { join } from 'path'
+import { setTimeout } from 'timers/promises'

 import { Resvg, initWasm } from './wasm/index.mjs'

@@ -39,6 +40,7 @@ for (let i = 0; i < 1000000; i++) {
   const resvg = new Resvg(svg, opts)
   const pngData = resvg.render()
   const pngBuffer = pngData.asPng()
+  await setTimeout(0)
   if (i % 10 === 0) {
     // NOTE: Do GC before measuring memory usage because the garbage is not measured.
     global.gc()

Log

$ node --expose-gc wasm-memory-leak-test.mjs
       i,         rss,   heapTotal,    heapUsed,    external,
       0,   123404288,     9895936,     4732256,    26217689,
      10,   166232064,     9895936,     4779880,    55645521,
      20,   182583296,    10158080,     5044056,    62154603,
      30,   185434112,     7012352,     4791968,    63371323,
      40,   184434688,     7012352,     4800976,    63371323,
      50,   185597952,     7012352,     4801072,    69424135,
      60,   185647104,     7012352,     4802312,    69728315,
      70,   185614336,     7012352,     4808896,    69728315,
      80,   185696256,     7012352,     4864728,    69728315,
      90,   185729024,     7012352,     4863800,    69728315,
     100,   200720384,     7012352,     4873792,    69728315,
     110,   201932800,     7012352,     4878392,    69728315,
     120,   204947456,     7012352,     4878760,    69728315,
     130,   204931072,     7012352,     4875680,    69728315,
     140,   204996608,     7012352,     4875680,    69728315,
     150,   204980224,     7012352,     4875680,    69728315,
     160,   204963840,     7012352,     4875680,    69728315,
     170,   205012992,     7012352,     4877560,    69728315,
     180,   208011264,     7012352,     4877560,    69728315,
     190,   207994880,     7012352,     4877560,    69728315,
     200,   208044032,     7012352,     4877560,    69728315,
     210,   208044032,     7012352,     4877560,    69728315,
     220,   208060416,     7012352,     4877560,    69728315,
     230,   208027648,     7012352,     4877560,    69728315,
     240,   208076800,     7012352,     4877560,    69728315,
     250,   211025920,     7012352,     4877416,    75781127,
     260,   211124224,     7012352,     4877560,    76085307,
     270,   211107840,     7012352,     4877560,    76085307,
     280,   211107840,     7012352,     4877560,    76085307,
     290,   211075072,     7012352,     4877560,    76085307,
     300,   211107840,     7012352,     4877560,    76085307,
     310,   214089728,     7012352,     4877560,    76085307,
     320,   216760320,     7012352,     4876968,    80921399,
     330,   216743936,     7012352,     4877864,    82442299,
     340,   216760320,     7012352,     4879032,    82442299,
     350,   216776704,     7012352,     4879032,    82442299,
     360,   216760320,     7012352,     4879032,    82442299,
     370,   216793088,     7012352,     4879032,    82442299,
     380,   219807744,     7012352,     4883816,    82442299,
     390,   219807744,     7012352,     4883816,    82442299,
     400,   219824128,     7012352,     4883816,    82442299,
^C

However, the heap allocated by the Resvg2 (Resvg) class is not free. This is due to rustwasm/wasm-bindgen#3854.

The following patch seems to free the heap.

--- a/wasm/index.mjs
+++ b/wasm/index.mjs
@@ -281,6 +281,7 @@ var Resvg = class {
         throw takeObject(r1);
       }
       this.__wbg_ptr = r0 >>> 0;
+      ResvgFinalization.register(this, this.__wbg_ptr, this);
       return this;
     } finally {
       wasm.__wbindgen_add_to_stack_pointer(16);

Log

$ node --expose-gc wasm-memory-leak-test.mjs
       i,         rss,   heapTotal,    heapUsed,    external,
       0,   131252224,    10158080,     4732504,    26217689,
      10,   169410560,    10158080,     4780640,    55645521,
      20,   183549952,    10158080,     5003136,    57014331,
      30,   185958400,     7012352,     4788232,    63488823,
      40,   186580992,     7012352,     4802240,    65009723,
      50,   186646528,     7012352,     4803008,    65009723,
      60,   186630144,     7012352,     4804248,    65009723,
      70,   186679296,     7012352,     4810832,    65009723,
      80,   186712064,     7012352,     4866448,    65009723,
      90,   186728448,     7012352,     4865736,    65009723,
     100,   189267968,     7012352,     4875728,    65009723,
     110,   190545920,     7012352,     4880328,    65009723,
     120,   190627840,     7012352,     4880696,    65009723,
     130,   193495040,     7012352,     4877616,    65009723,
     140,   193658880,     7012352,     4877616,    65009723,
     150,   193642496,     7012352,     4877616,    65009723,
     160,   193675264,     7012352,     4877616,    65009723,
     170,   193691648,     7012352,     4879496,    65009723,
     180,   194265088,     7012352,     4879496,    65009723,
     190,   194232320,     7012352,     4879496,    65009723,
     200,   194265088,     7012352,     4879496,    65009723,
     210,   194887680,     7012352,     4879496,    65009723,
     220,   195051520,     7012352,     4879496,    65009723,
     230,   195084288,     7012352,     4879496,    65009723,
     240,   194805760,     7012352,     4879496,    65009723,
     250,   194805760,     7012352,     4879496,    65009723,
     260,   194772992,     7012352,     4879496,    65009723,
     270,   194936832,     7012352,     4879496,    65009723,
     280,   195149824,     7012352,     4879496,    65009723,
     290,   195166208,     7012352,     4879496,    65009723,
     300,   195149824,     7012352,     4879496,    65009723,
     310,   195149824,     7012352,     4879496,    65009723,
     320,   195182592,     7012352,     4879496,    65009723,
     330,   195182592,     7012352,     4879496,    65009723,
     340,   195248128,     7012352,     4880968,    65009723,
     350,   195231744,     7012352,     4880968,    65009723,
     360,   195264512,     7012352,     4880968,    65009723,
     370,   195231744,     7012352,     4880968,    65009723,
     380,   195248128,     7012352,     4885752,    65009723,
     390,   195264512,     7012352,     4885752,    65009723,
     400,   195313664,     7012352,     4885752,    65009723,
^C

heapUsed is still increasing, but this is probably caused by the V8 JIT compilation. This is not a memory leak.

• https://developer.chrome.com/docs/devtools/memory-problems/heap-snapshots#compiled-code

[yisibl]

@mizdra Yes, I remembered, there has to sleep in the for loop, otherwise the process doesn't know when to insert GC.

[mizdra]

Hmm. It seems that even without sleep, GC is still being executed. This can be observed by running node --expose-gc --trace-gc wasm-memory-leak-test.mjs. While GC is being performed, the cleanup callback of FinalizationRegistry is not called.

This behavior seems to come from the implementation of V8 (the JavaScript engine used internally by Node.js). When an object registered with FinalizationRegistry is garbage-collected, V8 adds the cleanup callback to the task queue (see: What is a task queue). This means that the cleanup callback is not called immediately, but rather after the currently executing task is completed. For more details, refer to the following V8's test case:

• https://github.com/v8/v8/blob/e7a5d434638a43d6985bedd80fa88f392d8ba02c/test/mjsunit/harmony/weakrefs/cleanup-is-not-a-microtask.js

In other words, on Chrome, Edge, and Node.js using V8, if you use @resvg/resvg-wasm in fully synchronous code, cleanup callback will not be called and will leak memory. In such cases, the free() method must be called manually.

Furthermore, the ECMAScript specification does not define strictly when to call cleanup callback of FinalizationRegistry.

This specification does not make any guarantees that any object or symbol will be garbage collected. Objects or symbols which are not live may be released after long periods of time, or never at all. For this reason, this specification uses the term "may" when describing behavior triggered by garbage collection.
https://tc39.es/ecma262/multipage/executable-code-and-execution-contexts.html#sec-weakref-processing-model

Therefore, with non-V8 JavaScript engines, even code with sleep may cause memory leaks (but this may not be realistic). Therefore, to ensure that memory leaks are prevented, you must call free() method manually.

[mizdra]

IMO: I agree with supporting automatic freeing using FinalizationRegistry. It prevents common user mistakes.

It does not cover some edge cases, but I think that is acceptable. Edge cases can still be avoided by calling .free() method as before. The situation will not be any worse than it is now if automatic freeing is supported.

[yisibl]

IMO: I agree with supporting automatic freeing using FinalizationRegistry. It prevents common user mistakes.

It does not cover some edge cases, but I think that is acceptable. Edge cases can still be avoided by calling .free() method as before. The situation will not be any worse than it is now if automatic freeing is supported.

Yes, I agree that both can be used at the same time so that they are compatible with more environments.

[mizdra]

I reported the behavior of memory leaks in the synchronous code to wasm-bindgen: rustwasm/wasm-bindgen#3917

[martinklepsch]

Hey, I just ran into memory issues as well and tracking down some of the relevant issues I noticed that rustwasm/wasm-bindgen#3854 has been closed/solved.

It was the issue that was chosen in favor of the one opened by @mizdra and linked above (rustwasm/wasm-bindgen#3903)

So maybe this is unblocked?