Merge pull request #2330 from wiredfool/pr_2032

Close files after loading when possible.

PIL/DcxImagePlugin.py

@@ -41,7 +41,8 @@ class DcxImageFile(PcxImageFile):
 
     format = "DCX"
     format_description = "Intel DCX"
-
+    _close_exclusive_fp_after_loading = False
+    
     def _open(self):
 
         # Header

PIL/FliImagePlugin.py

@@ -37,7 +37,8 @@ class FliImageFile(ImageFile.ImageFile):
 
     format = "FLI"
     format_description = "Autodesk FLI/FLC Animation"
-
+    _close_exclusive_fp_after_loading = False
+    
     def _open(self):
 
         # HEAD

PIL/GifImagePlugin.py

@@ -47,6 +47,8 @@ class GifImageFile(ImageFile.ImageFile):
 
     format = "GIF"
     format_description = "Compuserve GIF"
+    _close_exclusive_fp_after_loading = False
+    
     global_palette = None
 
     def data(self):

PIL/ImImagePlugin.py

@@ -109,6 +109,7 @@ class ImImageFile(ImageFile.ImageFile):
 
     format = "IM"
     format_description = "IFUNC Image Memory"
+    _close_exclusive_fp_after_loading = False
 
     def _open(self):
 

PIL/Image.py

@@ -497,6 +497,7 @@ class Image(object):
     """
     format = None
     format_description = None
+    _close_exclusive_fp_after_loading = True
 
     def __init__(self):
         # FIXME: take "new" parameters / other image?
@@ -551,14 +552,25 @@ class Image(object):
         """
         try:
             self.fp.close()
+            self.fp = None
         except Exception as msg:
             logger.debug("Error closing: %s", msg)
 
+        if getattr(self, 'map', None):
+            self.map = None
+    
         # Instead of simply setting to None, we're setting up a
         # deferred error that will better explain that the core image
         # object is gone.
         self.im = deferred_error(ValueError("Operation on closed image"))
 
+    if sys.version_info >= (3,4,0):
+        def __del__(self):
+            if (hasattr(self, 'fp') and hasattr(self, '_exclusive_fp') 
+                and self.fp and self._exclusive_fp):
+                self.fp.close()
+            self.fp = None
+
     def _copy(self):
         self.load()
         self.im = self.im.copy()
@@ -2382,6 +2394,7 @@ def open(fp, mode="r"):
     if mode != "r":
         raise ValueError("bad mode %r" % mode)
 
+    exclusive_fp = False
     filename = ""
     if isPath(fp):
         filename = fp
@@ -2395,11 +2408,13 @@ def open(fp, mode="r"):
 
     if filename:
         fp = builtins.open(filename, "rb")
+        exclusive_fp = True
 
     try:
         fp.seek(0)
     except (AttributeError, io.UnsupportedOperation):
         fp = io.BytesIO(fp.read())
+        exclusive_fp = True
 
     prefix = fp.read(16)
 
@@ -2428,8 +2443,11 @@ def open(fp, mode="r"):
             im = _open_core(fp, filename, prefix)
 
     if im:
+        im._exclusive_fp = exclusive_fp
         return im
 
+    if exclusive_fp:
+        fp.close()
     raise IOError("cannot identify image file %r"
                   % (filename if filename else fp))
 

PIL/ImageFile.py

@@ -88,10 +88,13 @@ class ImageFile(Image.Image):
             # filename
             self.fp = open(fp, "rb")
             self.filename = fp
+            self._exclusive_fp = True
         else:
             # stream
             self.fp = fp
             self.filename = filename
+            # can be overridden
+            self._exclusive_fp = None
 
         try:
             self._open()
@@ -100,6 +103,9 @@ class ImageFile(Image.Image):
                 KeyError,  # unsupported mode
                 EOFError,  # got header but not the first frame
                 struct.error) as v:
+            # close the file only if we have opened it this constructor
+            if self._exclusive_fp:
+                self.fp.close()
             raise SyntaxError(v)
 
         if not self.mode or self.size[0] <= 0:
