Skip to content

Retain source Image/Canvas in CanvasPattern so the cairo surface stays valid#2583

Open
iurisilvio wants to merge 1 commit into
Automattic:masterfrom
iurisilvio:fix/canvas-pattern-source-retention
Open

Retain source Image/Canvas in CanvasPattern so the cairo surface stays valid#2583
iurisilvio wants to merge 1 commit into
Automattic:masterfrom
iurisilvio:fix/canvas-pattern-source-retention

Conversation

@iurisilvio
Copy link
Copy Markdown

Fixes #2579.

Pattern (the C++ class behind CanvasPattern) stores a cairo_pattern_t* created from a source Image or Canvas, but it doesn't retain the JS source object. When the source is garbage-collected, Canvas::destroySurface() calls cairo_surface_finish followed by cairo_surface_destroy — a finished surface is no longer a valid live backing store for pattern rendering, even though the cairo pattern still holds a reference to it.

The bug becomes reliably reproducible when finalizers run promptly (e.g. with NAPI_EXPERIMENTAL enabled and node_api_post_finalizer running the finalizer at a safe point); without that path, the source typically survives long enough by accident.

Fix: hold a Napi::Reference<Napi::Object> to the source JS object in Pattern and reset it from Pattern::Finalize(). Repro in #2579.

…s valid

Pattern stores a cairo_pattern_t* created from a source Image or Canvas
via cairo_pattern_create_for_surface(). The pattern references the cairo
surface internally but does not retain the JS source object. If the
source is garbage-collected, 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.

The bug is reliably reproducible when finalizers run promptly (e.g. with
NAPI_EXPERIMENTAL enabled and node_api_post_finalizer in use); without
that path, the source typically survives long enough by accident.

Fix: hold a Napi::Reference to the source JS object in Pattern and reset
it from Pattern::Finalize().

Fixes Automattic#2579
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

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

1 participant