Skip to content

CanvasPattern doesn't retain source Image/Canvas, breaks under prompt finalizers #2579

@iurisilvio

Description

@iurisilvio

(This issue makes sense only after #2562)

Bug

CanvasPattern (Pattern in C++) stores a cairo_pattern_t* created from a source Image or Canvas:

surface = canvas->ensureSurface();
_pattern = cairo_pattern_create_for_surface(surface);

The pattern internally references the cairo surface, but it does not retain the JS source object. If the source goes out of scope and V8 garbage-collects it, Canvas::destroySurface() calls cairo_surface_finish(_surface) followed by cairo_surface_destroy(_surface). A finished surface is no longer a valid live backing store for pattern rendering, even if cairo still holds a reference to it via the pattern.
Under prompt finalization (e.g. when NAPI_EXPERIMENTAL is enabled and node_api_post_finalizer runs the finalizer at a safe point), this becomes reliably reproducible. Without that path, the bug is masked because the source typically survives long enough by accident.

Reproduction

The existing test Context2d#createPattern(Canvas).setTransform() is sufficient when finalizers run promptly. Minimal version:

'use strict';
if (typeof gc !== 'function') {
  console.error('Run with --expose-gc');
  process.exit(1);
}
const assert = require('node:assert');
const { createCanvas } = require('canvas');
function makeCheckerboard(size, clr) {
  const c = createCanvas(size, size);
  const ctx = c.getContext('2d');
  ctx.fillStyle = `rgb(${clr},${clr},${clr})`;
  ctx.fillRect(0, 0, size, size);
  return c;
}
const target = createCanvas(64, 64);
const ctx = target.getContext('2d');
const size = 32;
const clr = 200;
// createPattern returns a CanvasPattern that internally holds a
// cairo_pattern_t* tied to the temporary canvas' cairo surface.
// We deliberately drop the JS reference to the temporary canvas.
const pat = ctx.createPattern(makeCheckerboard(size, clr), 'repeat');
ctx.fillStyle = pat;
gc(); gc();   // prompt-finalization: temp canvas finalized -> surface destroyed
              // pat now references a finished surface
pat.setTransform(new DOMMatrix().scale(0.5));
ctx.fillRect(0, 0, size * 0.5, size * 0.5);
const imageData = ctx.getImageData(0, 0, 1, 1).data;
console.log('pixel:', [...imageData]);
assert.ok(imageData[0] === clr && imageData[1] === clr && imageData[2] === clr,
          `expected ${clr},${clr},${clr},255 but got ${[...imageData]}`);

With the NAPI_EXPERIMENTAL path enabled, the assertion fails — the pattern reads garbage from the destroyed surface.

Fix

Have Pattern retain the source JS object for as long as the pattern exists:

class Pattern : public Napi::ObjectWrap<Pattern> {
  ...
  private:
    Napi::Reference<Napi::Object> _source;
    cairo_pattern_t *_pattern;
    ...
};
Pattern::Pattern(const Napi::CallbackInfo& info) ... {
  ...
  _pattern = cairo_pattern_create_for_surface(surface);
  _source = Napi::Persistent(obj);   // keep source alive
  ...
}
void Pattern::Finalize(Napi::Env env) {
  _source.Reset();
}

Affects

Correctness bug rather than an RSS leak. Any consumer that uses ctx.createPattern(...) on a temporary Canvas or Image needs this fix to be safe with prompt finalizers.

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