Skip to content

loadImage() doesn't clear image.src on error, delaying cleanup of partial cairo state #2576

@iurisilvio

Description

@iurisilvio

Bug

canvas.loadImage's onerror handler in index.js rejects the Promise but doesn't clear image.src. The Image keeps a reference to the input buffer (and, depending on the format, any partial cairo surface allocated before the decoder errored) until V8 garbage-collects the Image wrapper.
Under sustained load on malformed inputs that fail mid-decode, this delays cleanup arbitrarily.

Cause

// lib/image.js / index.js loadImage helper
image.onerror = (err) => { cleanup(); reject(err) }

reject(err) lets go of the Promise's success path, but image itself is still reachable via the rejection. The image.src setter holds the input buffer in a Napi::Reference. The next assignment to image.src (or GC of the Image) is what calls Image::clearData() to destroy any cairo surface and drop the buffer reference.
For callers that immediately drop the rejected Promise's result, this is invisible. For callers that hold the Image (or any framework that keeps it alive briefly), the partial state lingers.

Fix

Assign image.src = Buffer.alloc(0) before rejecting. This triggers Image::SetSource() which calls clearData() immediately — destroying any partial cairo surface and resetting the buffer reference.

   image.onload = () => { cleanup(); resolve(image) }
-  image.onerror = (err) => { cleanup(); reject(err) }
+  image.onerror = (err) => {
+    cleanup()
+    image.src = Buffer.alloc(0)
+    reject(err)
+  }

Repro

Validated on canvas@3.2.3 from npm. Defensive cleanup — the partial cairo state most affected by this is a separate libjpeg longjmp leak (truncated JPEG that allocates the cairo surface then fails in jpeg_read_scanlines). Even with that addressed, dropping src here removes one source of unintentional retention on rejected loads.

'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] || '2000', 10);
const mb = (n) => +(n / 1024 / 1024).toFixed(1);
function makeTruncatedJpeg() {
  const c = canvas.createCanvas(1024, 1024);
  const ctx = c.getContext('2d');
  ctx.fillStyle = 'red';
  ctx.fillRect(0, 0, 1024, 1024);
  const full = c.toBuffer('image/jpeg', { quality: 0.9 });
  c.width = 0; c.height = 0;
  return full.slice(0, 600); // header without scan data — short-circuits in libjpeg before alloc
}
(async () => {
  const buf = makeTruncatedJpeg();
  gc(); gc();
  const baseline = mb(process.memoryUsage().rss);
  console.log(`baseline rss=${baseline} MiB`);
  for (let i = 1; i <= ITER; i++) {
    try { await canvas.loadImage(buf); } catch (_) {}
    if (i % 500 === 0) {
      gc(); gc();
      console.log(`iter=${i} rss=${mb(process.memoryUsage().rss)} MiB`);
    }
  }
  gc(); gc();
  console.log(`final rss=${mb(process.memoryUsage().rss)} MiB`);
})();

In isolation the slope is small (~2-3 KiB/iter on canvas@3.2.3 Linux x64) because libjpeg fails before allocating the cairo surface. The fix value compounds with a separate libjpeg longjmp leak (filed in its own issue), which leaks the full decoded surface when truncated JPEGs fail inside jpeg_read_scanlines.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions