Merge pull request #921 from wiredfool/jpeg-memoryleak

JPEG Encode memory leaks
This commit is contained in:
Hugo 2014-09-25 13:45:34 +03:00
commit 22862cee00
5 changed files with 261 additions and 25 deletions

212
Tests/check_jpeg_leaks.py Normal file
View File

@ -0,0 +1,212 @@
from helper import unittest, PillowTestCase, hopper
from PIL import Image
from io import BytesIO
import sys
iterations = 5000
"""
When run on a system without the jpeg leak fixes, the valgrind runs look like this.
NOSE_PROCESSES=0 NOSE_TIMEOUT=600 valgrind --tool=massif \
python test-installed.py -s -v Tests/check_jpeg_leaks.py
"""
@unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS")
class TestJpegLeaks(PillowTestCase):
"""
pre patch:
MB
31.62^ :
| @:@:@:@#::
| @:@:@@:@:@:@:@:@#::
| ::::::::@:@:@@:@:@:@:@:@#::
| :::::@::::::: ::::@:@:@@:@:@:@:@:@#::
| @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| ::::::@::::@:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| ::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| ::::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| : ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| @: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| @@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :@@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :@@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :@:@@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :@:@@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
| :@:@@: ::::::::: : :@: : @:::::::::::::@: : ::: ::::@:@:@@:@:@:@:@:@#::
0 +----------------------------------------------------------------------->Gi
0 8.535
post-patch:
MB
21.03^ :::@@:::@::::@@:::::::@@::::::::@::::::::::::@:::@:::::::@::::
| #:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| #:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :::#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| : :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| : :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| @: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| @@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| @@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| @@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@:@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@:@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@:@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@:@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
| :@:@@: :#:::@ :::@::::@ : :: : @ :::::: :@:: ::: :::: @:: @:::::::@::::
0 +----------------------------------------------------------------------->Gi
0 8.421
"""
def test_qtables_leak(self):
im = hopper('RGB')
standard_l_qtable = [int(s) for s in """
16 11 10 16 24 40 51 61
12 12 14 19 26 58 60 55
14 13 16 24 40 57 69 56
14 17 22 29 51 87 80 62
18 22 37 56 68 109 103 77
24 35 55 64 81 104 113 92
49 64 78 87 103 121 120 101
72 92 95 98 112 100 103 99
""".split(None)]
standard_chrominance_qtable = [int(s) for s in """
17 18 24 47 99 99 99 99
18 21 26 66 99 99 99 99
24 26 56 99 99 99 99 99
47 66 99 99 99 99 99 99
99 99 99 99 99 99 99 99
99 99 99 99 99 99 99 99
99 99 99 99 99 99 99 99
99 99 99 99 99 99 99 99
""".split(None)]
qtables = [standard_l_qtable,
standard_chrominance_qtable]
for count in range(iterations):
test_output = BytesIO()
im.save(test_output, "JPEG", qtables=qtables)
"""
pre patch:
MB
177.1^ #
| @@@#
| :@@@@@@#
| ::::@@@@@@#
| ::::::::@@@@@@#
| @@::::: ::::@@@@@@#
| @@@@ ::::: ::::@@@@@@#
| @@@@@@@ ::::: ::::@@@@@@#
| @@::@@@@@@@ ::::: ::::@@@@@@#
| @@@@ : @@@@@@@ ::::: ::::@@@@@@#
| @@@@@@ @@ : @@@@@@@ ::::: ::::@@@@@@#
| @@@@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| @::@@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| ::::@: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| :@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| ::@@::@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| @@::: @ ::@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| @::@ : : @ ::@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| :::@: @ : : @ ::@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
| @@@:: @: @ : : @ ::@@: : @: @@ @@ @@ @ @@ : @@@@@@@ ::::: ::::@@@@@@#
0 +----------------------------------------------------------------------->Gi
0 11.37
post patch:
MB
21.06^ ::::::::::::::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| ##::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| # ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| # ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| # ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @@@@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @ @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @ @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @ @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
| @ @@@# ::: ::::: : ::::::::::@::::@::::@::::@::::@::::@:::::::::@::::::
0 +----------------------------------------------------------------------->Gi
0 11.33
"""
def test_exif_leak(self):
im = hopper('RGB')
exif = b'12345678'*4096
for count in range(iterations):
test_output = BytesIO()
im.save(test_output, "JPEG", exif=exif)
"""
base case:
MB
20.99^ ::::: :::::::::::::::::::::::::::::::::::::::::::@:::
| ##: : ::::::@::::::: :::: :::: : : : : : : :::::::::::: :::@:::
| # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@# : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@@ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| :@@@@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| :@ @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| :@ @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| :@ @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
| :@ @@ @ # : : :: :: @:: :::: :::: :::: : : : : : : :::::::::::: :::@:::
0 +----------------------------------------------------------------------->Gi
0 7.882
"""
def test_base_save(self):
im = hopper('RGB')
for count in range(iterations):
test_output = BytesIO()
im.save(test_output, "JPEG")
if __name__ == '__main__':
unittest.main()

