From: Rob Browning <rlb@defaultvalue.org>
Date: Sat, 1 Jan 2022 19:33:21 +0000 (-0600)
Subject: Detect failures to explicitly close mmaps in py3 too
X-Git-Url: https://arthur.barton.de/gitweb/?p=bup.git;a=commitdiff_plain;h=4448f184253125e1b8bd0c55d0098a335d15b90b

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 9def12a..0057503 100644
--- 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
-    from mmap import mmap
     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
-    import mmap as py_mmap
 
     ModuleNotFoundError = ImportError
 
@@ -225,30 +223,6 @@ else:  # Python 2
 
     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:
diff --git a/lib/bup/helpers.py b/lib/bup/helpers.py
index af635ae..965b6d0 100644
--- 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 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
@@ -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 ''
-    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
@@ -832,7 +833,7 @@ if _mincore:
             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 ..."
diff --git a/lib/bup/io.py b/lib/bup/io.py
index 512d364..a384f10 100644
--- a/lib/bup/io.py
+++ b/lib/bup/io.py
@@ -1,7 +1,9 @@
 
 from __future__ import absolute_import, print_function
+import mmap as py_mmap
 
 from bup import compat
+from bup.compat import pending_raise
 
 
 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
+
+
+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