Context2d: keep fillStyle/strokeStyle strong, fix resetState and ~Context2d teardown#2598
Open
iurisilvio wants to merge 1 commit into
Open
Context2d: keep fillStyle/strokeStyle strong, fix resetState and ~Context2d teardown#2598iurisilvio wants to merge 1 commit into
iurisilvio wants to merge 1 commit into
Conversation
…text2d teardown Three Context2d state-handling correctness fixes, independent of the canvas_state_t cairo_pattern_t ownership change in Automattic#2582: - Hold a strong reference to the fillStyle/strokeStyle JS object (_fillStyle/_strokeStyle.Reset(obj, 1)). For an image-backed CanvasPattern the cairo pattern's surface points at the Image's pixel buffer, which Image::clearData() frees when the Image is GC'd; keeping the wrapper alive while the state still references its pattern prevents a use-after-free (matches the CanvasPattern source retention in Automattic#2583). - resetState(): pop all states and re-point state = &states.top() after emplace(); the original pop()+emplace() left `state` dangling and only cleared one level. - ~Context2d(): pop states before destroying the cairo context. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context2d state-handling correctness (split out of #2582)
Three small, independent fixes to
Context2dstate handling. Split from #2582 — which now contains only thecairo_pattern_trefcounting — this PR is the correctness half and does not depend on it.1. Keep
fillStyle/strokeStyleas strong references_fillStyle/_strokeStyleare weak references (.Reset(obj), refcount 0). For an image-backedCanvasPattern, the cairo pattern's surface points at theImage's pixel buffer (cairo_image_surface_create_for_data), whichImage::clearData()frees unconditionally when theImageis GC'd. If the wrapper is collected while the Context2d state still references that pattern, drawing reads freed memory..Reset(obj, 1)keeps the wrapper alive while it is the active style. (Pairs with the source retention in #2583, which keeps theImageitself alive for the pattern's lifetime.)2.
resetState()leavesstatedanglingFix: pop all states, emplace one, and re-point
state = &states.top().3.
~Context2d()teardown orderPop states before destroying the cairo context, so each
canvas_state_tis torn down while the context is still valid.Note: this overlaps with #2582 in
SetFillStyle/SetStrokeStyle(both touch those lines). Whichever merges first, the other rebases trivially.