]> arthur.barton.de Git - bup.git/commitdiff
Check that all context managed objects are properly closed
authorRob Browning <rlb@defaultvalue.org>
Sun, 24 Oct 2021 17:24:16 +0000 (12:24 -0500)
committerRob Browning <rlb@defaultvalue.org>
Mon, 22 Nov 2021 16:35:28 +0000 (10:35 -0600)
Ensure all of our context managed objects have a __del__ that asserts
that the instance has been properly closed so that we'll be more
likely to notice related oversights.

This will only work in cases where __del__ is called before shutdown,
but that should normally be the case for cpython.

Signed-off-by: Rob Browning <rlb@defaultvalue.org>
Tested-by: Rob Browning <rlb@defaultvalue.org>
lib/bup/bloom.py
lib/bup/client.py
lib/bup/cmd/split.py
lib/bup/compat.py
lib/bup/git.py
lib/bup/helpers.py
lib/bup/hlinkdb.py
lib/bup/index.py
lib/bup/midx.py
lib/bup/repo.py
lib/bup/vfs.py

index 97024b90f7da67def21e0296124ea653769392ed..b686127f294026911f442a8ff4ac7cbcd62a011b 100644 (file)
@@ -107,6 +107,7 @@ bloom_add = _helpers.bloom_add
 class ShaBloom:
     """Wrapper which contains data from multiple index files. """
     def __init__(self, filename, f=None, readwrite=False, expected=-1):
+        self.closed = False
         self.name = filename
         self.readwrite = readwrite
         self.file = None
@@ -183,6 +184,7 @@ class ShaBloom:
         return self.map and self.bits
 
     def close(self):
+        self.closed = True
         try:
             if self.map and self.readwrite:
                 debug2("bloom: closing with %d entries\n" % self.entries)
@@ -198,6 +200,9 @@ class ShaBloom:
         finally:  # This won't handle pending exceptions correctly in py2
             self._init_failed()
 
+    def __del__(self):
+        assert self.closed
+
     def __enter__(self):
         return self
 
index ed902b74479c0f5db4ecb39689cd852919d470cd..7c284c4af4aa613db536cf5db637382988e1db02 100644 (file)
@@ -69,6 +69,7 @@ def parse_remote(remote):
 
 class Client:
     def __init__(self, remote, create=False):
+        self.closed = False
         self._busy = self.conn = None
         self.sock = self.p = self.pout = self.pin = None
         is_reverse = environ.get(b'BUP_SERVER_REVERSE')
@@ -117,14 +118,8 @@ class Client:
             self.check_ok()
         self.sync_indexes()
 
-    def __enter__(self):
-        return self
-
-    def __exit__(self, type, value, traceback):
-        with pending_raise(value, rethrow=False):
-            self.close()
-
     def close(self):
+        self.closed = True
         if self.conn and not self._busy:
             self.conn.write(b'quit\n')
         if self.pin:
@@ -146,6 +141,16 @@ class Client:
         self.conn = None
         self.sock = self.p = self.pin = self.pout = None
 
+    def __del__(self):
+        assert self.closed
+
+    def __enter__(self):
+        return self
+
+    def __exit__(self, type, value, traceback):
+        with pending_raise(value, rethrow=False):
+            self.close()
+
     def check_ok(self):
         if self.p:
             rv = self.p.poll()
@@ -494,6 +499,7 @@ class PackWriter_Remote(git.PackWriter):
                                 compression_level=compression_level,
                                 max_pack_size=max_pack_size,
                                 max_pack_objects=max_pack_objects)
+        self.remote_closed = False
         self.file = conn
         self.filename = b'remote socket'
         self.suggest_packs = suggest_packs
@@ -528,10 +534,15 @@ class PackWriter_Remote(git.PackWriter):
 
     def close(self):
         # Called by inherited __exit__
+        self.remote_closed = True
         id = self._end()
         self.file = None
         return id
 
+    def __del__(self):
+        assert self.remote_closed
+        super(PackWriter_Remote, self).__del__()
+
     def abort(self):
         raise ClientError("don't know how to abort remote pack writing")
 
