loadSVGFromBuffer leaks RsvgHandle when SVG has no intrinsic size

[iurisilvio]

Bug

Image::loadSVGFromBuffer() calls rsvg_handle_new_from_data() to build _rsvg, then calls rsvg_handle_get_intrinsic_size_in_pixels() to read d_width / d_height. If the SVG parses as valid XML but has no intrinsic size (no width/height attribute on <svg> and no element that implies one), rsvg_handle_get_intrinsic_size_in_pixels() returns FALSE and leaves the out-params untouched. The current code:

double d_width;
double d_height;

rsvg_handle_get_intrinsic_size_in_pixels(_rsvg, &d_width, &d_height);

width  = naturalWidth  = d_width;
height = naturalHeight = d_height;

if (width <= 0 || height <= 0) {
  this->errorInfo.set("Width and height must be set on the svg element");
  return CAIRO_STATUS_READ_ERROR;
}

• d_width / d_height are read uninitialized when rsvg_handle_get_intrinsic_size_in_pixels returns FALSE.
• The error return leaves _rsvg attached to the Image. The RsvgHandle (and the parsed SVG DOM it owns) is only released when the JS Image wrapper is eventually finalized.

A related path in Image::renderSVGToSurface() has the same shape: some error branches g_object_unref(_rsvg) but don't clear _rsvg, and don't destroy _surface / the cairo context that have already been created. Image::surface() then runs a redundant g_object_unref(_rsvg) if renderSVGToSurface already unreffed it on its error path — which is what masks the leak from showing as a crash in callers that retry.

Reproduction

'use strict';
if (typeof gc !== 'function') {
  console.error('Run with --expose-gc');
  process.exit(1);
}
const canvas = require('canvas');
const ITER = parseInt(process.argv[2] || '10000', 10);
const mb = (n) => +(n / 1024 / 1024).toFixed(1);

// Valid XML, parses cleanly. No width/height, no element that implies a size.
const svg = Buffer.from(
  '<svg xmlns="http://www.w3.org/2000/svg"><path d="M1,1"/></svg>'
);

(async () => {
  gc(); gc();
  const baseline = mb(process.memoryUsage().rss);
  let last = baseline;
  console.log(`baseline rss=${baseline} MiB`);
  for (let i = 1; i <= ITER; i++) {
    try { await canvas.loadImage(svg); } catch (_) { /* expected */ }
    if (i % 2000 === 0) {
      gc(); gc();
      const rss = mb(process.memoryUsage().rss);
      console.log(`iter=${i} rss=${rss}M slope=${(((rss - last) * 1024) / 2000).toFixed(3)} KiB/iter`);
      last = rss;
    }
  }
  gc(); gc();
  const end = mb(process.memoryUsage().rss);
  console.log(`baseline=${baseline}M end=${end}M delta=${(end - baseline).toFixed(1)}M per-iter=${(((end - baseline) * 1024) / ITER).toFixed(3)} KiB`);
})();

Observed on canvas@3.2.3 from npm (Linux x86_64 in node:20-bookworm)

baseline rss=45.9 MiB
iter=2000  rss=65.7M  slope=10.138 KiB/iter
iter=4000  rss=73.4M  slope=3.942  KiB/iter
iter=6000  rss=80.7M  slope=3.738  KiB/iter
iter=8000  rss=87.7M  slope=3.584  KiB/iter
iter=10000 rss=94.7M  slope=3.584  KiB/iter
baseline=45.9M end=94.7M delta=48.8M per-iter=4.997 KiB

Steady ~4 KiB/iter slope — one orphaned RsvgHandle (with its parsed DOM) per failed loadImage. The slope is also visible without --expose-gc, just with more noise from delayed finalization.

Cause

1. Image::loadSVGFromBuffer() doesn't check the return value of rsvg_handle_get_intrinsic_size_in_pixels() and uses uninitialized d_width / d_height when it returns FALSE.
2. On the missing-dimensions error return, _rsvg is not released and _is_svg is left set, so the orphaned RsvgHandle lives until the Image wrapper is collected.
3. Image::renderSVGToSurface() has the inverse problem on its own error branches: _rsvg is unreffed but not nulled, and _surface / the cairo context aren't torn down, so a later cleanup pass (or a redundant g_object_unref in the surface() caller) can either leak or double-unref depending on which branch was hit.

Fix

In loadSVGFromBuffer:

• initialize d_width = 0 / d_height = 0
• use the return value of rsvg_handle_get_intrinsic_size_in_pixels() to decide whether to trust the values
• on the missing-dimensions error path: g_object_unref(_rsvg), set _rsvg = NULL, and clear _is_svg before returning the error

In renderSVGToSurface:

• on each error branch, destroy the cairo context if created, destroy _surface and null it, unref and null _rsvg, and clear _is_svg
• callers (Image::surface()) should not unref _rsvg again after renderSVGToSurface() has already cleaned up on its error path

After applying both fixes, the same reproduction runs flat (slope is in the same range as N-API finalizer / cairo allocator noise).