[Numpy-discussion] mmap bug

[Geoffrey Irving]

Hello,

If I create an mmap'ed array, and then generate another array
referencing only its base, destruction of the original mmap'ed array
closes the mmap.  The second array is then prone to segfaults.

I think the best fix is to put the responsibility for flushing the
mmap onto the actual mmap object (and therefore the base object)
directly, so that the numpy memmap object has no cleanup
responsibilities.  A patch to core/memmap.py follows.  It would be
nicer to put the mmap_flush class declaration somewhere outside the
function, but that conflicts with the local "import mmap."

Thanks,
Geoffrey

diff --git a/numpy/core/memmap.py b/numpy/core/memmap.py
index 83de1cf..b2cbc86 100644
--- a/numpy/core/memmap.py
+++ b/numpy/core/memmap.py
@@ -219,14 +219,25 @@ class memmap(ndarray):
         else:
             acc = mmap.ACCESS_WRITE

+        # Ensure writable mmaps are always flushed before close
+        if acc==mmap.ACCESS_WRITE:
+            class mmap_flush(mmap.mmap):
+                def __del__(self):
+                    try:
+                        self.flush()
+                    except ValueError: # mmap is already closed
+                        pass
+        else:
+            mmap_flush = mmap.mmap
+
         if sys.version_info[:2] >= (2,6):
             # The offset keyword in mmap.mmap needs Python >= 2.6
             start = offset - offset % mmap.ALLOCATIONGRANULARITY
             bytes -= start
             offset -= start
-            mm = mmap.mmap(fid.fileno(), bytes, access=acc, offset=start)
+            mm = mmap_flush(fid.fileno(), bytes, access=acc, offset=start)
         else:
-            mm = mmap.mmap(fid.fileno(), bytes, access=acc)
+            mm = mmap_flush(fid.fileno(), bytes, access=acc)

         self = ndarray.__new__(subtype, shape, dtype=descr, buffer=mm,
             offset=offset, order=order)
@@ -273,29 +284,9 @@ class memmap(ndarray):
         warnings.warn("Use ``flush``.", DeprecationWarning)
         self.flush()

-    def _close(self):
-        """Close the memmap file.  Only do this when deleting the object."""
-        if self.base is self._mmap:
-            # The python mmap probably causes flush on close, but
-            # we put this here for safety
-            self._mmap.flush()
-            self._mmap.close()
-            self._mmap = None
-
     def close(self):
         """Close the memmap file. Does nothing."""
         warnings.warn("``close`` is deprecated on memmap arrays.  Use del",
                       DeprecationWarning)

-    def __del__(self):
-        # We first check if we are the owner of the mmap, rather than
-        # a view, so deleting a view does not call _close
-        # on the parent mmap
-        if self._mmap is self.base:
-            try:
-                # First run tell() to see whether file is open
-                self._mmap.tell()
-            except ValueError:
-                pass
-            else:
-                self._close()
+    # No need to define __del__ since mmap_flush.__del__ will do the flush