index 59ad46c4c91fe2d94d21cc23f4f5627a3ede8340..87da1c28d0ebde152b749af1a23b1b86cb391aae 100755 (executable)
@@ -50,7 +50,9 @@ class NoOpPackWriter:
     def __exit__(self, type, value, traceback):
         return None  # since close() does nothing
     def close(self):
-        return None
+        assert self.closed
+    def __del__(self):
+        assert self.closed
     def new_blob(self, content):
         return git.calc_hash(b'blob', content)
     def new_tree(self, shalist):
index 68006f5178679e1796ecf55a67da8e761043a5d5..dfeb51aeacc50b62c495b3c3327dd82681e4f6d2 100644 (file)
@@ -48,13 +48,17 @@ if py3:
 
         """
         def __init__(self, ex, rethrow=True):
+            self.closed = False
             self.ex = ex
             self.rethrow = rethrow
         def __enter__(self):
             return None
         def __exit__(self, exc_type, exc_value, traceback):
+            self.closed = True
             if not exc_type and self.ex and self.rethrow:
                 raise self.ex
+        def __del__(self):
+            assert self.closed
 
     def items(x):
         return x.items()
@@ -146,12 +150,14 @@ else:  # Python 2
 
         """
         def __init__(self, ex, rethrow=True):
+            self.closed = False
             self.ex = ex
             self.rethrow = rethrow
         def __enter__(self):
             if self.ex:
                 add_ex_tb(self.ex)
         def __exit__(self, exc_type, exc_value, traceback):
+            self.closed = True
             if exc_value:
                 if self.ex:
                     add_ex_tb(exc_value)
@@ -159,6 +165,8 @@ else:  # Python 2
                 return
             if self.rethrow and self.ex:
                 raise self.ex
+        def __del__(self):
+            assert self.closed
 
     def dump_traceback(ex):
         stack = [ex]
@@ -217,15 +225,26 @@ 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 = False
+            # 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)
         def __enter__(self):
             return self
         def __exit__(self, type, value, traceback):
+            self._bup_closed = True
             with pending_raise(value, rethrow=False):
                 self.close()
+        def __del__(self):
+            assert self._bup_closed
 
 try:
     import bup_main
index e3b235c5b0f170b56f987f860956d07a4f178304..c038fd1587b2abeb5ea920db7e476cf32194de92 100644 (file)
@@ -414,6 +414,7 @@ class PackIdx:
 class PackIdxV1(PackIdx):
     """Object representation of a Git pack index (version 1) file."""
     def __init__(self, filename, f):
+        self.closed = False
         self.name = filename
         self.idxnames = [self.name]
         self.map = mmap_read(f)
@@ -453,15 +454,20 @@ class PackIdxV1(PackIdx):
             yield self.map[ofs : ofs + 20]
 
     def close(self):
+        self.closed = True
         if self.map is not None:
             self.shatable = None
             self.map.close()
             self.map = None
 
+    def __del__(self):
+        assert self.closed
+
 
 class PackIdxV2(PackIdx):
     """Object representation of a Git pack index (version 2) file."""
     def __init__(self, filename, f):
+        self.closed = False
         self.name = filename
         self.idxnames = [self.name]
         self.map = mmap_read(f)
@@ -509,11 +515,15 @@ class PackIdxV2(PackIdx):
             yield self.map[ofs : ofs + 20]
 
     def close(self):
+        self.closed = True
         if self.map is not None:
             self.shatable = None
             self.map.close()
             self.map = None
 
+    def __del__(self):
+        assert self.closed
+
 
 _mpi_count = 0
 class PackIdxList:
@@ -758,6 +768,7 @@ class PackWriter:
     def __init__(self, objcache_maker=_make_objcache, compression_level=1,
                  run_midx=True, on_pack_finish=None,
                  max_pack_size=None, max_pack_objects=None, repo_dir=None):
+        self.closed = False
         self.repo_dir = repo_dir or repo()
         self.file = None
         self.parentfd = None
