Some improvements to resource cleanup upon HDUList.close() to ensure …

…that filehandles are actually closed when HDUList.close() is called (with the exception of when a user is holding a reference to an mmap'd array--in this case we don't sweep it out from under their feet (though maybe a warning would be in order?). This was especially needed for compressed HDUs which sometimes were holding file handles open for longer than welcome, causing some tests to fail.

astropy/io/fits/hdu/base.py

@@ -756,6 +756,15 @@ def _writeto(self, fileobj, inplace=False, copy=False):
         self._data_size = datsize
         self._data_replaced = False
 
+    def _close(self, closed=True):
+        # If the data was mmap'd, close the underlying mmap (this will
+        # prevent any future access to the .data attribute if there are
+        # not other references to it; if there are other references then
+        # it is up to the user to clean those up
+        if (closed and self._data_loaded and
+                _get_array_mmap(self.data) is not None):
+            del self.data
+
 # For backwards-compatibility, though nobody should have
 # been using this directly:
 _AllHDU = _BaseHDU

astropy/io/fits/hdu/compressed.py

@@ -1,6 +1,7 @@
 # Licensed under a 3-clause BSD style license - see PYFITS.rst
 
 import ctypes
+import gc
 import math
 import re
 import time
@@ -16,7 +17,8 @@
 from ..column import KEYWORD_NAMES as TABLE_KEYWORD_NAMES
 from ..fitsrec import FITS_rec
 from ..header import Header
-from ..util import _is_pseudo_unsigned, _unsigned_zero, _is_int
+from ..util import (_is_pseudo_unsigned, _unsigned_zero, _is_int,
+                    _get_array_mmap)
 
 from ....extern.six import string_types, iteritems
 from ....utils import lazyproperty, deprecated
@@ -1436,6 +1438,26 @@ def compressed_data(self):
 
         return self.compressed_data
 
+    @compressed_data.deleter
+    def compressed_data(self):
+        # Deleting the compressed_data attribute has to be handled
+        # with a little care to prevent a reference leak
+        # First delete the ._coldefs attributes under it to break a possible
+        # reference cycle
+        if 'compressed_data' in self.__dict__:
+            del self.__dict__['compressed_data']._coldefs
+
+            # Now go ahead and delete from self.__dict__; normally
+            # lazyproperty.__delete__ does this for us, but we can prempt it to
+            # do some additional cleanup
+            del self.__dict__['compressed_data']
+
+            # If this file was mmap'd, numpy.memmap will hold open a file
+            # handle until the underlying mmap object is garbage-collected;
+            # since this reference leak can sometimes hang around longer than
+            # welcome go ahead and force a garbage collection
+            gc.collect()
+
     @lazyproperty
     @deprecated('0.3', alternative='the ``compressed_data`` attribute',
                 pending=True)
@@ -1641,6 +1663,10 @@ def _update_compressed_data(self):
                 del self._header['THEAP']
             self._theap = tbsize
 
+            # First delete the original compressed data, if it exists
+            del self.compressed_data
+
+
             # Compress the data.
             # The current implementation of compress_hdu assumes the empty
             # compressed data table has already been initialized in
@@ -1839,6 +1865,17 @@ def _writedata(self, fileobj):
             else:
                 del self.data
 
+    def _close(self, closed=True):
+        super(CompImageHDU, self)._close(closed=closed)
+
+        # Also make sure to close access to the compressed data mmaps
+        if (closed and self._data_loaded and
+                _get_array_mmap(self.compressed_data) is not None):
+            del self.compressed_data
+            # Close off the deprected compData attribute as well if it has been
+            # used
+            del self.compData
+
     # TODO: This was copied right out of _ImageBaseHDU; get rid of it once we
     # find a way to rewrite this class as either a subclass or wrapper for an
     # ImageHDU

astropy/io/fits/hdu/hdulist.py

@@ -712,6 +712,10 @@ def close(self, output_verify='exception', verbose=False, closed=True):
             if closed and hasattr(self._file, 'close'):
                 self._file.close()
 
+        # Give individual HDUs an opportunity to do on-close cleanup
+        for hdu in self:
+            hdu._close(closed=closed)
+
     def info(self, output=None):
         """
         Summarize the info of the HDUs in this `HDUList`.

astropy/io/fits/tests/test_image.py

@@ -859,7 +859,7 @@ def test_empty(self):
         """
 
         hdu = fits.CompImageHDU()
-        hdu.data is None
+        assert hdu.data is None
         hdu.writeto(self.temp('test.fits'))
 
         with fits.open(self.temp('test.fits'), mode='update') as hdul: