Skip to content

Commit c21e2ea

Browse files
Brooooooklynclaude
andauthored
fix: prevent memory leaks in encode_stream callback and SkPixmap (#1200)
Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
1 parent dd4f96c commit c21e2ea

File tree

2 files changed

+19
-10
lines changed

2 files changed

+19
-10
lines changed

skia-c/skia_c.cpp

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -293,28 +293,28 @@ bool skiac_surface_encode_stream(skiac_surface* c_surface,
293293
int quality,
294294
write_callback_t write_callback,
295295
void* context) {
296-
auto sk_pixmap = new SkPixmap();
297-
if (!SURFACE_CAST->peekPixels(sk_pixmap)) {
296+
SkPixmap sk_pixmap;
297+
if (!SURFACE_CAST->peekPixels(&sk_pixmap)) {
298298
return false;
299299
}
300300
SkJavaScriptWStream stream(write_callback, context);
301301
std::unique_ptr<SkEncoder> encoder;
302302
if (format == int(SkEncodedImageFormat::kJPEG)) {
303303
SkJpegEncoder::Options options;
304304
options.fQuality = quality;
305-
encoder = SkJpegEncoder::Make(&stream, *sk_pixmap, options);
305+
encoder = SkJpegEncoder::Make(&stream, sk_pixmap, options);
306306
} else if (format == int(SkEncodedImageFormat::kPNG)) {
307-
encoder = SkPngEncoder::Make(&stream, *sk_pixmap, SkPngEncoder::Options());
307+
encoder = SkPngEncoder::Make(&stream, sk_pixmap, SkPngEncoder::Options());
308308
} else if (format == int(SkEncodedImageFormat::kWEBP)) {
309309
SkWebpEncoder::Options options;
310310
options.fCompression = quality == 100
311311
? SkWebpEncoder::Compression::kLossless
312312
: SkWebpEncoder::Compression::kLossy;
313313
options.fQuality = quality == 100 ? 75 : quality;
314-
return SkWebpEncoder::Encode(&stream, *sk_pixmap, options);
314+
return SkWebpEncoder::Encode(&stream, sk_pixmap, options);
315315
}
316316
if (encoder) {
317-
return encoder->encodeRows(sk_pixmap->height());
317+
return encoder->encodeRows(sk_pixmap.height());
318318
}
319319
return false;
320320
}

src/lib.rs

Lines changed: 13 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -447,12 +447,17 @@ impl<'c> CanvasElement<'c> {
447447
quality: u8,
448448
callback: F,
449449
) -> bool {
450-
self.ctx.context.surface.encode_stream(
450+
let ptr = Box::into_raw(Box::new(callback));
451+
let result = self.ctx.context.surface.encode_stream(
451452
mime,
452453
quality,
453454
Some(encode_image_stream_callback::<F>),
454-
Box::into_raw(Box::new(callback)).cast(),
455-
)
455+
ptr.cast(),
456+
);
457+
// SAFETY: encode_stream is synchronous, so the callback is no longer referenced.
458+
// Recover the Box to free the allocation.
459+
unsafe { drop(Box::from_raw(ptr)) };
460+
result
456461
}
457462

458463
#[napi]
@@ -1056,6 +1061,10 @@ unsafe extern "C" fn encode_image_stream_callback<F: Fn(&[u8])>(
10561061
size: usize,
10571062
context: *mut c_void,
10581063
) {
1059-
let rust_callback: &mut F = unsafe { Box::leak(Box::from_raw(context.cast())) };
1064+
// SAFETY: context points to a live Box<F> owned by encode_image_inner.
1065+
// Borrow it rather than taking ownership, since this callback may be invoked
1066+
// multiple times during encoding. The Box is freed by encode_image_inner after
1067+
// encode_stream returns.
1068+
let rust_callback: &F = unsafe { &*context.cast() };
10601069
rust_callback(unsafe { slice::from_raw_parts(data.cast(), size) });
10611070
}

0 commit comments

Comments
 (0)