@@ -949,6 +960,7 @@ class PackWriter:
 
     def abort(self):
         """Remove the pack file from disk."""
+        self.closed = True
         self._end(abort=True)
 
     def breakpoint(self):
@@ -959,8 +971,12 @@ class PackWriter:
 
     def close(self, run_midx=True):
         """Close the pack file and move it to its definitive path."""
+        self.closed = True
         return self._end(run_midx=run_midx)
 
+    def __del__(self):
+        assert self.closed
+
 
 class PackIdxV2Writer:
     def __init__(self):
index 0530b51aa81575dadf67fd2101c507be31607beb..fdc683bd7c2ca739699e760a0b1eda5c3a8f156d 100644 (file)
@@ -454,11 +454,16 @@ class NotOk(Exception):
 
 class BaseConn:
     def __init__(self, outp):
+        self._base_closed = False
         self.outp = outp
 
     def close(self):
+        self._base_closed = True
         while self._read(65536): pass
 
+    def __del__(self):
+        assert self._base_closed
+
     def _read(self, size):
         raise NotImplementedError("Subclasses must implement _read")
 
index 05a52ef01072e5974959e96f215e215610505b65..29d575eec3f8aae567b4656140e9185226e31b47 100644 (file)
@@ -19,6 +19,7 @@ class Error(Exception):
 
 class HLinkDB:
     def __init__(self, filename):
+        self.closed = False
         # Map a "dev:ino" node to a list of paths associated with that node.
         self._node_paths = {}
         # Map a path to a "dev:ino" node.
@@ -72,6 +73,7 @@ class HLinkDB:
         self._save_prepared = True
 
     def commit_save(self):
+        self.closed = True
         if not self._save_prepared:
             raise Error('cannot commit save of %r; no save prepared'
                         % self._filename)
@@ -89,6 +91,7 @@ class HLinkDB:
         self._save_prepared = None
 
     def abort_save(self):
+        self.closed = True
         if self._tmpname:
             os.unlink(self._tmpname)
             self._tmpname = None
@@ -100,6 +103,9 @@ class HLinkDB:
         with pending_raise(value, rethrow=True):
             self.abort_save()
 
+    def __del__(self):
+        assert self.closed
+
     def add_path(self, path, dev, ino):
         # Assume path is new.
         node = b'%d:%d' % (dev, ino)
index b9a4013be032bfd75d68e350250a43d4b9571d85..591079a7b59b2963b47692cfe9ef40d6277c103b 100644 (file)
@@ -50,14 +50,19 @@ class Error(Exception):
 
 class MetaStoreReader:
     def __init__(self, filename):
+        self._closed = False
         self._file = None
         self._file = open(filename, 'rb')
 
     def close(self):
+        self._closed = True
         if self._file:
             self._file.close()
             self._file = None
 
+    def __del__(self):
+        assert self._closed
+
     def __enter__(self):
         return self
 
@@ -75,6 +80,7 @@ class MetaStoreWriter:
     # truncation or corruption somewhat sensibly.
 
     def __init__(self, filename):
+        self._closed = False
         # Map metadata hashes to bupindex.meta offsets.
         self._offsets = {}
         self._filename = filename
@@ -101,10 +107,14 @@ class MetaStoreWriter:
         self._file = open(filename, 'ab')
 
     def close(self):
+        self._closed = True
         if self._file:
             self._file.close()
             self._file = None
 
+    def __del__(self):
+        assert self._closed
+
     def __enter__(self):
         return self
 
@@ -413,6 +423,7 @@ class ExistingEntry(Entry):
 
 class Reader:
     def __init__(self, filename):
+        self.closed = False
         self.filename = filename
         self.m = b''
         self.writable = False
@@ -488,12 +499,16 @@ class Reader:
             self.m.flush()
 
     def close(self):
+        self.closed = True
         self.save()
         if self.writable and self.m:
             self.m.close()
             self.m = None
             self.writable = False
 
