From a2ff8aae8a1067c233b87e60001986df75c5953f 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` holds four `cairo_pattern_t*` members (fillPattern, strokePattern, fillGradient, strokeGradient) but the destructor only freed fontDescription, and the copy ctor copied the raw pointers — so save()/restore() chains shared the same `cairo_pattern_t*` with no shared ownership, and every state destroyed with a non-null pattern leaked it. Take proper refcounted ownership: - copy ctor and a new operator= `cairo_pattern_reference` each pattern; - ~canvas_state_t `cairo_pattern_destroy`s all four; - set*/clear* helpers handle replacement at the call sites (SetFillStyle, SetStrokeStyle, _setFillColor, _setStrokeColor). State now holds its own reference to the cairo pattern, so a gradient/pattern stays valid for the lifetime of the state regardless of the JS wrapper's GC. Scoped to the leak fix only; the fillStyle/strokeStyle strong-ref and Context2d state-lifetime changes are split into a separate PR. Fixes #2578. Co-Authored-By: Claude Opus 4.8 --- src/CanvasRenderingContext2d.cc | 13 +++-- src/CanvasRenderingContext2d.h | 92 +++++++++++++++++++++++++++++++-- 2 files changed, 94 insertions(+), 11 deletions(-) diff --git a/src/CanvasRenderingContext2d.cc b/src/CanvasRenderingContext2d.cc index 9c5482074..7eff5c338 100644 --- a/src/CanvasRenderingContext2d.cc +++ b/src/CanvasRenderingContext2d.cc @@ -90,7 +90,6 @@ inline static bool checkArgs(const Napi::CallbackInfo&info, double *args, int ar void Context2d::Initialize(Napi::Env& env, Napi::Object& exports) { - Napi::HandleScope scope(env); InstanceData* data = env.GetInstanceData(); Napi::Function ctor = DefineClass(env, "CanvasRenderingContext2D", { @@ -1968,11 +1967,11 @@ Context2d::SetFillStyle(const Napi::CallbackInfo& info, const Napi::Value& value if (obj.InstanceOf(data->CanvasGradientCtor.Value()).UnwrapOr(false)) { _fillStyle.Reset(obj); 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); Pattern *pattern = Pattern::Unwrap(obj); - state->fillPattern = pattern->pattern(); + state->setFillPattern(pattern->pattern()); } } } @@ -2008,11 +2007,11 @@ Context2d::SetStrokeStyle(const Napi::CallbackInfo& info, const Napi::Value& val if (obj.InstanceOf(data->CanvasGradientCtor.Value()).UnwrapOr(false)) { _strokeStyle.Reset(obj); 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); Pattern *pattern = Pattern::Unwrap(obj); - state->strokePattern = pattern->pattern(); + state->setStrokePattern(pattern->pattern()); } } } @@ -2193,7 +2192,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 +2218,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); + } }; /*