From 5ca57520b6d028b1a3ad81cbf927aa3ab219c9f1 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 11:39:57 +1000 Subject: [PATCH 01/26] Added Test for JPEG2000 memory leak, before the fix is added --- Tests/test_jp2k_leak.py | 30 ++++++++++++++++++++++++++++++ 1 file changed, 30 insertions(+) create mode 100644 Tests/test_jp2k_leak.py diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py new file mode 100644 index 000000000..ffd061e00 --- /dev/null +++ b/Tests/test_jp2k_leak.py @@ -0,0 +1,30 @@ +from helper import unittest, PillowTestCase, tearDownModule +import sys + +from PIL import Image +from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK + +# Limits for testing the leak +mem_limit = 512*1048576 +stack_size = 8*1048576 +iterations = (mem_limit/stack_size)*2 +codecs = dir(Image.core) +test_file = "Tests/images/rgb_trns_ycbc.jp2" + +@unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS") +class TestJp2kLeak(PillowTestCase): + def setUp(self): + if "jpeg2k_encoder" not in codecs or "jpeg2k_decoder" not in codecs: + self.skipTest('JPEG 2000 support not available') + + def test_leak(self): + setrlimit(RLIMIT_STACK, (stack_size, stack_size)) + setrlimit(RLIMIT_AS, (mem_limit, mem_limit)) + for count in range(iterations): + with Image.open(test_file) as im: + im.load() + im.close() + + +if __name__ == '__main__': + unittest.main() From 3acb06e9d229f9d7208e4a93b74d821ac8a8a7e1 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 13:34:44 +1000 Subject: [PATCH 02/26] Added the JPEG2000 memory leak fix to decode.c --- decode.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/decode.c b/decode.c index e9aa6a387..c56f42592 100644 --- a/decode.c +++ b/decode.c @@ -103,6 +103,8 @@ PyImaging_DecoderNew(int contextsize) static void _dealloc(ImagingDecoderObject* decoder) { + if (decoder->cleanup) + decoder->cleanup(&decoder->state); free(decoder->state.buffer); free(decoder->state.context); Py_XDECREF(decoder->lock); From c134e5ab456f6b8ebd5a18c93d7554c9131fa39a Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 13:43:11 +1000 Subject: [PATCH 03/26] Moved resource import to inside the function --- Tests/test_jp2k_leak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index ffd061e00..35040df20 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -2,7 +2,6 @@ from helper import unittest, PillowTestCase, tearDownModule import sys from PIL import Image -from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK # Limits for testing the leak mem_limit = 512*1048576 @@ -18,6 +17,7 @@ class TestJp2kLeak(PillowTestCase): self.skipTest('JPEG 2000 support not available') def test_leak(self): + from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK setrlimit(RLIMIT_STACK, (stack_size, stack_size)) setrlimit(RLIMIT_AS, (mem_limit, mem_limit)) for count in range(iterations): From bc1e1c148cf339c9024fa0381fecab1019063401 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 13:44:38 +1000 Subject: [PATCH 04/26] Casting the iterations variable to integer --- Tests/test_jp2k_leak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 35040df20..5b5fd6ab2 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -6,7 +6,7 @@ from PIL import Image # Limits for testing the leak mem_limit = 512*1048576 stack_size = 8*1048576 -iterations = (mem_limit/stack_size)*2 +iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" From f632baf4dc2497baa10b4a439c7dc6f95a55a1e1 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 14:04:24 +1000 Subject: [PATCH 05/26] Increased testing limit from 512MB to 1GB --- Tests/test_jp2k_leak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 5b5fd6ab2..89419866c 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -4,7 +4,7 @@ import sys from PIL import Image # Limits for testing the leak -mem_limit = 512*1048576 +mem_limit = 1024*1048576 stack_size = 8*1048576 iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) From a944ec92509ccc83a4802d890d70256f33e868cf Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 14:17:51 +1000 Subject: [PATCH 06/26] Increaded memory limit to 1.5Gb --- Tests/test_jp2k_leak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 89419866c..7c91b4b85 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -4,7 +4,7 @@ import sys from PIL import Image # Limits for testing the leak -mem_limit = 1024*1048576 +mem_limit = 1536*1048576 stack_size = 8*1048576 iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) From dea36ae610b9dcc1c3b5c9a55c661192d21c9cab Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 14:27:34 +1000 Subject: [PATCH 07/26] Tying 2GB max memory --- Tests/test_jp2k_leak.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 7c91b4b85..346416fe9 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -4,7 +4,7 @@ import sys from PIL import Image # Limits for testing the leak -mem_limit = 1536*1048576 +mem_limit = 2048*1048576 stack_size = 8*1048576 iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) From a1f66bf402da265279b87e4e384949772a18b49f Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 14:44:09 +1000 Subject: [PATCH 08/26] Added a check for PyPy, as it needs far more memory as CPython --- Tests/test_jp2k_leak.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 346416fe9..9b7db018f 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -1,10 +1,13 @@ from helper import unittest, PillowTestCase, tearDownModule import sys +from platform import python_implementation from PIL import Image # Limits for testing the leak -mem_limit = 2048*1048576 +mem_limit = 1024*1048576 +if python_implementation() == "PyPy": + mem_limit = 4096*1048576 stack_size = 8*1048576 iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) From 0cd1d9526daa9ab221c423b1caa8d2c37e8f1e9c Mon Sep 17 00:00:00 2001 From: root Date: Tue, 26 Aug 2014 16:36:15 +1000 Subject: [PATCH 09/26] Reverted back to 512M, PyPy doesn't seem to work anyway --- Tests/test_jp2k_leak.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 9b7db018f..70ae04c79 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -1,13 +1,11 @@ from helper import unittest, PillowTestCase, tearDownModule import sys -from platform import python_implementation +from os import getpid from PIL import Image # Limits for testing the leak -mem_limit = 1024*1048576 -if python_implementation() == "PyPy": - mem_limit = 4096*1048576 +mem_limit = 512*1048576 stack_size = 8*1048576 iterations = int(mem_limit/stack_size)*2 codecs = dir(Image.core) From 5a47b2bc845b899bab46589bc5a5b57389ec9637 Mon Sep 17 00:00:00 2001 From: root Date: Tue, 26 Aug 2014 16:56:35 +1000 Subject: [PATCH 10/26] Sanity check :) --- Tests/test_jp2k_leak.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leak.py index 70ae04c79..4c3b5b365 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leak.py @@ -26,6 +26,18 @@ class TestJp2kLeak(PillowTestCase): im.load() im.close() + def self_sanity_check(self): + # Arrange + j2k = Image.open('Tests/images/rgb_trns_ycbc.j2k') + jp2 = Image.open('Tests/images/rgb_trns_ycbc.jp2') + + # Act + j2k.load() + jp2.load() + + # Assert + self.assertEqual(j2k.mode, 'RGBA') + self.assertEqual(jp2.mode, 'RGBA') if __name__ == '__main__': unittest.main() From 72e2a6cade4a17a964da6435eacb58478f783a08 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 22:22:35 +1000 Subject: [PATCH 11/26] Fix memory leak in JPEG2000 decoding, and JPEG decoding using PyPy --- .../{test_jp2k_leak.py => test_jp2k_leaks.py} | 22 ++++--------------- 1 file changed, 4 insertions(+), 18 deletions(-) rename Tests/{test_jp2k_leak.py => test_jp2k_leaks.py} (63%) diff --git a/Tests/test_jp2k_leak.py b/Tests/test_jp2k_leaks.py similarity index 63% rename from Tests/test_jp2k_leak.py rename to Tests/test_jp2k_leaks.py index 4c3b5b365..1b7c68907 100644 --- a/Tests/test_jp2k_leak.py +++ b/Tests/test_jp2k_leaks.py @@ -1,18 +1,18 @@ from helper import unittest, PillowTestCase, tearDownModule import sys -from os import getpid +from StringIO import StringIO from PIL import Image # Limits for testing the leak -mem_limit = 512*1048576 +mem_limit = 768*1048576 stack_size = 8*1048576 -iterations = int(mem_limit/stack_size)*2 +iterations = int((mem_limit/stack_size)*20) codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" @unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS") -class TestJp2kLeak(PillowTestCase): +class TestJpegLeaks(PillowTestCase): def setUp(self): if "jpeg2k_encoder" not in codecs or "jpeg2k_decoder" not in codecs: self.skipTest('JPEG 2000 support not available') @@ -24,20 +24,6 @@ class TestJp2kLeak(PillowTestCase): for count in range(iterations): with Image.open(test_file) as im: im.load() - im.close() - - def self_sanity_check(self): - # Arrange - j2k = Image.open('Tests/images/rgb_trns_ycbc.j2k') - jp2 = Image.open('Tests/images/rgb_trns_ycbc.jp2') - - # Act - j2k.load() - jp2.load() - - # Assert - self.assertEqual(j2k.mode, 'RGBA') - self.assertEqual(jp2.mode, 'RGBA') if __name__ == '__main__': unittest.main() From 7db19efe750ad9dc4b8dfc06d40d013e1ace51a9 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 22:25:23 +1000 Subject: [PATCH 12/26] Reduced JPEG2000 test iterations, and added cleanup for decoding using PyPy --- PIL/ImageFile.py | 2 ++ Tests/test_jp2k_leaks.py | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 5e4745d76..0550137ea 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -227,6 +227,8 @@ class ImageFile(Image.Image): break b = b[n:] t = t + n + # Need to cleanup here to prevent leaks in PyPy + d.cleanup() self.tile = [] self.readonly = readonly diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index 1b7c68907..991a5cbf1 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -7,7 +7,7 @@ from PIL import Image # Limits for testing the leak mem_limit = 768*1048576 stack_size = 8*1048576 -iterations = int((mem_limit/stack_size)*20) +iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" From b78e5444f4c4725d24d9720f3b05a4c20d4c432d Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 00:01:20 +1000 Subject: [PATCH 13/26] Prevent multiple calls to ImagingIncrementalCodecDestroy --- libImaging/Jpeg2KDecode.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libImaging/Jpeg2KDecode.c b/libImaging/Jpeg2KDecode.c index 97ec81003..1477e68b9 100644 --- a/libImaging/Jpeg2KDecode.c +++ b/libImaging/Jpeg2KDecode.c @@ -800,6 +800,9 @@ ImagingJpeg2KDecodeCleanup(ImagingCodecState state) { if (context->decoder) ImagingIncrementalCodecDestroy(context->decoder); + /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ + context->decoder = NULL; + return -1; } From bb738aef383a4df46b122cd910a4929a7fd8023c Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 00:15:19 +1000 Subject: [PATCH 14/26] Removed unnecessary StringIO dependency from JPEG2K leak test --- Tests/test_jp2k_leaks.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index 991a5cbf1..210e9ec62 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -1,7 +1,5 @@ from helper import unittest, PillowTestCase, tearDownModule import sys -from StringIO import StringIO - from PIL import Image # Limits for testing the leak @@ -11,6 +9,7 @@ iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" + @unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS") class TestJpegLeaks(PillowTestCase): def setUp(self): From caa95a26b2a0f1bf001a314b3efa4fb5824aed64 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 15:30:31 +1000 Subject: [PATCH 15/26] Added memory leak fix and testing for Encoder --- PIL/ImageFile.py | 2 ++ Tests/test_jp2k_leaks.py | 21 ++++++++++++++++++++- encode.c | 13 +++++++++++++ libImaging/Jpeg2KEncode.c | 4 ++++ 4 files changed, 39 insertions(+), 1 deletion(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 0550137ea..bdcc769ec 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -473,6 +473,7 @@ def _save(im, fp, tile, bufsize=0): break if s < 0: raise IOError("encoder error %d when writing image file" % s) + e.cleanup() else: # slight speedup: compress to real file object for e, b, o, a in tile: @@ -483,6 +484,7 @@ def _save(im, fp, tile, bufsize=0): s = e.encode_to_file(fh, bufsize) if s < 0: raise IOError("encoder error %d when writing image file" % s) + e.cleanup() try: fp.flush() except: pass diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index 210e9ec62..f99c1e73a 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -8,6 +8,8 @@ stack_size = 8*1048576 iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" +from commands import getoutput +from os import getpid @unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS") @@ -16,7 +18,7 @@ class TestJpegLeaks(PillowTestCase): if "jpeg2k_encoder" not in codecs or "jpeg2k_decoder" not in codecs: self.skipTest('JPEG 2000 support not available') - def test_leak(self): + def test_leak_load(self): from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK setrlimit(RLIMIT_STACK, (stack_size, stack_size)) setrlimit(RLIMIT_AS, (mem_limit, mem_limit)) @@ -24,5 +26,22 @@ class TestJpegLeaks(PillowTestCase): with Image.open(test_file) as im: im.load() + def test_leak_save(self): + from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK + try: + from cStringIO import StringIO + except ImportError: + from io import StringIO + setrlimit(RLIMIT_STACK, (stack_size, stack_size)) + setrlimit(RLIMIT_AS, (mem_limit, mem_limit)) + for count in range(iterations): + with Image.open(test_file) as im: + im.load() + test_output = StringIO() + im.save(test_output, "JPEG2000") + test_output.seek(0) + output = test_output.read() + + if __name__ == '__main__': unittest.main() diff --git a/encode.c b/encode.c index 3fa900b1d..34e3b8933 100644 --- a/encode.c +++ b/encode.c @@ -99,6 +99,18 @@ _dealloc(ImagingEncoderObject* encoder) PyObject_Del(encoder); } +static PyObject* +_encode_cleanup(ImagingEncoderObject* encoder, PyObject* args) +{ + int status = 0; + + if (encoder->cleanup){ + status = encoder->cleanup(&encoder->state); + } + + return Py_BuildValue("i", status); +} + static PyObject* _encode(ImagingEncoderObject* encoder, PyObject* args) { @@ -239,6 +251,7 @@ _setimage(ImagingEncoderObject* encoder, PyObject* args) static struct PyMethodDef methods[] = { {"encode", (PyCFunction)_encode, 1}, + {"cleanup", (PyCFunction)_encode_cleanup, 1}, {"encode_to_file", (PyCFunction)_encode_to_file, 1}, {"setimage", (PyCFunction)_setimage, 1}, {NULL, NULL} /* sentinel */ diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c index e8eef08c1..b4c5284cb 100644 --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -585,6 +585,10 @@ ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { if (context->encoder) ImagingIncrementalCodecDestroy(context->encoder); + /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ + context->encoder = NULL; + + return -1; } From e791aa0325abd285b4b966d0a391fb3c1ee55bac Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 15:42:43 +1000 Subject: [PATCH 16/26] Removed unneeded dependencies --- Tests/test_jp2k_leaks.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index f99c1e73a..332520efb 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -8,8 +8,6 @@ stack_size = 8*1048576 iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) test_file = "Tests/images/rgb_trns_ycbc.jp2" -from commands import getoutput -from os import getpid @unittest.skipIf(sys.platform.startswith('win32'), "requires Unix or MacOS") From 11e34d695a651f155aea4828dc8bb7018f6f1204 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 16:03:07 +1000 Subject: [PATCH 17/26] Inwcreased max memory limit, and switched save test to use BytesIO instead of StringIO --- Tests/test_jp2k_leaks.py | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index 332520efb..b0d84f47f 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -1,9 +1,10 @@ from helper import unittest, PillowTestCase, tearDownModule import sys from PIL import Image +from io import BytesIO # Limits for testing the leak -mem_limit = 768*1048576 +mem_limit = 1024*1048576 stack_size = 8*1048576 iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) @@ -26,16 +27,12 @@ class TestJpegLeaks(PillowTestCase): def test_leak_save(self): from resource import setrlimit, RLIMIT_AS, RLIMIT_STACK - try: - from cStringIO import StringIO - except ImportError: - from io import StringIO setrlimit(RLIMIT_STACK, (stack_size, stack_size)) setrlimit(RLIMIT_AS, (mem_limit, mem_limit)) for count in range(iterations): with Image.open(test_file) as im: im.load() - test_output = StringIO() + test_output = BytesIO() im.save(test_output, "JPEG2000") test_output.seek(0) output = test_output.read() From dace8913b89e9f6d79fcdd39a5a17940c50faf41 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 16:40:38 +1000 Subject: [PATCH 18/26] Increase memory yet again --- Tests/test_jp2k_leaks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index b0d84f47f..d93fde71f 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -4,7 +4,7 @@ from PIL import Image from io import BytesIO # Limits for testing the leak -mem_limit = 1024*1048576 +mem_limit = 2048*1048576 stack_size = 8*1048576 iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) From 3da6768a7227cacdc4b21d5292be0dd7ae5c4179 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 18:50:57 +1000 Subject: [PATCH 19/26] Testing whether e.cleanup causes segfaults --- PIL/ImageFile.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index bdcc769ec..81549d8af 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -473,7 +473,7 @@ def _save(im, fp, tile, bufsize=0): break if s < 0: raise IOError("encoder error %d when writing image file" % s) - e.cleanup() + #e.cleanup() else: # slight speedup: compress to real file object for e, b, o, a in tile: @@ -484,7 +484,7 @@ def _save(im, fp, tile, bufsize=0): s = e.encode_to_file(fh, bufsize) if s < 0: raise IOError("encoder error %d when writing image file" % s) - e.cleanup() + #e.cleanup() try: fp.flush() except: pass From 7407371deb8b87e3049c9f1e8b04eac04a1fe6cc Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 19:13:42 +1000 Subject: [PATCH 20/26] NULLing pointers on cleanup --- PIL/ImageFile.py | 8 ++++---- libImaging/Jpeg2KDecode.c | 38 ++++++++++++++++++++------------------ libImaging/Jpeg2KEncode.c | 10 ++++++---- 3 files changed, 30 insertions(+), 26 deletions(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 81549d8af..1b5c2ebb2 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -135,8 +135,8 @@ class ImageFile(Image.Image): self.map = None use_mmap = self.filename and len(self.tile) == 1 # As of pypy 2.1.0, memory mapping was failing here. - use_mmap = use_mmap and not hasattr(sys, 'pypy_version_info') - + use_mmap = use_mmap and not hasattr(sys, 'pypy_version_info') + readonly = 0 # look for read/seek overrides @@ -473,7 +473,7 @@ def _save(im, fp, tile, bufsize=0): break if s < 0: raise IOError("encoder error %d when writing image file" % s) - #e.cleanup() + e.cleanup() else: # slight speedup: compress to real file object for e, b, o, a in tile: @@ -484,7 +484,7 @@ def _save(im, fp, tile, bufsize=0): s = e.encode_to_file(fh, bufsize) if s < 0: raise IOError("encoder error %d when writing image file" % s) - #e.cleanup() + e.cleanup() try: fp.flush() except: pass diff --git a/libImaging/Jpeg2KDecode.c b/libImaging/Jpeg2KDecode.c index 1477e68b9..bd6b59a62 100644 --- a/libImaging/Jpeg2KDecode.c +++ b/libImaging/Jpeg2KDecode.c @@ -68,7 +68,7 @@ j2k_skip(OPJ_OFF_T p_nb_bytes, void *p_user_data) typedef void (*j2k_unpacker_t)(opj_image_t *in, const JPEG2KTILEINFO *tileInfo, - const UINT8 *data, + const UINT8 *data, Imaging im); struct j2k_decode_unpacker { @@ -335,7 +335,7 @@ j2ku_srgb_rgb(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row = (UINT8 *)im->image[y0 + y] + x0 * 4; for (n = 0; n < 3; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 3; ++n) { UINT32 word = 0; @@ -388,7 +388,7 @@ j2ku_sycc_rgb(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row_start = row; for (n = 0; n < 3; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 3; ++n) { UINT32 word = 0; @@ -442,7 +442,7 @@ j2ku_srgba_rgba(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row = (UINT8 *)im->image[y0 + y] + x0 * 4; for (n = 0; n < 4; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 4; ++n) { UINT32 word = 0; @@ -494,7 +494,7 @@ j2ku_sycca_rgba(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row_start = row; for (n = 0; n < 4; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 4; ++n) { UINT32 word = 0; @@ -587,13 +587,13 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, /* Setup decompression context */ context->error_msg = NULL; - + opj_set_default_decoder_parameters(¶ms); params.cp_reduce = context->reduce; params.cp_layer = context->layers; - + codec = opj_create_decompress(context->format); - + if (!codec) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; @@ -616,7 +616,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, state->state = J2K_STATE_FAILED; goto quick_exit; } - + for (n = 1; n < image->numcomps; ++n) { if (image->comps[n].dx != 1 || image->comps[n].dy != 1) { state->errcode = IMAGING_CODEC_BROKEN; @@ -624,8 +624,8 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, goto quick_exit; } } - - /* + + /* Colorspace Number of components PIL mode ------------------------------------------------------ sRGB 3 RGB @@ -633,22 +633,22 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, gray 1 L or I gray 2 LA YCC 3 YCbCr - - + + If colorspace is unspecified, we assume: - + Number of components Colorspace ----------------------------------------- 1 gray 2 gray (+ alpha) 3 sRGB 4 sRGB (+ alpha) - + */ - + /* Find the correct unpacker */ color_space = image->color_space; - + if (color_space == OPJ_CLRSPC_UNSPECIFIED) { switch (image->numcomps) { case 1: case 2: color_space = OPJ_CLRSPC_GRAY; break; @@ -668,7 +668,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, if (!unpack) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; - goto quick_exit; + goto quick_exit; } /* Decode the image tile-by-tile; this means we only need use as much @@ -797,6 +797,8 @@ ImagingJpeg2KDecodeCleanup(ImagingCodecState state) { if (context->error_msg) free ((void *)context->error_msg); + context->error_msg = NULL; + if (context->decoder) ImagingIncrementalCodecDestroy(context->decoder); diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c index b4c5284cb..5d6caf30c 100644 --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -303,7 +303,7 @@ j2k_encode_entry(Imaging im, ImagingCodecState state, prec = 16; bpp = 12; } else if (strcmp (im->mode, "LA") == 0) { - components = 2; + components = 2; color_space = OPJ_CLRSPC_GRAY; pack = j2k_pack_la; } else if (strcmp (im->mode, "RGB") == 0) { @@ -340,7 +340,7 @@ j2k_encode_entry(Imaging im, ImagingCodecState state, context->error_msg = NULL; opj_set_default_encoder_parameters(¶ms); - + params.image_offset_x0 = context->offset_x; params.image_offset_y0 = context->offset_y; @@ -546,8 +546,8 @@ ImagingJpeg2KEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) return -1; if (state->state == J2K_STATE_START) { - int seekable = (context->format != OPJ_CODEC_J2K - ? INCREMENTAL_CODEC_SEEKABLE + int seekable = (context->format != OPJ_CODEC_J2K + ? INCREMENTAL_CODEC_SEEKABLE : INCREMENTAL_CODEC_NOT_SEEKABLE); context->encoder = ImagingIncrementalCodecCreate(j2k_encode_entry, @@ -582,6 +582,8 @@ ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { if (context->error_msg) free ((void *)context->error_msg); + context->error_msg = NULL; + if (context->encoder) ImagingIncrementalCodecDestroy(context->encoder); From 9ccc9307841bd8649d32fad91d2853bae6d436e0 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 19:21:40 +1000 Subject: [PATCH 21/26] Looking for cleanup segfault --- libImaging/Jpeg2KEncode.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) mode change 100644 => 100755 libImaging/Jpeg2KEncode.c diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c old mode 100644 new mode 100755 index 5d6caf30c..1ed5e08fc --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -584,11 +584,11 @@ ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { context->error_msg = NULL; - if (context->encoder) - ImagingIncrementalCodecDestroy(context->encoder); + //if (context->encoder) + // ImagingIncrementalCodecDestroy(context->encoder); /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ - context->encoder = NULL; + //context->encoder = NULL; return -1; From e4e1f5c2d4146d42806684f7603819f196b3b67e Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 19:30:19 +1000 Subject: [PATCH 22/26] More testing... --- libImaging/Jpeg2KEncode.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c index 1ed5e08fc..e7b5a78c3 100755 --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -576,13 +576,13 @@ int ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { JPEG2KENCODESTATE *context = (JPEG2KENCODESTATE *)state->context; - if (context->quality_layers) - Py_DECREF(context->quality_layers); + //if (context->quality_layers) + // Py_DECREF(context->quality_layers); - if (context->error_msg) - free ((void *)context->error_msg); + //if (context->error_msg) + // free ((void *)context->error_msg); - context->error_msg = NULL; + //context->error_msg = NULL; //if (context->encoder) // ImagingIncrementalCodecDestroy(context->encoder); From dd221d9ec061d51b53c5f07daef044f92bb5a2e5 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Wed, 27 Aug 2014 19:54:33 +1000 Subject: [PATCH 23/26] Don't Py_DECREF context->quality_layers if there is no encoder --- libImaging/Jpeg2KEncode.c | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c index e7b5a78c3..d493cd637 100755 --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -555,7 +555,6 @@ ImagingJpeg2KEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) INCREMENTAL_CODEC_WRITE, seekable, context->fd); - if (!context->encoder) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; @@ -576,19 +575,19 @@ int ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { JPEG2KENCODESTATE *context = (JPEG2KENCODESTATE *)state->context; - //if (context->quality_layers) - // Py_DECREF(context->quality_layers); + if (context->quality_layers && context->encoder) + Py_DECREF(context->quality_layers); - //if (context->error_msg) - // free ((void *)context->error_msg); + if (context->error_msg) + free ((void *)context->error_msg); - //context->error_msg = NULL; + context->error_msg = NULL; - //if (context->encoder) - // ImagingIncrementalCodecDestroy(context->encoder); + if (context->encoder) + ImagingIncrementalCodecDestroy(context->encoder); - /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ - //context->encoder = NULL; + /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ + context->encoder = NULL; return -1; From 94194ed24803249c379e65c57e7aab6c3f208b47 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Thu, 28 Aug 2014 21:33:18 +1000 Subject: [PATCH 24/26] Cleaning up. Reduced memory limit to 1GB --- PIL/ImageFile.py | 4 ++-- Tests/test_jp2k_leaks.py | 2 +- libImaging/Jpeg2KDecode.c | 40 +++++++++++++++++++-------------------- libImaging/Jpeg2KEncode.c | 10 +++++----- 4 files changed, 28 insertions(+), 28 deletions(-) diff --git a/PIL/ImageFile.py b/PIL/ImageFile.py index 1b5c2ebb2..bdcc769ec 100644 --- a/PIL/ImageFile.py +++ b/PIL/ImageFile.py @@ -135,8 +135,8 @@ class ImageFile(Image.Image): self.map = None use_mmap = self.filename and len(self.tile) == 1 # As of pypy 2.1.0, memory mapping was failing here. - use_mmap = use_mmap and not hasattr(sys, 'pypy_version_info') - + use_mmap = use_mmap and not hasattr(sys, 'pypy_version_info') + readonly = 0 # look for read/seek overrides diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py index d93fde71f..b0d84f47f 100644 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -4,7 +4,7 @@ from PIL import Image from io import BytesIO # Limits for testing the leak -mem_limit = 2048*1048576 +mem_limit = 1024*1048576 stack_size = 8*1048576 iterations = int((mem_limit/stack_size)*2) codecs = dir(Image.core) diff --git a/libImaging/Jpeg2KDecode.c b/libImaging/Jpeg2KDecode.c index bd6b59a62..abf8cebbe 100644 --- a/libImaging/Jpeg2KDecode.c +++ b/libImaging/Jpeg2KDecode.c @@ -68,7 +68,7 @@ j2k_skip(OPJ_OFF_T p_nb_bytes, void *p_user_data) typedef void (*j2k_unpacker_t)(opj_image_t *in, const JPEG2KTILEINFO *tileInfo, - const UINT8 *data, + const UINT8 *data, Imaging im); struct j2k_decode_unpacker { @@ -335,7 +335,7 @@ j2ku_srgb_rgb(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row = (UINT8 *)im->image[y0 + y] + x0 * 4; for (n = 0; n < 3; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 3; ++n) { UINT32 word = 0; @@ -388,7 +388,7 @@ j2ku_sycc_rgb(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row_start = row; for (n = 0; n < 3; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 3; ++n) { UINT32 word = 0; @@ -442,7 +442,7 @@ j2ku_srgba_rgba(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row = (UINT8 *)im->image[y0 + y] + x0 * 4; for (n = 0; n < 4; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 4; ++n) { UINT32 word = 0; @@ -494,7 +494,7 @@ j2ku_sycca_rgba(opj_image_t *in, const JPEG2KTILEINFO *tileinfo, UINT8 *row_start = row; for (n = 0; n < 4; ++n) data[n] = &cdata[n][csiz[n] * y * w]; - + for (x = 0; x < w; ++x) { for (n = 0; n < 4; ++n) { UINT32 word = 0; @@ -587,13 +587,13 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, /* Setup decompression context */ context->error_msg = NULL; - + opj_set_default_decoder_parameters(¶ms); params.cp_reduce = context->reduce; params.cp_layer = context->layers; - + codec = opj_create_decompress(context->format); - + if (!codec) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; @@ -616,7 +616,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, state->state = J2K_STATE_FAILED; goto quick_exit; } - + for (n = 1; n < image->numcomps; ++n) { if (image->comps[n].dx != 1 || image->comps[n].dy != 1) { state->errcode = IMAGING_CODEC_BROKEN; @@ -624,8 +624,8 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, goto quick_exit; } } - - /* + + /* Colorspace Number of components PIL mode ------------------------------------------------------ sRGB 3 RGB @@ -633,22 +633,22 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, gray 1 L or I gray 2 LA YCC 3 YCbCr - - + + If colorspace is unspecified, we assume: - + Number of components Colorspace ----------------------------------------- 1 gray 2 gray (+ alpha) 3 sRGB 4 sRGB (+ alpha) - + */ - + /* Find the correct unpacker */ color_space = image->color_space; - + if (color_space == OPJ_CLRSPC_UNSPECIFIED) { switch (image->numcomps) { case 1: case 2: color_space = OPJ_CLRSPC_GRAY; break; @@ -668,7 +668,7 @@ j2k_decode_entry(Imaging im, ImagingCodecState state, if (!unpack) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; - goto quick_exit; + goto quick_exit; } /* Decode the image tile-by-tile; this means we only need use as much @@ -797,11 +797,11 @@ ImagingJpeg2KDecodeCleanup(ImagingCodecState state) { if (context->error_msg) free ((void *)context->error_msg); - context->error_msg = NULL; - if (context->decoder) ImagingIncrementalCodecDestroy(context->decoder); + context->error_msg = NULL; + /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ context->decoder = NULL; diff --git a/libImaging/Jpeg2KEncode.c b/libImaging/Jpeg2KEncode.c index d493cd637..ea4bca2f2 100755 --- a/libImaging/Jpeg2KEncode.c +++ b/libImaging/Jpeg2KEncode.c @@ -303,7 +303,7 @@ j2k_encode_entry(Imaging im, ImagingCodecState state, prec = 16; bpp = 12; } else if (strcmp (im->mode, "LA") == 0) { - components = 2; + components = 2; color_space = OPJ_CLRSPC_GRAY; pack = j2k_pack_la; } else if (strcmp (im->mode, "RGB") == 0) { @@ -340,7 +340,7 @@ j2k_encode_entry(Imaging im, ImagingCodecState state, context->error_msg = NULL; opj_set_default_encoder_parameters(¶ms); - + params.image_offset_x0 = context->offset_x; params.image_offset_y0 = context->offset_y; @@ -546,8 +546,8 @@ ImagingJpeg2KEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) return -1; if (state->state == J2K_STATE_START) { - int seekable = (context->format != OPJ_CODEC_J2K - ? INCREMENTAL_CODEC_SEEKABLE + int seekable = (context->format != OPJ_CODEC_J2K + ? INCREMENTAL_CODEC_SEEKABLE : INCREMENTAL_CODEC_NOT_SEEKABLE); context->encoder = ImagingIncrementalCodecCreate(j2k_encode_entry, @@ -555,6 +555,7 @@ ImagingJpeg2KEncode(Imaging im, ImagingCodecState state, UINT8 *buf, int bytes) INCREMENTAL_CODEC_WRITE, seekable, context->fd); + if (!context->encoder) { state->errcode = IMAGING_CODEC_BROKEN; state->state = J2K_STATE_FAILED; @@ -589,7 +590,6 @@ ImagingJpeg2KEncodeCleanup(ImagingCodecState state) { /* Prevent multiple calls to ImagingIncrementalCodecDestroy */ context->encoder = NULL; - return -1; } From b07baf4549829dd81703ad9e6c59c0dd7feb6de1 Mon Sep 17 00:00:00 2001 From: Josh Ware Date: Tue, 26 Aug 2014 13:22:53 +1000 Subject: [PATCH 25/26] Removed tearDownModule from Test import --- Tests/test_jp2k_leaks.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) mode change 100644 => 100755 Tests/test_jp2k_leaks.py diff --git a/Tests/test_jp2k_leaks.py b/Tests/test_jp2k_leaks.py old mode 100644 new mode 100755 index b0d84f47f..9dbb8c1f4 --- a/Tests/test_jp2k_leaks.py +++ b/Tests/test_jp2k_leaks.py @@ -1,4 +1,4 @@ -from helper import unittest, PillowTestCase, tearDownModule +from helper import unittest, PillowTestCase import sys from PIL import Image from io import BytesIO From 922a040629aeaeaa7700c0c7fd28b8abf955f7db Mon Sep 17 00:00:00 2001 From: wiredfool Date: Fri, 12 Sep 2014 22:45:02 -0700 Subject: [PATCH 26/26] Don't run memory leak test automatically. --- Tests/{test_jp2k_leaks.py => check_j2k_leaks.py} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename Tests/{test_jp2k_leaks.py => check_j2k_leaks.py} (100%) diff --git a/Tests/test_jp2k_leaks.py b/Tests/check_j2k_leaks.py similarity index 100% rename from Tests/test_jp2k_leaks.py rename to Tests/check_j2k_leaks.py