From ef08a1e7cb9b49aaa8a80ccf3258beeb215c954b Mon Sep 17 00:00:00 2001 From: Iuri de Silvio Date: Mon, 18 May 2026 09:33:16 +0200 Subject: [PATCH] Take ownership of cairo_pattern_t in canvas_state_t with refcounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit canvas_state_t held four cairo_pattern_t* members (fillPattern, strokePattern, fillGradient, strokeGradient) but the destructor only freed fontDescription. Every Context2d that ended life with a non-null pattern leaked the pattern. The copy constructor copied the raw pointers, so save()/restore() shared the same pattern across states without refcounting — making any per-state destroy unsafe. Switch to proper cairo refcount discipline: - canvas_state_t copy ctor + new operator= reference each pattern. - ~canvas_state_t releases all four patterns. - New setFillPattern / setStrokePattern / setFillGradient / setStrokeGradient helpers replace the previous pattern (refcount--) before storing the new one (refcount++). clearFillPattern / clearStrokePattern release both pattern and gradient when switching to a solid color. Also fix correctness issues that become visible once finalizers run promptly (NAPI_EXPERIMENTAL path): - _fillStyle and _strokeStyle now use Reset(obj, 1) (strong ref) so the JS gradient/pattern object stays alive while the Context2d's state still references its native cairo_pattern_t. - ~Context2d pops all states before destroying _layout and _context so canvas_state_t destructors run with a valid context. The destructor intentionally does NOT call _resetPersistentHandles() itself — that invokes napi_delete_reference which is unsafe inside GC. The handles are reset from Finalize(env), which runs deferred via node_api_post_finalizer. - resetState clears all states (not just pop()) and re-points 'state' at the freshly emplaced top; the original pop()+emplace() left 'state' dangling when states was empty. Fixes #2578 --- src/CanvasRenderingContext2d.cc | 31 ++++++----- src/CanvasRenderingContext2d.h | 92 +++++++++++++++++++++++++++++++-- 2 files changed, 107 insertions(+), 16 deletions(-) diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc index 9c5482074..fc1a7dedb 100644 --- a/src/CanvasRenderingContext2d.cc +++ b/src/CanvasRenderingContext2d.cc @@ -242,9 +242,15 @@ Context2d::Context2d(const Napi::CallbackInfo& info) : Napi::ObjectWrapfontDescription); _resetPersistentHandles(); } @@ -1966,13 +1973,13 @@ Context2d::SetFillStyle(const Napi::CallbackInfo& info, const Napi::Value& value InstanceData *data = env.GetInstanceData(); Napi::Object obj = value.As(); if (obj.InstanceOf(data->CanvasGradientCtor.Value()).UnwrapOr(false)) { - _fillStyle.Reset(obj); + _fillStyle.Reset(obj, 1); Gradient *grad = Gradient::Unwrap(obj); - state->fillGradient = grad->pattern(); + state->setFillGradient(grad->pattern()); } else if (obj.InstanceOf(data->CanvasPatternCtor.Value()).UnwrapOr(false)) { - _fillStyle.Reset(obj); + _fillStyle.Reset(obj, 1); Pattern *pattern = Pattern::Unwrap(obj); - state->fillPattern = pattern->pattern(); + state->setFillPattern(pattern->pattern()); } } } @@ -2006,13 +2013,13 @@ Context2d::SetStrokeStyle(const Napi::CallbackInfo& info, const Napi::Value& val InstanceData *data = env.GetInstanceData(); Napi::Object obj = value.As(); if (obj.InstanceOf(data->CanvasGradientCtor.Value()).UnwrapOr(false)) { - _strokeStyle.Reset(obj); + _strokeStyle.Reset(obj, 1); Gradient *grad = Gradient::Unwrap(obj); - state->strokeGradient = grad->pattern(); + state->setStrokeGradient(grad->pattern()); } else if (obj.InstanceOf(data->CanvasPatternCtor.Value()).UnwrapOr(false)) { - _strokeStyle.Reset(value); + _strokeStyle.Reset(value, 1); Pattern *pattern = Pattern::Unwrap(obj); - state->strokePattern = pattern->pattern(); + state->setStrokePattern(pattern->pattern()); } } } @@ -2193,7 +2200,7 @@ Context2d::_setFillColor(Napi::Value arg) { if (status != napi_ok) return; uint32_t rgba = rgba_from_string(buf, &ok); if (!ok) return; - state->fillPattern = state->fillGradient = NULL; + state->clearFillPattern(); state->fill = rgba_create(rgba); } } @@ -2219,7 +2226,7 @@ Context2d::_setStrokeColor(Napi::Value arg) { std::string str = arg.As(); uint32_t rgba = rgba_from_string(str.c_str(), &ok); if (!ok) return; - state->strokePattern = state->strokeGradient = NULL; + state->clearStrokePattern(); state->stroke = rgba_create(rgba); } diff --git a/src/CanvasRenderingContext2d.h b/src/CanvasRenderingContext2d.h index eee266036..cb2c4b3c3 100644 --- a/src/CanvasRenderingContext2d.h +++ b/src/CanvasRenderingContext2d.h @@ -47,10 +47,10 @@ struct canvas_state_t { fill = other.fill; stroke = other.stroke; patternQuality = other.patternQuality; - fillPattern = other.fillPattern; - strokePattern = other.strokePattern; - fillGradient = other.fillGradient; - strokeGradient = other.strokeGradient; + fillPattern = referencePattern(other.fillPattern); + strokePattern = referencePattern(other.strokePattern); + fillGradient = referencePattern(other.fillGradient); + strokeGradient = referencePattern(other.strokeGradient); globalAlpha = other.globalAlpha; textAlignment = other.textAlignment; textBaseline = other.textBaseline; @@ -66,9 +66,93 @@ struct canvas_state_t { lang = other.lang; } + canvas_state_t& operator=(const canvas_state_t& other) { + if (this == &other) return *this; + + fill = other.fill; + stroke = other.stroke; + patternQuality = other.patternQuality; + releasePattern(fillPattern); + releasePattern(strokePattern); + releasePattern(fillGradient); + releasePattern(strokeGradient); + fillPattern = referencePattern(other.fillPattern); + strokePattern = referencePattern(other.strokePattern); + fillGradient = referencePattern(other.fillGradient); + strokeGradient = referencePattern(other.strokeGradient); + globalAlpha = other.globalAlpha; + textAlignment = other.textAlignment; + textBaseline = other.textBaseline; + shadow = other.shadow; + shadowBlur = other.shadowBlur; + shadowOffsetX = other.shadowOffsetX; + shadowOffsetY = other.shadowOffsetY; + textDrawingMode = other.textDrawingMode; + pango_font_description_free(fontDescription); + fontDescription = pango_font_description_copy(other.fontDescription); + font = other.font; + imageSmoothingEnabled = other.imageSmoothingEnabled; + direction = other.direction; + lang = other.lang; + + return *this; + } + ~canvas_state_t() { + releasePattern(fillPattern); + releasePattern(strokePattern); + releasePattern(fillGradient); + releasePattern(strokeGradient); pango_font_description_free(fontDescription); } + + void setFillPattern(cairo_pattern_t* pattern) { + releasePattern(fillGradient); + replacePattern(fillPattern, pattern); + } + + void setStrokePattern(cairo_pattern_t* pattern) { + releasePattern(strokeGradient); + replacePattern(strokePattern, pattern); + } + + void setFillGradient(cairo_pattern_t* pattern) { + releasePattern(fillPattern); + replacePattern(fillGradient, pattern); + } + + void setStrokeGradient(cairo_pattern_t* pattern) { + releasePattern(strokePattern); + replacePattern(strokeGradient, pattern); + } + + void clearFillPattern() { + releasePattern(fillPattern); + releasePattern(fillGradient); + } + + void clearStrokePattern() { + releasePattern(strokePattern); + releasePattern(strokeGradient); + } + + static cairo_pattern_t* referencePattern(cairo_pattern_t* pattern) { + if (pattern) cairo_pattern_reference(pattern); + return pattern; + } + + static void releasePattern(cairo_pattern_t*& pattern) { + if (pattern) { + cairo_pattern_destroy(pattern); + pattern = nullptr; + } + } + + static void replacePattern(cairo_pattern_t*& current, cairo_pattern_t* next) { + if (current == next) return; + releasePattern(current); + current = referencePattern(next); + } }; /*