Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Revised fix of memory leak #318 #365

Merged
merged 7 commits into from

4 participants

@kkoopa

Based on #318 by @jasonrose, I have corrected some things that were causing me problems.

@kangax
Collaborator

@rvagg @TooTallNate looks good?

@rvagg
Collaborator

I guess, the only thing I don't quite follow is the jpeg_free_custom_allocations but I'm guessing it properly deals with the unmatched malloc for these buffers. If it works then :+1: from me!

@jasonrose

I believe that the buffer wasn't being cleaned up when the object was being destroyed, causing a leak. I destroy it in that function.

@kangax kangax merged commit 285dc4f into Automattic:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
Showing with 34 additions and 9 deletions.
  1. +20 −8 src/Image.cc
  2. +14 −1 src/JPEGStream.h
View
28 src/Image.cc
@@ -207,7 +207,7 @@ Image::loadFromBuffer(uint8_t *buf, unsigned len) {
#endif
#ifdef HAVE_JPEG
#if CAIRO_VERSION_MINOR < 10
- if (isJPEG(buf)) return loadJPEGFromBuffer(buf, len);
+ if (isJPEG(buf)) return loadJPEGFromBuffer(buf, len);
#else
if (isJPEG(buf)) {
if (DATA_IMAGE == data_mode) return loadJPEGFromBuffer(buf, len);
@@ -314,6 +314,16 @@ Image::Image() {
Image::~Image() {
clearData();
+
+ if (onerror) {
+ delete onerror;
+ onerror = NULL;
+ }
+
+ if (onload) {
+ delete onload;
+ onload = NULL;
+ }
}
/*
@@ -346,6 +356,7 @@ Image::loaded() {
if (onload != NULL) {
onload->Call(0, NULL);
delete onload;
+ onload = NULL;
}
}
@@ -360,12 +371,13 @@ Image::error(Local<Value> err) {
Local<Value> argv[1] = { err };
onerror->Call(1, argv);
delete onerror;
+ onerror = NULL;
}
}
/*
* Load cairo surface from the image src.
- *
+ *
* TODO: support more formats
* TODO: use node IO or at least thread pool
*/
@@ -457,7 +469,7 @@ Image::loadGIF(FILE *stream) {
if (fstat(fd, &s) < 0) {
fclose(stream);
return CAIRO_STATUS_READ_ERROR;
- }
+ }
uint8_t *buf = (uint8_t *) malloc(s.st_size);
@@ -490,10 +502,10 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) {
#if GIFLIB_MAJOR >= 5
int errorcode;
if ((gif = DGifOpen((void*) &gifd, read_gif_from_memory, &errorcode)) == NULL)
- return CAIRO_STATUS_READ_ERROR;
+ return CAIRO_STATUS_READ_ERROR;
#else
if ((gif = DGifOpen((void*) &gifd, read_gif_from_memory)) == NULL)
- return CAIRO_STATUS_READ_ERROR;
+ return CAIRO_STATUS_READ_ERROR;
#endif
if (GIF_OK != DGifSlurp(gif)) {
@@ -560,9 +572,9 @@ Image::loadGIFFromBuffer(uint8_t *buf, unsigned len) {
dst_data++;
src_data++;
}
- }
+ }
}
- } else {
+ } else {
// Image is interlaced so that it streams nice over 14.4k and 28.8k modems :)
// We first load in 1/8 of the image, followed by another 1/8, followed by
// 1/4 and finally the remaining 1/2.
@@ -700,7 +712,7 @@ Image::decodeJPEGIntoSurface(jpeg_decompress_struct *args) {
*pixel = 255 << 24
| src[bx + 0] << 16
| src[bx + 1] << 8
- | src[bx + 2];
+ | src[bx + 2];
}
}
}
View
15 src/JPEGStream.h
@@ -30,6 +30,7 @@ init_closure_destination(j_compress_ptr cinfo){
boolean
empty_closure_output_buffer(j_compress_ptr cinfo){
+ NanScope();
closure_destination_mgr *dest = (closure_destination_mgr *) cinfo->dest;
Local<Object> buf = NanNewBufferHandle((char *)dest->buffer, dest->bufsize);
Local<Value> argv[3] = {
@@ -45,6 +46,7 @@ empty_closure_output_buffer(j_compress_ptr cinfo){
void
term_closure_destination(j_compress_ptr cinfo){
+ NanScope();
closure_destination_mgr *dest = (closure_destination_mgr *) cinfo->dest;
/* emit remaining data */
size_t remaining = dest->bufsize - cinfo->dest->free_in_buffer;
@@ -80,7 +82,7 @@ jpeg_closure_dest(j_compress_ptr cinfo, closure_t * closure, int bufsize){
(*cinfo->mem->alloc_small) ((j_common_ptr) cinfo, JPOOL_PERMANENT,
sizeof(closure_destination_mgr));
}
-
+
dest = (closure_destination_mgr *) cinfo->dest;
cinfo->dest->init_destination = &init_closure_destination;
@@ -96,6 +98,16 @@ jpeg_closure_dest(j_compress_ptr cinfo, closure_t * closure, int bufsize){
}
void
+jpeg_free_custom_allocations(j_compress_ptr cinfo){
+ closure_destination_mgr * dest;
+ dest = (closure_destination_mgr *) cinfo->dest;
+ if (dest->buffer) {
+ free(dest->buffer);
+ dest->buffer = NULL;
+ }
+}
+
+void
write_to_jpeg_stream(cairo_surface_t *surface, int bufsize, int quality, bool progressive, closure_t *closure){
int w = cairo_image_surface_get_width(surface);
int h = cairo_image_surface_get_height(surface);
@@ -137,6 +149,7 @@ write_to_jpeg_stream(cairo_surface_t *surface, int bufsize, int quality, bool pr
}
free(dst);
jpeg_finish_compress(&cinfo);
+ jpeg_free_custom_allocations(&cinfo);
jpeg_destroy_compress(&cinfo);
}
Something went wrong with that request. Please try again.