View File

@ -295,6 +295,21 @@ class TestFileJpeg(PillowTestCase):
1: standard_chrominance_qtable}), 1: standard_chrominance_qtable}),
30) 30)
# not a sequence
self.assertRaises(Exception, lambda: self.roundtrip(im, qtables='a'))
# sequence wrong length
self.assertRaises(Exception, lambda: self.roundtrip(im, qtables=[]))
# sequence wrong length
self.assertRaises(Exception, lambda: self.roundtrip(im, qtables=[1,2,3,4,5]))
# qtable entry not a sequence
self.assertRaises(Exception, lambda: self.roundtrip(im, qtables=[1]))
# qtable entry has wrong number of items
self.assertRaises(Exception, lambda: self.roundtrip(im, qtables=[[1,2,3,4]]))
@unittest.skipUnless(djpeg_available(), "djpeg not available") @unittest.skipUnless(djpeg_available(), "djpeg not available")
def test_load_djpeg(self): def test_load_djpeg(self):
img = Image.open(TEST_FILE) img = Image.open(TEST_FILE)

View File

@ -535,12 +535,12 @@ PyImaging_ZipEncoderNew(PyObject* self, PyObject* args)
#include "Jpeg.h" #include "Jpeg.h"
static unsigned int** get_qtables_arrays(PyObject* qtables, int* qtablesLen) { static unsigned int* get_qtables_arrays(PyObject* qtables, int* qtablesLen) {
PyObject* tables; PyObject* tables;
PyObject* table; PyObject* table;
PyObject* table_data; PyObject* table_data;
int i, j, num_tables; int i, j, num_tables;
unsigned int **qarrays; unsigned int *qarrays;
if ((qtables == NULL) || (qtables == Py_None)) { if ((qtables == NULL) || (qtables == Py_None)) {
return NULL; return NULL;
@ -554,10 +554,12 @@ static unsigned int** get_qtables_arrays(PyObject* qtables, int* qtablesLen) {
tables = PySequence_Fast(qtables, "expected a sequence"); tables = PySequence_Fast(qtables, "expected a sequence");
num_tables = PySequence_Size(qtables); num_tables = PySequence_Size(qtables);
if (num_tables < 1 || num_tables > NUM_QUANT_TBLS) { if (num_tables < 1 || num_tables > NUM_QUANT_TBLS) {
PyErr_SetString(PyExc_ValueError, "Not a valid numbers of quantization tables. Should be between 1 and 4."); PyErr_SetString(PyExc_ValueError,
"Not a valid number of quantization tables. Should be between 1 and 4.");
Py_DECREF(tables);
return NULL; return NULL;
} }
qarrays = (unsigned int**) PyMem_Malloc(num_tables * sizeof(unsigned int*)); qarrays = (unsigned int*) malloc(num_tables * DCTSIZE2 * sizeof(unsigned int));
if (!qarrays) { if (!qarrays) {
Py_DECREF(tables); Py_DECREF(tables);
PyErr_NoMemory(); PyErr_NoMemory();
@ -566,39 +568,33 @@ static unsigned int** get_qtables_arrays(PyObject* qtables, int* qtablesLen) {
for (i = 0; i < num_tables; i++) { for (i = 0; i < num_tables; i++) {
table = PySequence_Fast_GET_ITEM(tables, i); table = PySequence_Fast_GET_ITEM(tables, i);
if (!PySequence_Check(table)) { if (!PySequence_Check(table)) {
Py_DECREF(tables);
PyErr_SetString(PyExc_ValueError, "Invalid quantization tables"); PyErr_SetString(PyExc_ValueError, "Invalid quantization tables");
return NULL; goto JPEG_QTABLES_ERR;
} }
if (PySequence_Size(table) != DCTSIZE2) { if (PySequence_Size(table) != DCTSIZE2) {
Py_DECREF(tables); PyErr_SetString(PyExc_ValueError, "Invalid quantization table size");
PyErr_SetString(PyExc_ValueError, "Invalid quantization tables"); goto JPEG_QTABLES_ERR;
return NULL;
} }
table_data = PySequence_Fast(table, "expected a sequence"); table_data = PySequence_Fast(table, "expected a sequence");
qarrays[i] = (unsigned int*) PyMem_Malloc(DCTSIZE2 * sizeof(unsigned int));
if (!qarrays[i]) {
Py_DECREF(tables);
PyErr_NoMemory();
return NULL;
}
for (j = 0; j < DCTSIZE2; j++) { for (j = 0; j < DCTSIZE2; j++) {
qarrays[i][j] = PyInt_AS_LONG(PySequence_Fast_GET_ITEM(table_data, j)); qarrays[i * DCTSIZE2 + j] = PyInt_AS_LONG(PySequence_Fast_GET_ITEM(table_data, j));
} }
Py_DECREF(table_data);
} }
Py_DECREF(tables);
*qtablesLen = num_tables; *qtablesLen = num_tables;
JPEG_QTABLES_ERR:
Py_DECREF(tables); // Run on both error and not error
if (PyErr_Occurred()) { if (PyErr_Occurred()) {
PyMem_Free(qarrays); free(qarrays);
qarrays = NULL; qarrays = NULL;
return NULL;
} }
return qarrays; return qarrays;
} }
PyObject* PyObject*
PyImaging_JpegEncoderNew(PyObject* self, PyObject* args) PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
{ {
@ -614,7 +610,7 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
int xdpi = 0, ydpi = 0; int xdpi = 0, ydpi = 0;
int subsampling = -1; /* -1=default, 0=none, 1=medium, 2=high */ int subsampling = -1; /* -1=default, 0=none, 1=medium, 2=high */
PyObject* qtables=NULL; PyObject* qtables=NULL;
unsigned int **qarrays = NULL; unsigned int *qarrays = NULL;
int qtablesLen = 0; int qtablesLen = 0;
char* extra = NULL; char* extra = NULL;
int extra_size; int extra_size;
@ -638,7 +634,7 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
qarrays = get_qtables_arrays(qtables, &qtablesLen); qarrays = get_qtables_arrays(qtables, &qtablesLen);
if (extra && extra_size > 0) { if (extra && extra_size > 0) {
char* p = malloc(extra_size); char* p = malloc(extra_size); // Freed in JpegEncode, Case 5
if (!p) if (!p)
return PyErr_NoMemory(); return PyErr_NoMemory();
memcpy(p, extra, extra_size); memcpy(p, extra, extra_size);
@ -647,7 +643,7 @@ PyImaging_JpegEncoderNew(PyObject* self, PyObject* args)
extra = NULL; extra = NULL;
if (rawExif && rawExifLen > 0) { if (rawExif && rawExifLen > 0) {
char* pp = malloc(rawExifLen); char* pp = malloc(rawExifLen); // Freed in JpegEncode, Case 5
if (!pp) if (!pp)
return PyErr_NoMemory(); return PyErr_NoMemory();
memcpy(pp, rawExif, rawExifLen); memcpy(pp, rawExif, rawExifLen);

View File

@ -89,7 +89,9 @@ typedef struct {
int subsampling; int subsampling;
/* Custom quantization tables () */ /* Custom quantization tables () */
unsigned int **qtables; unsigned int *qtables;
/* in factors of DCTSIZE2 */
int qtablesLen; int qtablesLen;
/* Extra data (to be injected after header) */ /* Extra data (to be injected after header) */

View File

@ -153,7 +153,7 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
} }
for (i = 0; i < context->qtablesLen; i++) { for (i = 0; i < context->qtablesLen; i++) {
// TODO: Should add support for none baseline // TODO: Should add support for none baseline
jpeg_add_quant_table(&context->cinfo, i, context->qtables[i], jpeg_add_quant_table(&context->cinfo, i, &context->qtables[i * DCTSIZE2],
quality, TRUE); quality, TRUE);
} }
} else if (context->quality > 0) { } else if (context->quality > 0) {
@ -289,8 +289,19 @@ ImagingJpegEncode(Imaging im, ImagingCodecState state, UINT8* buf, int bytes)
jpeg_finish_compress(&context->cinfo); jpeg_finish_compress(&context->cinfo);
/* Clean up */ /* Clean up */
if (context->extra) if (context->extra) {
free(context->extra); free(context->extra);
context->extra = NULL;
}
if (context->rawExif) {
free(context->rawExif);
context->rawExif = NULL;
}
if (context->qtables) {
free(context->qtables);
context->qtables = NULL;
}
jpeg_destroy_compress(&context->cinfo); jpeg_destroy_compress(&context->cinfo);
/* if (jerr.pub.num_warnings) return BROKEN; */ /* if (jerr.pub.num_warnings) return BROKEN; */
state->errcode = IMAGING_CODEC_END; state->errcode = IMAGING_CODEC_END;