From 8bf189556a45f7f5720445a6dcbf0c2acffa4a69 Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Wed, 5 Mar 2025 20:10:37 -0800 Subject: [PATCH 1/2] gh-130806: emit ResourceWarning if GzipFile unclosed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This may indicate accidental data loss. Ways to make sure all data is written: 1. Use the [file-like object](https://docs.python.org/3/glossary.html#term-file-object) as a [“With Statement Context Manager”](https://docs.python.org/3/reference/datamodel.html#context-managers). - All objects which [inherit](https://docs.python.org/3/tutorial/classes.html#inheritance) from [IOBase](https://docs.python.org/3/library/io.html#io.IOBase) support this. - [`BufferedIOBase`](https://docs.python.org/3/library/io.html#io.BufferedIOBase), [`BufferedWriter`](https://docs.python.org/3/library/io.html#io.BufferedWriter), and [`GzipFile`](https://docs.python.org/3/library/gzip.html#gzip.GzipFile) all support this. - Ensures `.close()` is called in both exception and regular cases. 1. Ensure [`.close()`](https://docs.python.org/3/library/io.html#io.IOBase.close) is always called which flushes data before closing. 1. If the underlying stream need to be kept open, use [`.detach()`](https://docs.python.org/3/library/io.html#io.BufferedIOBase.detach) Since 3.12 flushing has been necessary in GzipFile (see gh-105808 which was a release blocker), this makes that more visible. Users have been encountering as they upgrade to 3.12 (ex. gh-129726). There are a number of cases of unclosed file-like objects being deleted in CPython libraries and the test suite. This issue includes resolving those cases where the new ResourceWarning is emitted. --- Lib/gzip.py | 14 +++++++++++++- Lib/test/test_gzip.py | 8 +++++--- .../2025-03-05-20-02-21.gh-issue-130806.o0l2FJ.rst | 2 ++ 3 files changed, 20 insertions(+), 4 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-03-05-20-02-21.gh-issue-130806.o0l2FJ.rst diff --git a/Lib/gzip.py b/Lib/gzip.py index 7e384f8a568c1c..576267cd9df686 100644 --- a/Lib/gzip.py +++ b/Lib/gzip.py @@ -193,6 +193,11 @@ def __init__(self, filename=None, mode=None, """ + # Ensure attributes exist at __del__ + self.mode = None + self.fileobj = None + self._buffer = None + if mode and ('t' in mode or 'U' in mode): raise ValueError("Invalid mode: {!r}".format(mode)) if mode and 'b' not in mode: @@ -358,7 +363,7 @@ def closed(self): def close(self): fileobj = self.fileobj - if fileobj is None or self._buffer.closed: + if fileobj is None or self._buffer is None or self._buffer.closed: return try: if self.mode == WRITE: @@ -435,6 +440,13 @@ def readline(self, size=-1): self._check_not_closed() return self._buffer.readline(size) + def __del__(self): + if self.mode == WRITE and not self.closed: + import warnings + warnings.warn("unclosed GzipFile", + ResourceWarning, source=self, stacklevel=2) + + return super().__del__() def _read_exact(fp, n): '''Read exactly *n* bytes from `fp` diff --git a/Lib/test/test_gzip.py b/Lib/test/test_gzip.py index 0940bb114df625..7047220900f827 100644 --- a/Lib/test/test_gzip.py +++ b/Lib/test/test_gzip.py @@ -9,6 +9,7 @@ import struct import sys import unittest +import warnings from subprocess import PIPE, Popen from test.support import catch_unraisable_exception from test.support import import_helper @@ -867,9 +868,10 @@ def test_refloop_unraisable(self): # fileobj would be closed before the GzipFile as the result of a # reference loop. See issue gh-129726 with catch_unraisable_exception() as cm: - gzip.GzipFile(fileobj=io.BytesIO(), mode="w") - gc.collect() - self.assertIsNone(cm.unraisable) + with self.assertWarns(ResourceWarning): + gzip.GzipFile(fileobj=io.BytesIO(), mode="w") + gc.collect() + self.assertIsNone(cm.unraisable) class TestOpen(BaseTest): diff --git a/Misc/NEWS.d/next/Library/2025-03-05-20-02-21.gh-issue-130806.o0l2FJ.rst b/Misc/NEWS.d/next/Library/2025-03-05-20-02-21.gh-issue-130806.o0l2FJ.rst new file mode 100644 index 00000000000000..37c3d12549eb91 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-03-05-20-02-21.gh-issue-130806.o0l2FJ.rst @@ -0,0 +1,2 @@ +Deleting :class:`gzip.GzipFile` before it is closed now emits a +:exc:`ResourceWarning`. From da8a02111bfb96775845199f855c2337290767fa Mon Sep 17 00:00:00 2001 From: Cody Maloney Date: Thu, 6 Mar 2025 13:18:09 -0800 Subject: [PATCH 2/2] Apply suggestions from code review Co-authored-by: Victor Stinner --- Lib/gzip.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/Lib/gzip.py b/Lib/gzip.py index 576267cd9df686..91632b1c05216c 100644 --- a/Lib/gzip.py +++ b/Lib/gzip.py @@ -363,7 +363,9 @@ def closed(self): def close(self): fileobj = self.fileobj - if fileobj is None or self._buffer is None or self._buffer.closed: + if fileobj is None: + return + if self._buffer is None or self._buffer.closed: return try: if self.mode == WRITE: @@ -446,7 +448,7 @@ def __del__(self): warnings.warn("unclosed GzipFile", ResourceWarning, source=self, stacklevel=2) - return super().__del__() + super().__del__() def _read_exact(fp, n): '''Read exactly *n* bytes from `fp`