+    def __del__(self):
+        assert self.closed
+
     def filter(self, prefixes, wantrecurse=None):
         for (rp, path) in reduce_paths(prefixes):
             any_entries = False
@@ -524,6 +539,7 @@ def pathsplit(p):
 
 class Writer:
     def __init__(self, filename, metastore, tmax):
+        self.closed = False
         self.rootlevel = self.level = Level([], None)
         self.f = None
         self.count = 0
@@ -545,6 +561,7 @@ class Writer:
             self.abort()
 
     def abort(self):
+        self.closed = True
         f = self.f
         self.f = None
         if f:
@@ -563,6 +580,7 @@ class Writer:
         assert(self.level == None)
 
     def close(self):
+        self.closed = True
         self.flush()
         f = self.f
         self.f = None
@@ -570,6 +588,9 @@ class Writer:
             f.close()
             os.rename(self.tmpname, self.filename)
 
+    def __del__(self):
+        assert self.closed
+
     def _add(self, ename, entry):
         if self.lastfile and self.lastfile <= ename:
             raise Error('%r must come before %r'
index e2458160915c51899026db91770272ddcc2a730b..1b3708e81fe0b714736f9aaba65bc5f5a86d30be 100644 (file)
@@ -22,6 +22,7 @@ class PackMidx:
     amounts of files.
     """
     def __init__(self, filename):
+        self.closed = False
         self.name = filename
         self.force_keep = False
         self.map = None
@@ -92,11 +93,15 @@ class PackMidx:
         return self.idxnames[self._get_idx_i(i)]
 
     def close(self):
+        self.closed = True
         if self.map is not None:
             self.fanout = self.shatable = self.whichlist = self.idxnames = None
             self.map.close()
             self.map = None
 
+    def __del__(self):
+        assert self.closed
+
     def exists(self, hash, want_source=False):
         """Return nonempty if the object exists in the index files."""
         global _total_searches, _total_steps
index 64feeb70f509031d1292c4fe379ac4a47e97eefd..87305fbb06323fcfb24e1afe3302d9d3c578f230 100644 (file)
@@ -21,6 +21,7 @@ def _repo_id(key):
 
 class LocalRepo:
     def __init__(self, repo_dir=None):
+        self.closed = False
         self.repo_dir = realpath(repo_dir or git.repo())
         self._cp = git.cp(self.repo_dir)
         self.update_ref = partial(git.update_ref, repo_dir=self.repo_dir)
@@ -28,7 +29,10 @@ class LocalRepo:
         self._id = _repo_id(self.repo_dir)
 
     def close(self):
-        pass
+        self.closed = True
+
+    def __del__(self):
+        assert self.closed
 
     def __enter__(self):
         return self
@@ -86,6 +90,7 @@ class LocalRepo:
 
 class RemoteRepo:
     def __init__(self, address):
+        self.closed = False
         self.address = address
         self.client = client.Client(address)
         self.new_packwriter = self.client.new_packwriter
@@ -94,10 +99,14 @@ class RemoteRepo:
         self._id = _repo_id(self.address)
 
     def close(self):
-        if self.client:
+        if not self.closed:
+            self.closed = True
             self.client.close()
             self.client = None
 
+    def __del__(self):
+        assert self.closed
+
     def __enter__(self):
         return self
 
index f5badf6982fd1f4ed87ceee32bf28dbc6a462af1..683c9ac8a327bb9422dc3a352e4f5db0435b2278 100644 (file)
@@ -195,6 +195,7 @@ class _ChunkReader:
 class _FileReader(object):
     def __init__(self, repo, oid, known_size=None):
         assert len(oid) == 20
+        self.closed = False
         self.oid = oid
         self.ofs = 0
         self.reader = None
@@ -231,10 +232,14 @@ class _FileReader(object):
         return buf
 
     def close(self):
-        pass
+        self.closed = True
+
+    def __del__(self):
+        assert self.closed
 
     def __enter__(self):
         return self
+
     def __exit__(self, type, value, traceback):
         with pending_raise(value, rethrow=False):
             self.close()