Detect failures to explicitly close mmaps in py3 too

...since they're expensive, and the wrapper has detected issues a
number of times.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>

diff --git a/lib/bup/compat.py b/lib/bup/compat.py
index 9def12a9d729a7044a6102381d94d772ef31564b..00575036e27cc46f7fb6b3408e273831526b9daa 100644 (file)
--- a/lib/bup/compat.py
+++ b/lib/bup/compat.py
@@ -14,7 +14,6 @@ if py3:
 
     # pylint: disable=unused-import
     from contextlib import ExitStack, nullcontext
 
     # pylint: disable=unused-import
     from contextlib import ExitStack, nullcontext

-    from mmap import mmap

     from os import environb as environ
     from os import fsdecode, fsencode
     from shlex import quote
     from os import environb as environ
     from os import fsdecode, fsencode
     from shlex import quote


@@ -89,7 +88,6 @@ if py3:
 else:  # Python 2
 
     from contextlib import contextmanager
 else:  # Python 2
 
     from contextlib import contextmanager

-    import mmap as py_mmap

 
     ModuleNotFoundError = ImportError
 
 
     ModuleNotFoundError = ImportError
 


@@ -225,30 +223,6 @@ else:  # Python 2
 
     buffer = buffer
 
 
     buffer = buffer
 

-    assert not hasattr(py_mmap.mmap, '__del__')
-    assert not hasattr(py_mmap.mmap, '__enter__')
-    assert not hasattr(py_mmap.mmap, '__exit__')
-
-    class mmap(py_mmap.mmap):
-        def __init__(self, *args, **kwargs):
-            self._bup_closed = True
-            # Silence deprecation warnings.  mmap's current parent is
-            # object, which accepts no params and as of at least 2.7
-            # warns about them.
-            if py_mmap.mmap.__init__ is not object.__init__:
-                super(mmap, self).__init__(self, *args, **kwargs)
-            self._bup_closed = False
-        def close(self):
-            self._bup_closed = True
-            super(mmap, self).close()
-        def __enter__(self):
-            return self
-        def __exit__(self, type, value, traceback):
-            with pending_raise(value, rethrow=False):
-                self.close()
-        def __del__(self):
-            assert self._bup_closed
-

 try:
     import bup_main
 except ModuleNotFoundError:
 try:
     import bup_main
 except ModuleNotFoundError:

diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py
index af635ae6e8b78cdd54d37fe9ff20ac253f9f382f..965b6d072228ff9c6fdaa752cd280c4bf881ec2b 100644 (file)
--- a/lib/bup/helpers.py
+++ b/lib/bup/helpers.py
@@ -12,6 +12,7 @@ import hashlib, heapq, math, operator, time, tempfile
 
 from bup import _helpers
 from bup import compat
 
 from bup import _helpers
 from bup import compat

+from bup import io

 from bup.compat import argv_bytes, byte_int, nullcontext, pending_raise
 from bup.io import byte_stream, path_msg
 # This function should really be in helpers, not in bup.options.  But we
 from bup.compat import argv_bytes, byte_int, nullcontext, pending_raise
 from bup.io import byte_stream, path_msg
 # This function should really be in helpers, not in bup.options.  But we


@@ -770,7 +771,7 @@ def _mmap_do(f, sz, flags, prot, close):
         # string has all the same behaviour of a zero-length map, ie. it has
         # no elements :)
         return ''
         # string has all the same behaviour of a zero-length map, ie. it has
         # no elements :)
         return ''

-    map = compat.mmap(f.fileno(), sz, flags, prot)
+    map = io.mmap(f.fileno(), sz, flags, prot)

     if close:
         f.close()  # map will persist beyond file close
     return map
     if close:
         f.close()  # map will persist beyond file close
     return map


@@ -832,7 +833,7 @@ if _mincore:
             pos = _fmincore_chunk_size * ci;
             msize = min(_fmincore_chunk_size, st.st_size - pos)
             try:
             pos = _fmincore_chunk_size * ci;
             msize = min(_fmincore_chunk_size, st.st_size - pos)
             try:

-                m = compat.mmap(fd, msize, mmap.MAP_PRIVATE, 0, 0, pos)
+                m = io.mmap(fd, msize, mmap.MAP_PRIVATE, 0, 0, pos)

             except mmap.error as ex:
                 if ex.errno == errno.EINVAL or ex.errno == errno.ENODEV:
                     # Perhaps the file was a pipe, i.e. "... | bup split ..."
             except mmap.error as ex:
                 if ex.errno == errno.EINVAL or ex.errno == errno.ENODEV:
                     # Perhaps the file was a pipe, i.e. "... | bup split ..."

diff --git a/lib/bup/io.py b/lib/bup/io.py
index 512d36434ed57e800f3b1f647a8982a909ec6ac2..a384f1007e5d5f0452d646633a658b01b1dad9f0 100644 (file)
--- a/lib/bup/io.py
+++ b/lib/bup/io.py
@@ -1,7 +1,9 @@
 
 from __future__ import absolute_import, print_function
 
 from __future__ import absolute_import, print_function

+import mmap as py_mmap

 
 from bup import compat
 
 from bup import compat

+from bup.compat import pending_raise

 
 
 if compat.py_maj > 2:
 
 
 if compat.py_maj > 2:


@@ -20,3 +22,47 @@ else:
         """Return a string representation of a path."""
         # FIXME: configurability (might git-config quotePath be involved?)
         return x
         """Return a string representation of a path."""
         # FIXME: configurability (might git-config quotePath be involved?)
         return x

+
+
+assert not hasattr(py_mmap.mmap, '__del__')
+if hasattr(py_mmap.mmap, '__enter__'):
+    assert hasattr(py_mmap.mmap, '__exit__')
+
+class mmap(py_mmap.mmap):
+    '''mmap.mmap wrapper that detects and complains about any instances
+    that aren't explicitly closed.
+
+    '''
+
+    def __init__(self, *args, **kwargs):
+        self._bup_closed = True
+        # Silence deprecation warnings.  mmap's current parent is
+        # object, which accepts no params and as of at least 2.7
+        # warns about them.
+        if py_mmap.mmap.__init__ is not object.__init__:
+            super(mmap, self).__init__(self, *args, **kwargs)
+        self._bup_closed = False
+
+    def close(self):
+        self._bup_closed = True
+        super(mmap, self).close()
+
+    if hasattr(py_mmap.mmap, '__enter__'):
+        def __enter__(self):
+            super(mmap, self).__enter__()
+            return self
+        def __exit__(self, type, value, traceback):
+            # Don't call self.close() when the parent has its own __exit__;
+            # defer to it.
+            self._bup_closed = True
+            result = super(mmap, self).__exit__(type, value, traceback)
+            return result
+    else:
+        def __enter__(self):
+            return self
+        def __exit__(self, type, value, traceback):
+            with pending_raise(value, rethrow=False):
+                self.close()
+
+    def __del__(self):
+        assert self._bup_closed