BF+BK - fix too many open filehandles

Author: matthew-brett

nibabel/analyze.py

@@ -1279,45 +1279,37 @@ def set_data_dtype(self, dtype):
     def get_shape(self):
         return self._data.shape
 
-    @staticmethod
-    def _get_open_files(file_map, mode='rb'):
-        ''' Utility method to open necessary files for read/write
-
-        This method is to allow for formats (nifti single in particular)
-        that may have the same file for header and image
-        '''
-        hdrf = file_map['header'].get_prepare_fileobj(mode=mode)
-        imgf = file_map['image'].get_prepare_fileobj(mode=mode)
-        return hdrf, imgf
-
-    def _close_filenames(self, file_map, hdrf, imgf):
-        ''' Utility method to close any files no longer required
-
-        Called by the image writing routines.
-
-        This method is to allow for formats (nifti single in particular)
-        that may have the same file for header and image
-        '''
-        if file_map['header'].fileobj is None: # was filename
-            hdrf.close()
-        if file_map['image'].fileobj is None: # was filename
-            imgf.close()
-
     @classmethod
     def from_file_map(klass, file_map):
         ''' class method to create image from mapping in `file_map ``
         '''
-        hdrf, imgf = klass._get_open_files(file_map, 'rb')
+        hdr_fh, img_fh = klass._get_fileholders(file_map)
+        hdrf = hdr_fh.get_prepare_fileobj(mode='rb')
         header = klass.header_class.from_fileobj(hdrf)
+        if hdr_fh.fileobj is None: # was filename
+            hdrf.close()
         affine = header.get_best_affine()
         hdr_copy = header.copy()
+        imgf = img_fh.fileobj
+        if imgf is None:
+            imgf = img_fh.filename
         data = klass.ImageArrayProxy(imgf, hdr_copy)
         img = klass(data, affine, header, file_map=file_map)
         img._load_cache = {'header': hdr_copy,
                            'affine': affine.copy(),
                            'file_map': copy_file_map(file_map)}
         return img
 
+    @staticmethod
+    def _get_fileholders(file_map):
+        """ Return fileholder for header and image
+
+        Allows single-file image types to return one fileholder for both types.
+        For Analyze there are two fileholders, one for the header, one for the
+        image.
+        """
+        return file_map['header'], file_map['image']
+
     def _write_header(self, header_file, header, slope, inter):
         ''' Utility routine to write header
 
@@ -1384,10 +1376,15 @@ def to_file_map(self, file_map=None):
         self.update_header()
         hdr = self.get_header()
         slope, inter, mn, mx = hdr.scaling_from_data(data)
-        hdrf, imgf = self._get_open_files(file_map, 'wb')
+        hdr_fh, img_fh = self._get_fileholders(file_map)
+        hdrf = hdr_fh.get_prepare_fileobj(mode='wb')
+        imgf = img_fh.get_prepare_fileobj(mode='wb')
         self._write_header(hdrf, hdr, slope, inter)
         self._write_image(imgf, data, hdr, slope, inter, mn, mx)
-        self._close_filenames(file_map, hdrf, imgf)
+        if hdr_fh.fileobj is None: # was filename
+            hdrf.close()
+        if img_fh.fileobj is None: # was filename
+            imgf.close()
         self._header = hdr
         self.file_map = file_map
 

nibabel/nifti1.py

@@ -1334,20 +1334,6 @@ class Nifti1PairHeader(Nifti1Header):
 class Nifti1Pair(analyze.AnalyzeImage):
     header_class = Nifti1PairHeader
 
-    @classmethod
-    def from_file_map(klass, file_map):
-        hdrf, imgf = klass._get_open_files(file_map, 'rb')
-        header = klass.header_class.from_fileobj(hdrf)
-        extra = None
-        affine = header.get_best_affine()
-        hdr_copy = header.copy()
-        data = klass.ImageArrayProxy(imgf, hdr_copy)
-        img = klass(data, affine, header, extra, file_map)
-        img._load_cache = {'header': hdr_copy,
-                           'affine': affine.copy(),
-                           'file_map': copy_file_map(file_map)}
-        return img
-
     def _write_header(self, header_file, header, slope, inter):
         super(Nifti1Pair, self)._write_header(header_file,
                                               header,
@@ -1383,13 +1369,13 @@ class Nifti1Image(Nifti1Pair):
     files_types = (('image', '.nii'),)
 
     @staticmethod
-    def _get_open_files(file_map, mode='rb'):
-        hdrf = file_map['image'].get_prepare_fileobj(mode=mode)
-        return hdrf, hdrf
+    def _get_fileholders(file_map):
+        """ Return fileholder for header and image
 
-    def _close_filenames(self, file_map, hdrf, imgf):
-        if file_map['image'].fileobj is None: # was filename
-            imgf.close()
+        For single-file niftis, the fileholder for the header and the image will
+        be the same
+        """
+        return file_map['image'], file_map['image']
 
     def _write_header(self, header_file, header, slope, inter):
         super(Nifti1Image, self)._write_header(header_file,

nibabel/tests/test_filehandles.py

@@ -0,0 +1,48 @@
+"""
+Check that loading an image does not use up filehandles.
+"""
+from __future__ import with_statement
+
+from os.path import join as pjoin
+import shutil
+from tempfile import mkdtemp
+from warnings import warn
+
+import numpy as np
+
+try:
+    import resource as res
+except ImportError:
+    # Not on Unix, guess limit
+    SOFT_LIMIT = 512
+else:
+    SOFT_LIMIT, HARD_LIMIT = res.getrlimit(res.RLIMIT_NOFILE)
+
+from ..loadsave import load, save
+from ..nifti1 import Nifti1Image
+
+from numpy.testing import (assert_array_almost_equal,
+                           assert_array_equal)
+
+from nose.tools import assert_true, assert_equal, assert_raises
+
+
+def test_multiload():
+    # Make a tiny image, save, load many times.  If we are leaking filehandles,
+    # this will cause us to run out and generate an error
+    N = SOFT_LIMIT + 100
+    if N > 5000:
+        warn('It would take too long to test file handles, aborting')
+        return
+    arr = np.arange(24).reshape((2,3,4))
+    img = Nifti1Image(arr, np.eye(4))
+    try:
+        tmpdir = mkdtemp()
+        fname = pjoin(tmpdir, 'test.img')
+        save(img, fname)
+        imgs = []
+        for i in range(N):
+            imgs.append(load(fname))
+    finally:
+        del img, imgs
+        shutil.rmtree(tmpdir)