Skip to content

Commit 60a34e8

Browse files
Brooooooklynclaude
andauthored
fix: plug native memory leaks in SVG canvas, bitmap shader, and stream allocations (#1201)
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent cf8f313 commit 60a34e8

File tree

5 files changed

+355
-17
lines changed

5 files changed

+355
-17
lines changed

.github/workflows/CI.yaml

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -341,6 +341,8 @@ jobs:
341341

342342
- name: Test bindings
343343
run: yarn test:ci
344+
env:
345+
RUN_STRESS_TESTS: 'true'
344346

345347
- name: Test failed
346348
if: ${{ failure() }}
Lines changed: 316 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,316 @@
1+
import { join, dirname } from 'node:path'
2+
import { fileURLToPath } from 'node:url'
3+
import { readFileSync, unlinkSync, existsSync } from 'node:fs'
4+
import { tmpdir } from 'node:os'
5+
6+
import { default as ava } from 'ava'
7+
8+
import {
9+
createCanvas,
10+
Path2D,
11+
Image,
12+
GlobalFonts,
13+
SvgExportFlag,
14+
SvgCanvas,
15+
convertSVGTextToPath,
16+
} from '../index'
17+
18+
const shouldSkip = process.env.RUN_STRESS_TESTS !== 'true'
19+
20+
const test = shouldSkip ? ava.skip : ava
21+
22+
const __dirname = dirname(fileURLToPath(import.meta.url))
23+
const ITERATIONS = 500
24+
25+
// ============================================================================
26+
// Fix #1: Canvas-pattern shader bitmap leak (stack-allocated SkBitmap)
27+
// Exercises: skiac_bitmap_get_shader with is_canvas=true
28+
// Risk: dangling pointer if SkBitmap scope is wrong
29+
// ============================================================================
30+
test('stress: createPattern with canvas source should not crash (bitmap shader fix)', (t) => {
31+
const canvas = createCanvas(256, 256)
32+
const ctx = canvas.getContext('2d')!
33+
const patternCanvas = createCanvas(32, 32)
34+
const patternCtx = patternCanvas.getContext('2d')!
35+
36+
for (let i = 0; i < ITERATIONS; i++) {
37+
patternCtx.fillStyle = `rgb(${i % 256}, ${(i * 7) % 256}, ${(i * 13) % 256})`
38+
patternCtx.fillRect(0, 0, 32, 32)
39+
40+
const pattern = ctx.createPattern(patternCanvas, 'repeat')
41+
ctx.fillStyle = pattern
42+
ctx.fillRect(0, 0, 256, 256)
43+
}
44+
45+
t.pass(`Completed ${ITERATIONS} canvas-pattern shader cycles without crash`)
46+
})
47+
48+
// ============================================================================
49+
// Fix #2: SVG bitmap decode stream leak (stack-allocated SkMemoryStream)
50+
// Exercises: skiac_bitmap_make_from_svg
51+
// Risk: use-after-free if stack stream is used after destruction
52+
// ============================================================================
53+
test('stress: SVG image decode should not crash (SVG decode stream fix)', async (t) => {
54+
const svgData = Buffer.from(
55+
'<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100">' +
56+
'<rect width="100" height="100" fill="red"/>' +
57+
'<circle cx="50" cy="50" r="40" fill="blue"/>' +
58+
'</svg>',
59+
)
60+
61+
for (let i = 0; i < ITERATIONS; i++) {
62+
const image = new Image()
63+
image.src = svgData
64+
await image.decode()
65+
// Draw it to exercise the full pipeline
66+
const canvas = createCanvas(100, 100)
67+
const ctx = canvas.getContext('2d')!
68+
ctx.drawImage(image, 0, 0)
69+
}
70+
71+
t.pass(`Completed ${ITERATIONS} SVG image decode cycles without crash`)
72+
})
73+
74+
// ============================================================================
75+
// Fix #3: convertSVGTextToPath stream leaks (stack-allocated streams)
76+
// Exercises: skiac_svg_text_to_path
77+
// Risk: use-after-free if stack streams are used after destruction
78+
// ============================================================================
79+
test('stress: convertSVGTextToPath should not crash (text-to-path stream fix)', (t) => {
80+
GlobalFonts.registerFromPath(join(__dirname, 'fonts', 'iosevka-slab-regular.ttf'))
81+
const svgFixture = readFileSync(join(__dirname, 'text.svg'), 'utf8')
82+
83+
for (let i = 0; i < ITERATIONS; i++) {
84+
const result = convertSVGTextToPath(svgFixture)
85+
t.truthy(result.length > 0)
86+
}
87+
88+
t.pass(`Completed ${ITERATIONS} convertSVGTextToPath cycles without crash`)
89+
})
90+
91+
// ============================================================================
92+
// Fix #4: SVGCanvas lifecycle canvas leak (is_svg + Drop)
93+
// Exercises: SVGCanvas creation, getContent, resize, and drop
94+
// Risk: double-free if canvas pointer not nulled after manual destroy
95+
// ============================================================================
96+
test('stress: SVGCanvas getContent + drop should not double-free', (t) => {
97+
for (let i = 0; i < ITERATIONS; i++) {
98+
const canvas: SvgCanvas = createCanvas(200, 200, SvgExportFlag.ConvertTextToPaths)
99+
const ctx = canvas.getContext('2d')
100+
ctx.fillStyle = 'red'
101+
ctx.fillRect(0, 0, 200, 200)
102+
103+
// getContent destroys the canvas, creates a new one, then object is dropped
104+
// This tests the null-out-after-destroy + Drop interaction
105+
const content = canvas.getContent()
106+
t.truthy(content.length > 0)
107+
}
108+
// If we get here without SIGSEGV/SIGABRT, no double-free
109+
t.pass(`Completed ${ITERATIONS} SVGCanvas getContent+drop cycles without crash`)
110+
})
111+
112+
test('stress: SVGCanvas resize should not leak or double-free', (t) => {
113+
for (let i = 0; i < ITERATIONS; i++) {
114+
const canvas: SvgCanvas = createCanvas(200, 200, SvgExportFlag.ConvertTextToPaths)
115+
const ctx = canvas.getContext('2d')
116+
ctx.fillRect(0, 0, 200, 200)
117+
118+
// Resize triggers mem::replace which drops old Context
119+
// The old SVG canvas must be destroyed in Drop without double-free
120+
canvas.width = 100 + (i % 100)
121+
canvas.height = 100 + (i % 100)
122+
123+
ctx.fillRect(0, 0, canvas.width, canvas.height)
124+
const content = canvas.getContent()
125+
t.truthy(content.length > 0)
126+
}
127+
128+
t.pass(`Completed ${ITERATIONS} SVGCanvas resize cycles without crash`)
129+
})
130+
131+
test('stress: SVGCanvas drop without getContent should not leak or crash', (t) => {
132+
for (let i = 0; i < ITERATIONS; i++) {
133+
// Create, draw, then let it be GC'd WITHOUT calling getContent
134+
// This exercises the Drop path where canvas pointer is non-null
135+
const canvas: SvgCanvas = createCanvas(128, 128, SvgExportFlag.ConvertTextToPaths)
136+
const ctx = canvas.getContext('2d')
137+
ctx.fillStyle = `hsl(${i % 360}, 80%, 50%)`
138+
ctx.fillRect(0, 0, 128, 128)
139+
// canvas goes out of scope — Drop should clean up SVG canvas
140+
}
141+
142+
t.pass(`Completed ${ITERATIONS} SVGCanvas drop-without-getContent cycles without crash`)
143+
})
144+
145+
// ============================================================================
146+
// Fix #5: SVG surface creation error-path leak (delete w_stream on error)
147+
// Hard to trigger since SkSVGCanvas::Make rarely fails with valid dimensions.
148+
// We test the success path heavily to ensure the normal path is unaffected.
149+
// ============================================================================
150+
test('stress: SVGCanvas creation at various sizes should not crash', (t) => {
151+
for (let i = 0; i < ITERATIONS; i++) {
152+
const w = 1 + (i % 500)
153+
const h = 1 + ((i * 7) % 500)
154+
const canvas: SvgCanvas = createCanvas(w, h, SvgExportFlag.ConvertTextToPaths)
155+
const ctx = canvas.getContext('2d')
156+
ctx.fillRect(0, 0, w, h)
157+
const content = canvas.getContent()
158+
t.truthy(content.length > 0)
159+
}
160+
161+
t.pass(`Completed ${ITERATIONS} SVGCanvas creation cycles without crash`)
162+
})
163+
164+
// ============================================================================
165+
// Fix #6: Path2D CString leak (CString::from_raw after FFI)
166+
// Exercises: Path::from_svg_path
167+
// Risk: use-after-free if CString is reclaimed before C++ reads it
168+
// ============================================================================
169+
test('stress: Path2D from SVG string should not crash (CString fix)', (t) => {
170+
const paths = [
171+
'M 10 80 C 40 10, 65 10, 95 80 S 150 150, 180 80',
172+
'M 0 0 L 100 0 L 100 100 L 0 100 Z',
173+
'M108.956,403.826c0,0,0.178,3.344-1.276,3.311c-1.455-0.033-30.507-84.917-66.752-80.957',
174+
'M 10 10 H 90 V 90 H 10 Z',
175+
'M 50 0 A 50 50 0 1 0 50 100 A 50 50 0 1 0 50 0',
176+
]
177+
178+
for (let i = 0; i < ITERATIONS; i++) {
179+
const svgPath = paths[i % paths.length]
180+
const p = new Path2D(svgPath)
181+
182+
// Also use the path to ensure it's valid
183+
const canvas = createCanvas(200, 200)
184+
const ctx = canvas.getContext('2d')!
185+
ctx.fill(p)
186+
}
187+
188+
t.pass(`Completed ${ITERATIONS} Path2D SVG string cycles without crash`)
189+
})
190+
191+
// ============================================================================
192+
// Fix #7: save_png SkImage leak (image.get() instead of image.release())
193+
// Exercises: skiac_surface_save
194+
// Risk: if sk_sp is prematurely destroyed, image becomes invalid during encode
195+
// ============================================================================
196+
test('stress: savePng should not crash (SkImage lifetime fix)', (t) => {
197+
const tmpPath = join(tmpdir(), `canvas-stress-test-${Date.now()}.png`)
198+
199+
try {
200+
const canvas = createCanvas(128, 128)
201+
const ctx = canvas.getContext('2d')!
202+
203+
for (let i = 0; i < ITERATIONS; i++) {
204+
ctx.fillStyle = `rgb(${i % 256}, ${(i * 3) % 256}, ${(i * 7) % 256})`
205+
ctx.fillRect(0, 0, 128, 128)
206+
ctx.strokeStyle = 'white'
207+
ctx.strokeRect(10, 10, 108, 108)
208+
209+
// This calls skiac_surface_save which we fixed
210+
;(canvas as any).savePng(tmpPath)
211+
}
212+
213+
t.pass(`Completed ${ITERATIONS} savePng cycles without crash`)
214+
} finally {
215+
if (existsSync(tmpPath)) {
216+
unlinkSync(tmpPath)
217+
}
218+
}
219+
})
220+
221+
// Also test the encode path (skiac_surface_png_data / skiac_surface_encode_data)
222+
// to ensure those existing correct paths still work
223+
test('stress: encodeSync png/jpeg/webp should not crash', (t) => {
224+
const canvas = createCanvas(128, 128)
225+
const ctx = canvas.getContext('2d')!
226+
227+
for (let i = 0; i < ITERATIONS; i++) {
228+
ctx.fillStyle = `hsl(${i % 360}, 70%, 50%)`
229+
ctx.fillRect(0, 0, 128, 128)
230+
231+
const png = canvas.encodeSync('png')
232+
t.truthy(png.length > 0)
233+
234+
const jpeg = canvas.encodeSync('jpeg', 80)
235+
t.truthy(jpeg.length > 0)
236+
237+
const webp = canvas.encodeSync('webp', 80)
238+
t.truthy(webp.length > 0)
239+
}
240+
241+
t.pass(`Completed ${ITERATIONS} encodeSync cycles without crash`)
242+
})
243+
244+
// ============================================================================
245+
// Fix #8: EXIF canvas leak (delete canvas after drawImage)
246+
// Exercises: skiac_bitmap_make_from_buffer with EXIF orientation
247+
// Risk: if canvas is deleted too early, drawImage writes to freed memory
248+
// ============================================================================
249+
test('stress: EXIF-rotated JPEG decode should not crash (EXIF canvas fix)', async (t) => {
250+
const exifJpeg = readFileSync(join(__dirname, 'fixtures', 'with-exif.jpg'))
251+
252+
for (let i = 0; i < ITERATIONS; i++) {
253+
const image = new Image()
254+
image.src = exifJpeg
255+
await image.decode()
256+
257+
// Draw the decoded image to verify it's valid
258+
const canvas = createCanvas(image.width, image.height)
259+
const ctx = canvas.getContext('2d')!
260+
ctx.drawImage(image, 0, 0)
261+
}
262+
263+
t.pass(`Completed ${ITERATIONS} EXIF JPEG decode cycles without crash`)
264+
})
265+
266+
// ============================================================================
267+
// Combined stress: exercise multiple fixed paths in interleaved sequence
268+
// This is the most likely to trigger double-free if ownership is confused
269+
// ============================================================================
270+
test('stress: combined interleaved operations should not crash', async (t) => {
271+
const exifJpeg = readFileSync(join(__dirname, 'fixtures', 'with-exif.jpg'))
272+
const svgData = Buffer.from(
273+
'<svg xmlns="http://www.w3.org/2000/svg" width="64" height="64">' +
274+
'<circle cx="32" cy="32" r="30" fill="green"/></svg>',
275+
)
276+
277+
for (let i = 0; i < 200; i++) {
278+
// Path2D from string
279+
const path = new Path2D('M 0 0 L 50 50 L 100 0 Z')
280+
281+
// Canvas pattern from canvas
282+
const srcCanvas = createCanvas(64, 64)
283+
const srcCtx = srcCanvas.getContext('2d')!
284+
srcCtx.fillStyle = 'blue'
285+
srcCtx.fill(path)
286+
287+
const dstCanvas = createCanvas(256, 256)
288+
const dstCtx = dstCanvas.getContext('2d')!
289+
const pattern = dstCtx.createPattern(srcCanvas, 'repeat')
290+
dstCtx.fillStyle = pattern
291+
dstCtx.fillRect(0, 0, 256, 256)
292+
293+
// Encode
294+
dstCanvas.encodeSync('png')
295+
296+
// SVG canvas
297+
const svgCanvas: SvgCanvas = createCanvas(128, 128, SvgExportFlag.ConvertTextToPaths)
298+
const svgCtx = svgCanvas.getContext('2d')
299+
svgCtx.fillRect(0, 0, 128, 128)
300+
svgCanvas.width = 64
301+
svgCanvas.getContent()
302+
303+
// SVG image decode
304+
const svgImg = new Image()
305+
svgImg.src = svgData
306+
await svgImg.decode()
307+
308+
// EXIF decode
309+
const exifImg = new Image()
310+
exifImg.src = exifJpeg
311+
await exifImg.decode()
312+
dstCtx.drawImage(exifImg, 0, 0, 256, 256)
313+
}
314+
315+
t.pass('Completed 200 combined interleaved cycles without crash')
316+
})

0 commit comments

Comments
 (0)