Преглед изворни кода

Make object store close() idempotent and warn on unclosed resources

Jelmer Vernooij пре 1 недеља
родитељ
комит
0afe402158
3 измењених фајлова са 43 додато и 9 уклоњено
  1. 4 0
      NEWS
  2. 24 5
      dulwich/dumb.py
  3. 15 4
      dulwich/object_store.py

+ 4 - 0
NEWS

@@ -3,6 +3,10 @@
  * Fix test failure when GPG raises ``InvalidSigners`` instead of
  * Fix test failure when GPG raises ``InvalidSigners`` instead of
    ``GPGMEError`` on systems without usable secret keys. (#2063)
    ``GPGMEError`` on systems without usable secret keys. (#2063)
 
 
+ * Object store ``close()`` methods can now be called multiple times safely.
+   Object stores now raise ``ResourceWarning`` when destroyed with unclosed
+   resources. (Jelmer Vernooij)
+
 0.25.1	2026-01-08
 0.25.1	2026-01-08
 
 
  * Add signature vendor system for signing and verifying Git objects.
  * Add signature vendor system for signing and verifying Git objects.

+ 24 - 5
dulwich/dumb.py

@@ -362,12 +362,31 @@ class DumbHTTPObjectStore(BaseObjectStore):
         """Add a set of objects to this object store."""
         """Add a set of objects to this object store."""
         raise NotImplementedError("Cannot add objects to dumb HTTP repository")
         raise NotImplementedError("Cannot add objects to dumb HTTP repository")
 
 
-    def __del__(self) -> None:
-        """Clean up temporary directory on deletion."""
-        if self._temp_pack_dir and os.path.exists(self._temp_pack_dir):
-            import shutil
+    def close(self) -> None:
+        """Close the object store and release resources.
+
+        This method cleans up the temporary pack directory.
+        Can be called multiple times safely.
+        """
+        if self._temp_pack_dir is not None:
+            if os.path.exists(self._temp_pack_dir):
+                import shutil
 
 
-            shutil.rmtree(self._temp_pack_dir, ignore_errors=True)
+                shutil.rmtree(self._temp_pack_dir, ignore_errors=True)
+            self._temp_pack_dir = None
+
+    def __del__(self) -> None:
+        """Warn if the object store is being deleted without closing."""
+        if self._temp_pack_dir is not None:
+            import warnings
+
+            warnings.warn(
+                f"DumbHTTPObjectStore {self!r} was destroyed without calling close(). "
+                "Temporary pack directory may not be cleaned up properly.",
+                ResourceWarning,
+                stacklevel=2,
+            )
+            self.close()
 
 
 
 
 class DumbRemoteHTTPRepo:
 class DumbRemoteHTTPRepo:

+ 15 - 4
dulwich/object_store.py

@@ -847,7 +847,6 @@ class PackBasedObjectStore(PackCapableObjectStore, PackedObjectContainer):
         """
         """
         super().__init__(object_format=object_format)
         super().__init__(object_format=object_format)
         self._pack_cache: dict[str, Pack] = {}
         self._pack_cache: dict[str, Pack] = {}
-        self._closed = False
         self.pack_compression_level = pack_compression_level
         self.pack_compression_level = pack_compression_level
         self.pack_index_version = pack_index_version
         self.pack_index_version = pack_index_version
         self.pack_delta_window_size = pack_delta_window_size
         self.pack_delta_window_size = pack_delta_window_size
@@ -1011,15 +1010,26 @@ class PackBasedObjectStore(PackCapableObjectStore, PackedObjectContainer):
         """Close the object store and release resources.
         """Close the object store and release resources.
 
 
         This method closes all cached pack files and frees associated resources.
         This method closes all cached pack files and frees associated resources.
+        Can be called multiple times safely.
         """
         """
-        self._closed = True
         self._clear_cached_packs()
         self._clear_cached_packs()
 
 
+    def __del__(self) -> None:
+        """Warn if the object store is being deleted with unclosed packs."""
+        if self._pack_cache:
+            import warnings
+
+            warnings.warn(
+                f"ObjectStore {self!r} was destroyed with {len(self._pack_cache)} "
+                "unclosed pack(s). Please call close() explicitly.",
+                ResourceWarning,
+                stacklevel=2,
+            )
+            self.close()
+
     @property
     @property
     def packs(self) -> list[Pack]:
     def packs(self) -> list[Pack]:
         """List with pack objects."""
         """List with pack objects."""
-        if self._closed:
-            raise ValueError("Cannot access packs on a closed object store")
         return list(self._iter_cached_packs()) + list(self._update_pack_cache())
         return list(self._iter_cached_packs()) + list(self._update_pack_cache())
 
 
     def count_pack_files(self) -> int:
     def count_pack_files(self) -> int:
@@ -2437,6 +2447,7 @@ class DiskObjectStore(PackBasedObjectStore):
         """Close the object store and release resources.
         """Close the object store and release resources.
 
 
         This method closes all cached pack files, MIDX, and frees associated resources.
         This method closes all cached pack files, MIDX, and frees associated resources.
+        Can be called multiple times safely.
         """
         """
         # Close MIDX if it's loaded
         # Close MIDX if it's loaded
         if self._midx is not None:
         if self._midx is not None: