From e48aea12ac001e2e33875f93a43c6fe465a05d2b Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Sun, 4 Oct 2026 19:28:56 -0700 Subject: [PATCH 1/8] Always close ZipFile in __del__ even if the warning fails ZipFile.__del__ emitted its ResourceWarning before calling close(). When warnings.warn() raised, close() was skipped and the central directory of a writable archive was never written, leaving a corrupt file. This happens at interpreter shutdown, where resolving the lazy "warnings" import raises ImportError, and with -W error::ResourceWarning at any time. Call close() in a finally block. --- Lib/test/test_zipfile/test_core.py | 25 ++++++++++++++++++++++++- Lib/zipfile/__init__.py | 10 ++++++---- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 708db4d4387df5e..10b3aad25fb9b41 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -26,7 +26,7 @@ from test.support import ( findfile, requires_zlib, requires_bz2, requires_lzma, requires_zstd, captured_stdout, captured_stderr, requires_subprocess, - cpython_only, gc_collect + cpython_only, gc_collect, catch_unraisable_exception ) from test.support.os_helper import ( TESTFN, unlink, rmtree, temp_dir, temp_cwd, fd_count, FakePath, @@ -4095,6 +4095,29 @@ def test_garbage_collection(self): zf.close() del zf + def test_garbage_collection_closes_when_warning_fails(self): + buf = io.BytesIO() + zf = zipfile.ZipFile(buf, "w") + zf.writestr("f.txt", b"data") + with warnings.catch_warnings(), catch_unraisable_exception() as cm: + warnings.simplefilter("error", ResourceWarning) + del zf + gc_collect() + self.assertIsInstance(cm.unraisable.exc_value, ResourceWarning) + with zipfile.ZipFile(buf) as zf: + self.assertEqual(zf.namelist(), ["f.txt"]) + + @requires_subprocess() + def test_unclosed_at_shutdown(self): + self.addCleanup(unlink, TESTFN) + script_helper.assert_python_ok("-c", f"""if 1: + import zipfile + zf = zipfile.ZipFile({TESTFN!r}, "w") + zf.writestr("f.txt", b"data") + """) + with zipfile.ZipFile(TESTFN) as zf: + self.assertEqual(zf.namelist(), ["f.txt"]) + def test_unsupported_version(self): # File has an extract_version of 120 data = (b'PK\x03\x04x\x00\x00\x00\x00\x00!p\xa1@\x00\x00\x00\x00\x00\x00' diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index d817bdd8769e7d7..debfbaabe34a009 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -2650,10 +2650,12 @@ def __del__(self): """Call the "close()" method in case the user forgot.""" # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. - if self.fp is not None and self.mode in ('w', 'x', 'a'): - warnings.warn(f"unclosed ZipFile {self!r}", - ResourceWarning, source=self, stacklevel=2) - self.close() + try: + if self.fp is not None and self.mode in ('w', 'x', 'a'): + warnings.warn(f"unclosed ZipFile {self!r}", + ResourceWarning, source=self, stacklevel=2) + finally: + self.close() def close(self): """Close the file, and for mode 'w', 'x' and 'a' write the ending From 29ef75407d7691e0d729913c5172924c6f440547 Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Wed, 14 Oct 2026 07:48:14 -0700 Subject: [PATCH 2/8] Add NEWS entry --- .../next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst | 3 +++ 1 file changed, 3 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst diff --git a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst new file mode 100644 index 000000000000000..fddb9eabe96c2a1 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst @@ -0,0 +1,3 @@ +Fix data loss when a writable :class:`zipfile.ZipFile` that was never closed +is finalized during interpreter shutdown: :meth:`~zipfile.ZipFile.close` is +now called even if emitting the :exc:`ResourceWarning` fails. From 9c41b9519cf7bda2b6e81c7863e9dc725f27bf12 Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Wed, 14 Oct 2026 14:42:47 -0700 Subject: [PATCH 3/8] Explain why ZipFile.__del__ wraps the warning in try/finally --- Lib/zipfile/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index debfbaabe34a009..70346cf2bffc8cc 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -2651,6 +2651,8 @@ def __del__(self): # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. try: + # Wrap within a try to ensure the file is closed even if the + # warnings module is unable to load during finalization. if self.fp is not None and self.mode in ('w', 'x', 'a'): warnings.warn(f"unclosed ZipFile {self!r}", ResourceWarning, source=self, stacklevel=2) From 48693fb48e196d7a5b1ac3b5b57dde979fbfbc90 Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Thu, 15 Oct 2026 15:02:10 -0700 Subject: [PATCH 4/8] Import warnings eagerly in zipfile and simplify the tests --- Lib/test/test_zipfile/test_core.py | 25 ++++--------------- Lib/zipfile/__init__.py | 14 ++++------- ...6-10-14-14-47-05.gh-issue-81954.KvM0gY.rst | 7 +++--- 3 files changed, 14 insertions(+), 32 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 10b3aad25fb9b41..037433df2cc4534 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -4095,28 +4095,13 @@ def test_garbage_collection(self): zf.close() del zf - def test_garbage_collection_closes_when_warning_fails(self): - buf = io.BytesIO() - zf = zipfile.ZipFile(buf, "w") - zf.writestr("f.txt", b"data") - with warnings.catch_warnings(), catch_unraisable_exception() as cm: - warnings.simplefilter("error", ResourceWarning) + # ResourceWarning escalated to an error is reported as unraisable. + zf = zipfile.ZipFile(io.BytesIO(), "w") + with (warnings.catch_warnings(action="error", category=ResourceWarning), + catch_unraisable_exception() as cm): del zf gc_collect() - self.assertIsInstance(cm.unraisable.exc_value, ResourceWarning) - with zipfile.ZipFile(buf) as zf: - self.assertEqual(zf.namelist(), ["f.txt"]) - - @requires_subprocess() - def test_unclosed_at_shutdown(self): - self.addCleanup(unlink, TESTFN) - script_helper.assert_python_ok("-c", f"""if 1: - import zipfile - zf = zipfile.ZipFile({TESTFN!r}, "w") - zf.writestr("f.txt", b"data") - """) - with zipfile.ZipFile(TESTFN) as zf: - self.assertEqual(zf.namelist(), ["f.txt"]) + self.assertIs(cm.unraisable.exc_type, ResourceWarning) def test_unsupported_version(self): # File has an extract_version of 120 diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index 70346cf2bffc8cc..ff2e3ed438fc5b0 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -12,7 +12,7 @@ import sys import threading import time -lazy import warnings +import warnings # must stay eager to support finalization try: import zlib # We may need its compression method @@ -2650,14 +2650,10 @@ def __del__(self): """Call the "close()" method in case the user forgot.""" # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. - try: - # Wrap within a try to ensure the file is closed even if the - # warnings module is unable to load during finalization. - if self.fp is not None and self.mode in ('w', 'x', 'a'): - warnings.warn(f"unclosed ZipFile {self!r}", - ResourceWarning, source=self, stacklevel=2) - finally: - self.close() + if self.fp is not None and self.mode in ('w', 'x', 'a'): + warnings.warn(f"unclosed ZipFile {self!r}", + ResourceWarning, source=self, stacklevel=2) + self.close() def close(self): """Close the file, and for mode 'w', 'x' and 'a' write the ending diff --git a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst index fddb9eabe96c2a1..b8f4693029b488d 100644 --- a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst +++ b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst @@ -1,3 +1,4 @@ -Fix data loss when a writable :class:`zipfile.ZipFile` that was never closed -is finalized during interpreter shutdown: :meth:`~zipfile.ZipFile.close` is -now called even if emitting the :exc:`ResourceWarning` fails. +:mod:`zipfile` now imports :mod:`warnings` eagerly so that a writable +:class:`~zipfile.ZipFile` left unclosed at interpreter shutdown can emit its +:exc:`ResourceWarning` and be closed instead of failing with an +:exc:`ImportError`. From 251fa94e6d662410c2b08bd39c23eeec50e3a809 Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Fri, 16 Oct 2026 13:55:02 -0700 Subject: [PATCH 5/8] Restore try/finally in ZipFile.__del__ and check the archive in the test Keep a reference to warnings.warn as a default argument, like subprocess.Popen.__del__, and always call close() even if emitting the ResourceWarning raises. Verify the archive contents after collection so the test catches a skipped close(). Shorten the NEWS entry. --- Lib/test/test_zipfile/test_core.py | 11 +++++++++-- Lib/zipfile/__init__.py | 12 +++++++----- .../2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst | 6 ++---- 3 files changed, 18 insertions(+), 11 deletions(-) diff --git a/Lib/test/test_zipfile/test_core.py b/Lib/test/test_zipfile/test_core.py index 037433df2cc4534..cb9061042937cf9 100644 --- a/Lib/test/test_zipfile/test_core.py +++ b/Lib/test/test_zipfile/test_core.py @@ -4095,14 +4095,21 @@ def test_garbage_collection(self): zf.close() del zf - # ResourceWarning escalated to an error is reported as unraisable. - zf = zipfile.ZipFile(io.BytesIO(), "w") + # ResourceWarning escalated to an error is reported as unraisable, + # and the archive is still closed. + buf = io.BytesIO() + zf = zipfile.ZipFile(buf, "w") + zf.writestr("f.txt", b"data") with (warnings.catch_warnings(action="error", category=ResourceWarning), catch_unraisable_exception() as cm): del zf gc_collect() self.assertIs(cm.unraisable.exc_type, ResourceWarning) + with zipfile.ZipFile(buf, "r") as zf: + self.assertEqual(zf.namelist(), ["f.txt"]) + self.assertEqual(zf.read("f.txt"), b"data") + def test_unsupported_version(self): # File has an extract_version of 120 data = (b'PK\x03\x04x\x00\x00\x00\x00\x00!p\xa1@\x00\x00\x00\x00\x00\x00' diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index ff2e3ed438fc5b0..fb37282e1e12d98 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -2646,14 +2646,16 @@ def mkdir(self, zinfo_or_directory_name, mode=511): self.fp.write(zinfo.FileHeader(False)) self.start_dir = self.fp.tell() - def __del__(self): + def __del__(self, _warn=warnings.warn): """Call the "close()" method in case the user forgot.""" # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. - if self.fp is not None and self.mode in ('w', 'x', 'a'): - warnings.warn(f"unclosed ZipFile {self!r}", - ResourceWarning, source=self, stacklevel=2) - self.close() + try: + if self.fp is not None and self.mode in ('w', 'x', 'a'): + _warn(f"unclosed ZipFile {self!r}", + ResourceWarning, source=self, stacklevel=2) + finally: + self.close() def close(self): """Close the file, and for mode 'w', 'x' and 'a' write the ending diff --git a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst index b8f4693029b488d..2e9b96cf10621e5 100644 --- a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst +++ b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst @@ -1,4 +1,2 @@ -:mod:`zipfile` now imports :mod:`warnings` eagerly so that a writable -:class:`~zipfile.ZipFile` left unclosed at interpreter shutdown can emit its -:exc:`ResourceWarning` and be closed instead of failing with an -:exc:`ImportError`. +Garbage collecting an unclosed writable :class:`zipfile.ZipFile` now closes it +even if emitting the :exc:`ResourceWarning` raises an exception. From 64af802f3aaf6e0f469c4306ebac358df1727749 Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Mon, 19 Oct 2026 08:32:56 -0700 Subject: [PATCH 6/8] Explain why ZipFile.__del__ wraps the warning in try/finally --- Lib/zipfile/__init__.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index fb37282e1e12d98..d0b9e0dbaa18393 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -2651,6 +2651,8 @@ def __del__(self, _warn=warnings.warn): # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. try: + # Wrap within a try to ensure the file is closed even if the + # warning raises, e.g. when ResourceWarning is made an error. if self.fp is not None and self.mode in ('w', 'x', 'a'): _warn(f"unclosed ZipFile {self!r}", ResourceWarning, source=self, stacklevel=2) From 728e7fe2d34a793ff867b6b8374e90d920d17bdc Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Mon, 19 Oct 2026 08:34:14 -0700 Subject: [PATCH 7/8] Reword NEWS entry to describe the shutdown ImportError --- .../Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst index 2e9b96cf10621e5..5ced3b94d223b25 100644 --- a/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst +++ b/Misc/NEWS.d/next/Library/2026-10-14-14-47-05.gh-issue-81954.KvM0gY.rst @@ -1,2 +1,2 @@ -Garbage collecting an unclosed writable :class:`zipfile.ZipFile` now closes it -even if emitting the :exc:`ResourceWarning` raises an exception. +Fix :class:`zipfile.ZipFile` not being closed at interpreter shutdown when +emitting its :exc:`ResourceWarning` failed with an :exc:`ImportError`. From 5e6d164c590f8c26fb0dee071759da45ccdd120b Mon Sep 17 00:00:00 2001 From: Brittany Reynoso Date: Mon, 19 Oct 2026 08:47:07 -0700 Subject: [PATCH 8/8] Simplify comment in ZipFile.__del__ --- Lib/zipfile/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/zipfile/__init__.py b/Lib/zipfile/__init__.py index d0b9e0dbaa18393..c0923cb5c92f232 100644 --- a/Lib/zipfile/__init__.py +++ b/Lib/zipfile/__init__.py @@ -2651,8 +2651,8 @@ def __del__(self, _warn=warnings.warn): # gh-81954: Warn if writable ZipFile is implicitly closed. # GC cleanup order is non-deterministic and can result in data loss. try: - # Wrap within a try to ensure the file is closed even if the - # warning raises, e.g. when ResourceWarning is made an error. + # Wrap within a try to ensure the file is closed even if + # warnings raises. if self.fp is not None and self.mode in ('w', 'x', 'a'): _warn(f"unclosed ZipFile {self!r}", ResourceWarning, source=self, stacklevel=2)