From cd3b196382b3849391f8b1b556683ff15ff8e0b9 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Tue, 13 May 2025 11:16:13 -0700 Subject: [PATCH 1/2] gh-133982: Run unclosed file test on all io implementations Update `test_io` `_check_warn_on_dealloc` to use `self.` to dispatch to different I/O implementations. Update the `_pyio` implementation to match expected behavior, using the same `_dealloc_warn` design as the C implementation uses to report the topmost `__del__` object. --- Lib/_pyio.py | 19 +++++++++++++++++-- Lib/test/test_io.py | 4 ++-- ...-05-17-20-23-57.gh-issue-133982.smS7au.rst | 3 +++ 3 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-05-17-20-23-57.gh-issue-133982.smS7au.rst diff --git a/Lib/_pyio.py b/Lib/_pyio.py index a870de5b532542..f2298bcac2f814 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -407,6 +407,9 @@ def __del__(self): if closed: return + if dealloc_warn := getattr(self, "_dealloc_warn", None): + dealloc_warn(self) + # If close() fails, the caller logs the exception with # sys.unraisablehook. close() must be called at the end at __del__(). self.close() @@ -853,6 +856,10 @@ def __repr__(self): else: return "<{}.{} name={!r}>".format(modname, clsname, name) + def _dealloc_warn(self, source): + if dealloc_warn := getattr(self.raw, "_dealloc_warn", None): + dealloc_warn(source) + ### Lower-level APIs ### def fileno(self): @@ -1600,11 +1607,15 @@ def __init__(self, file, mode='r', closefd=True, opener=None): raise self._fd = fd - def __del__(self): + def _dealloc_warn(self, source): if self._fd >= 0 and self._closefd and not self.closed: import warnings - warnings.warn('unclosed file %r' % (self,), ResourceWarning, + warnings.warn(f'unclosed file {source!r}', ResourceWarning, stacklevel=2, source=self) + + def __del__(self): + if self._fd >= 0 and self._closefd and not self.closed: + self._dealloc_warn(self) self.close() def __getstate__(self): @@ -2689,6 +2700,10 @@ def readline(self, size=None): def newlines(self): return self._decoder.newlines if self._decoder else None + def _dealloc_warn(self, source): + if dealloc_warn := getattr(self.buffer, "_dealloc_warn", None): + dealloc_warn(source) + class StringIO(TextIOWrapper): """Text I/O implementation using an in-memory buffer. diff --git a/Lib/test/test_io.py b/Lib/test/test_io.py index 5a8f1949baaa98..bddea894438ff8 100644 --- a/Lib/test/test_io.py +++ b/Lib/test/test_io.py @@ -4417,7 +4417,7 @@ def test_abc_inheritance_official(self): self._check_abc_inheritance(io) def _check_warn_on_dealloc(self, *args, **kwargs): - f = open(*args, **kwargs) + f = self.open(*args, **kwargs) r = repr(f) with self.assertWarns(ResourceWarning) as cm: f = None @@ -4446,7 +4446,7 @@ def cleanup_fds(): r, w = os.pipe() fds += r, w with warnings_helper.check_no_resource_warning(self): - open(r, *args, closefd=False, **kwargs) + self.open(r, *args, closefd=False, **kwargs) @unittest.skipUnless(hasattr(os, "pipe"), "requires os.pipe()") def test_warn_on_dealloc_fd(self): diff --git a/Misc/NEWS.d/next/Library/2025-05-17-20-23-57.gh-issue-133982.smS7au.rst b/Misc/NEWS.d/next/Library/2025-05-17-20-23-57.gh-issue-133982.smS7au.rst new file mode 100644 index 00000000000000..a6753145981181 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-05-17-20-23-57.gh-issue-133982.smS7au.rst @@ -0,0 +1,3 @@ +Emit :exc:`RuntimeWarning` in the Python implementation of :mod:`io` when +the :term:`file-like object ` is not closed explicitly in the +presence of multiple I/O layers. From 704bf460f2891d02f102238a6b27e3c1e7e40060 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Sun, 18 May 2025 10:50:27 -0400 Subject: [PATCH 2/2] Rely on IOBase __del__ implementation in FileIO The FileIO one now matches all the others, so can use IOBase. There was a missing check on closing (self._fd must be valid), add that check --- Lib/_pyio.py | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/Lib/_pyio.py b/Lib/_pyio.py index f2298bcac2f814..7bb0d522df4fdb 100644 --- a/Lib/_pyio.py +++ b/Lib/_pyio.py @@ -1613,11 +1613,6 @@ def _dealloc_warn(self, source): warnings.warn(f'unclosed file {source!r}', ResourceWarning, stacklevel=2, source=self) - def __del__(self): - if self._fd >= 0 and self._closefd and not self.closed: - self._dealloc_warn(self) - self.close() - def __getstate__(self): raise TypeError(f"cannot pickle {self.__class__.__name__!r} object") @@ -1791,7 +1786,7 @@ def close(self): if not self.closed: self._stat_atopen = None try: - if self._closefd: + if self._closefd and self._fd >= 0: os.close(self._fd) finally: super().close()