bpo-41490: ``path`` and ``contents`` to aggressively close handles (#22915)
* bpo-41490: ``path`` method to aggressively close handles
* Add blurb
* In ZipReader.contents, eagerly evaluate the contents to release references to the zipfile.
* Instead use _ensure_sequence to ensure any iterable from a reader is eagerly converted to a list if it's not already a sequence.
diff --git a/Lib/importlib/_common.py b/Lib/importlib/_common.py
index b15c59e..71ce6af 100644
--- a/Lib/importlib/_common.py
+++ b/Lib/importlib/_common.py
@@ -88,6 +88,7 @@ def _tempfile(reader, suffix=''):
try:
os.write(fd, reader())
os.close(fd)
+ del reader
yield pathlib.Path(raw_path)
finally:
try:
@@ -97,14 +98,12 @@ def _tempfile(reader, suffix=''):
@functools.singledispatch
-@contextlib.contextmanager
def as_file(path):
"""
Given a Traversable object, return that object as a
path on the local file system in a context manager.
"""
- with _tempfile(path.read_bytes, suffix=path.name) as local:
- yield local
+ return _tempfile(path.read_bytes, suffix=path.name)
@as_file.register(pathlib.Path)
diff --git a/Lib/importlib/readers.py b/Lib/importlib/readers.py
index 6331e4d..74a63e4 100644
--- a/Lib/importlib/readers.py
+++ b/Lib/importlib/readers.py
@@ -22,8 +22,8 @@ def files(self):
class ZipReader(abc.TraversableResources):
def __init__(self, loader, module):
_, _, name = module.rpartition('.')
- prefix = loader.prefix.replace('\\', '/') + name + '/'
- self.path = zipfile.Path(loader.archive, prefix)
+ self.prefix = loader.prefix.replace('\\', '/') + name + '/'
+ self.archive = loader.archive
def open_resource(self, resource):
try:
@@ -38,4 +38,4 @@ def is_resource(self, path):
return target.is_file() and target.exists()
def files(self):
- return self.path
+ return zipfile.Path(self.archive, self.prefix)
diff --git a/Lib/importlib/resources.py b/Lib/importlib/resources.py
index 4535619..4169171 100644
--- a/Lib/importlib/resources.py
+++ b/Lib/importlib/resources.py
@@ -1,8 +1,9 @@
import os
+import io
from . import _common
from ._common import as_file, files
-from contextlib import contextmanager, suppress
+from contextlib import suppress
from importlib.abc import ResourceLoader
from io import BytesIO, TextIOWrapper
from pathlib import Path
@@ -10,6 +11,8 @@
from typing import ContextManager, Iterable, Union
from typing import cast
from typing.io import BinaryIO, TextIO
+from collections.abc import Sequence
+from functools import singledispatch
__all__ = [
@@ -102,22 +105,26 @@ def path(
"""
reader = _common.get_resource_reader(_common.get_package(package))
return (
- _path_from_reader(reader, resource)
+ _path_from_reader(reader, _common.normalize_path(resource))
if reader else
_common.as_file(
_common.files(package).joinpath(_common.normalize_path(resource)))
)
-@contextmanager
def _path_from_reader(reader, resource):
- norm_resource = _common.normalize_path(resource)
+ return _path_from_resource_path(reader, resource) or \
+ _path_from_open_resource(reader, resource)
+
+
+def _path_from_resource_path(reader, resource):
with suppress(FileNotFoundError):
- yield Path(reader.resource_path(norm_resource))
- return
- opener_reader = reader.open_resource(norm_resource)
- with _common._tempfile(opener_reader.read, suffix=norm_resource) as res:
- yield res
+ return Path(reader.resource_path(resource))
+
+
+def _path_from_open_resource(reader, resource):
+ saved = io.BytesIO(reader.open_resource(resource).read())
+ return _common._tempfile(saved.read, suffix=resource)
def is_resource(package: Package, name: str) -> bool:
@@ -146,7 +153,7 @@ def contents(package: Package) -> Iterable[str]:
package = _common.get_package(package)
reader = _common.get_resource_reader(package)
if reader is not None:
- return reader.contents()
+ return _ensure_sequence(reader.contents())
# Is the package a namespace package? By definition, namespace packages
# cannot have resources.
namespace = (
@@ -156,3 +163,13 @@ def contents(package: Package) -> Iterable[str]:
if namespace or not package.__spec__.has_location:
return ()
return list(item.name for item in _common.from_package(package).iterdir())
+
+
+@singledispatch
+def _ensure_sequence(iterable):
+ return list(iterable)
+
+
+@_ensure_sequence.register(Sequence)
+def _(iterable):
+ return iterable