@@ -115,6 +121,8 @@ class ImageFile(Image.Image):
 
         # raise exception if something's wrong.  must be called
         # directly after open, and closes file when finished.
+        if self._exclusive_fp:
+            self.fp.close()
         self.fp = None
 
     def load(self):
@@ -234,14 +242,16 @@ class ImageFile(Image.Image):
         self.tile = []
         self.readonly = readonly
 
-        self.fp = None  # might be shared
+        self.load_end()
+
+        if self._exclusive_fp and self._close_exclusive_fp_after_loading:
+            self.fp.close()
+        self.fp = None
 
         if not self.map and not LOAD_TRUNCATED_IMAGES and err_code < 0:
             # still raised if decoder fails to return anything
             raise_ioerror(err_code)
 
-        self.load_end()
-
         return Image.Image.load(self)
 
     def load_prepare(self):

PIL/MicImagePlugin.py

@@ -39,6 +39,7 @@ class MicImageFile(TiffImagePlugin.TiffImageFile):
 
     format = "MIC"
     format_description = "Microsoft Image Composer"
+    _close_exclusive_fp_after_loading = False
 
     def _open(self):
 

PIL/MpoImagePlugin.py

@@ -39,7 +39,8 @@ class MpoImageFile(JpegImagePlugin.JpegImageFile):
 
     format = "MPO"
     format_description = "MPO (CIPA DC-007)"
-
+    _close_exclusive_fp_after_loading = False
+    
     def _open(self):
         self.fp.seek(0)  # prep the fp in order to pass the JPEG test
         JpegImagePlugin.JpegImageFile._open(self)

PIL/SpiderImagePlugin.py

@@ -97,6 +97,7 @@ class SpiderImageFile(ImageFile.ImageFile):
 
     format = "SPIDER"
     format_description = "Spider 2D image"
+    _close_exclusive_fp_after_loading = False
 
     def _open(self):
         # check header

PIL/TiffImagePlugin.py

@@ -884,6 +884,7 @@ class TiffImageFile(ImageFile.ImageFile):
 
     format = "TIFF"
     format_description = "Adobe TIFF"
+    _close_exclusive_fp_after_loading = False
 
     def _open(self):
         "Open the first image in a TIFF file"
@@ -969,6 +970,7 @@ class TiffImageFile(ImageFile.ImageFile):
             self.__frame += 1
         self.fp.seek(self._frame_pos[frame])
         self.tag_v2.load(self.fp)
+        self.__next = self.tag_v2.next
         # fill the legacy tag/ifd entries
         self.tag = self.ifd = ImageFileDirectory_v1.from_v2(self.tag_v2)
         self.__frame = frame
@@ -1008,6 +1010,12 @@ class TiffImageFile(ImageFile.ImageFile):
             return self._load_libtiff()
         return super(TiffImageFile, self).load()
 
+    def load_end(self):
+        # allow closing if we're on the first frame, there's no next
+        # This is the ImageFile.load path only, libtiff specific below.
+        if self.__frame == 0 and not self.__next:
+            self._close_exclusive_fp_after_loading = True
+
     def _load_libtiff(self):
         """ Overload method triggered when we detect a compressed tiff
             Calls out to libtiff """
@@ -1085,16 +1093,14 @@ class TiffImageFile(ImageFile.ImageFile):
         self.tile = []
         self.readonly = 0
         # libtiff closed the fp in a, we need to close self.fp, if possible
-        if hasattr(self.fp, 'close'):
-            if not self.__next:
+        if self._exclusive_fp:
+            if self.__frame == 0 and not self.__next:
                 self.fp.close()
-        self.fp = None  # might be shared
+                self.fp = None  # might be shared
 
         if err < 0:
             raise IOError(err)
 
-        self.load_end()
-
         return Image.Image.load(self)
 
     def _setup(self):

PIL/XpmImagePlugin.py

@@ -116,8 +116,6 @@ class XpmImageFile(ImageFile.ImageFile):
         for i in range(ysize):
             s[i] = self.fp.readline()[1:xsize+1].ljust(xsize)
 
-        self.fp = None
-
         return b"".join(s)
 
 #

Tests/test_file_jpeg.py

@@ -3,6 +3,7 @@ from helper import djpeg_available, cjpeg_available
 
 from io import BytesIO
 import os
+import sys
 
 from PIL import Image
 from PIL import ImageFile
@@ -529,5 +530,27 @@ class TestFileJpeg(PillowTestCase):
         self.assertEqual(im.info.get("dpi"), (72, 72))
 
 
+@unittest.skipUnless(sys.platform.startswith('win32'), "Windows only")
+class TestFileCloseW32(PillowTestCase):
+    def setUp(self):
+        if "jpeg_encoder" not in codecs or "jpeg_decoder" not in codecs:
+            self.skipTest("jpeg support not available")
+            
+    def test_fd_leak(self):
+        tmpfile = self.tempfile("temp.jpg")
+        import os
+
+        with Image.open("Tests/images/hopper.jpg") as im:
+            im.save(tmpfile)
+
+        im = Image.open(tmpfile)
+        fp = im.fp
+        self.assertFalse(fp.closed)
+        self.assertRaises(Exception, lambda: os.remove(tmpfile))
+        im.load()
+        self.assertTrue(fp.closed)
+        # this should not fail, as load should have closed the file. 
+        os.remove(tmpfile)
+
 if __name__ == '__main__':
     unittest.main()

Tests/test_file_libtiff.py

@@ -401,6 +401,19 @@ class TestFileLibTiff(LibTiffTestCase):
 
         TiffImagePlugin.READ_LIBTIFF = False
 
+    def test_multipage_nframes(self):
+        # issue #862
+        TiffImagePlugin.READ_LIBTIFF = True
+        im = Image.open('Tests/images/multipage.tiff')
+        frames = im.n_frames
+        self.assertEqual(frames, 3)
+        for idx in range(frames):
+            im.seek(0)
+            # Should not raise ValueError: I/O operation on closed file
+            im.load()
+
+        TiffImagePlugin.READ_LIBTIFF = False
+
     def test__next(self):
         TiffImagePlugin.READ_LIBTIFF = True
         im = Image.open('Tests/images/hopper.tif')

Tests/test_file_tiff.py

@@ -1,6 +1,7 @@
 import logging
 from io import BytesIO
 import struct
+import sys
 
 from helper import unittest, PillowTestCase, hopper, py3
 
@@ -468,6 +469,44 @@ class TestFileTiff(PillowTestCase):
 
         self.assertEqual(b'Dummy value', reloaded.info['icc_profile'])
 
+    def test_close_on_load(self):
+        # same as test_fd_leak, but runs on unixlike os
+        tmpfile = self.tempfile("temp.tif")
+
+        with Image.open("Tests/images/uint16_1_4660.tif") as im:
+            im.save(tmpfile)
+
+        im = Image.open(tmpfile)
+        fp = im.fp
+        self.assertFalse(fp.closed)
+        im.load()
+        self.assertTrue(fp.closed)
+
+        
+
+@unittest.skipUnless(sys.platform.startswith('win32'), "Windows only")
+class TestFileTiffW32(PillowTestCase):
+    def test_fd_leak(self):
+        tmpfile = self.tempfile("temp.tif")
+        import os
+
+        # this is an mmaped file. 
+        with Image.open("Tests/images/uint16_1_4660.tif") as im:
+            im.save(tmpfile)
+
+        im = Image.open(tmpfile)
+        fp = im.fp
+        self.assertFalse(fp.closed)
+        self.assertRaises(Exception, lambda: os.remove(tmpfile))
+        im.load()
+        self.assertTrue(fp.closed)
+
+        # this closes the mmap
+        im.close()
+        
+        # this should not fail, as load should have closed the file pointer,
+        # and close should have closed the mmap
+        os.remove(tmpfile)
 
 if __name__ == '__main__':
     unittest.main()