Skip to content

Commit 6319891

Browse files
authored
Merge pull request #440 from matthewwithanm/fix/open-files-leak
Fixed #429 Do not leak open files after generation
2 parents 7e23384 + 6ee9313 commit 6319891

File tree

2 files changed

+40
-12
lines changed

2 files changed

+40
-12
lines changed

imagekit/specs/__init__.py

Lines changed: 15 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -143,23 +143,26 @@ def generate(self):
143143
raise MissingSource("The spec '%s' has no source file associated"
144144
" with it." % self)
145145

146-
file_opened_locally = False
147146
# TODO: Move into a generator base class
148147
# TODO: Factor out a generate_image function so you can create a generator and only override the PIL.Image creating part. (The tricky part is how to deal with original_format since generator base class won't have one.)
149-
try:
150-
img = open_image(self.source)
151-
except ValueError:
152148

153-
# Re-open the file -- https://code.djangoproject.com/ticket/13750
149+
closed = self.source.closed
150+
if closed:
151+
# Django file object should know how to reopen itself if it was closed
152+
# https://code.djangoproject.com/ticket/13750
154153
self.source.open()
155-
file_opened_locally = True
156-
img = open_image(self.source)
157154

158-
new_image = process_image(img, processors=self.processors,
159-
format=self.format, autoconvert=self.autoconvert,
160-
options=self.options)
161-
if file_opened_locally:
162-
self.source.close()
155+
try:
156+
img = open_image(self.source)
157+
new_image = process_image(img,
158+
processors=self.processors,
159+
format=self.format,
160+
autoconvert=self.autoconvert,
161+
options=self.options)
162+
finally:
163+
if closed:
164+
# We need to close the file if it was opened by us
165+
self.source.close()
163166
return new_image
164167

165168

tests/test_closing_fieldfiles.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
from nose.tools import assert_false, assert_true
2+
3+
from .models import Thumbnail
4+
from .utils import create_photo
5+
6+
7+
def test_do_not_leak_open_files():
8+
instance = create_photo('leak-test.jpg')
9+
source_file = instance.original_image
10+
# Ensure the FieldFile is closed before generation
11+
source_file.close()
12+
image_generator = Thumbnail(source=source_file)
13+
image_generator.generate()
14+
assert_true(source_file.closed)
15+
16+
17+
def test_do_not_close_open_files_after_generate():
18+
instance = create_photo('do-not-close-test.jpg')
19+
source_file = instance.original_image
20+
# Ensure the FieldFile is opened before generation
21+
source_file.open()
22+
image_generator = Thumbnail(source=source_file)
23+
image_generator.generate()
24+
assert_false(source_file.closed)
25+
source_file.close()

0 commit comments

Comments
 (0)