Skip to content

Commit 5e16278

Browse files
committed
gh-81954: Warn when ZipFile is closed with unwritten data
Bring `zipfile.ZipFile` into alignment with other buffered file writers like `GzipFile` and `TextIOWrapper` by emitting a `ResourceWarning` if there is unwritten data and it is closed implicitly. ZipFile should be closed either by using it as a context manager or explicitly calling `.close()`.
1 parent bbf7786 commit 5e16278

5 files changed

Lines changed: 50 additions & 5 deletions

File tree

Doc/library/zipfile.rst

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,6 +285,10 @@ ZipFile objects
285285
Added support for specifying member name encoding for reading
286286
metadata in the zipfile's directory and file headers.
287287

288+
.. versionchanged:: next
289+
Deleting a :class:`zipfile.ZipFile` which contains unwritten data before
290+
it is closed now emits a :exc:`ResourceWarning`. Use as a
291+
:term:`context manager` or call :meth:`~zipfile.ZipFile.close` explicitly.
288292

289293
.. method:: ZipFile.close()
290294

Lib/test/test_zipfile/_path/test_path.py

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import stat
77
import sys
88
import unittest
9+
import warnings
910
import zipfile
1011
import zipfile._path
1112

@@ -86,6 +87,12 @@ class TestPath(unittest.TestCase):
8687
def setUp(self):
8788
self.fixtures = contextlib.ExitStack()
8889
self.addCleanup(self.fixtures.close)
90+
# These tests use zipfiles as fixtures which do not get closed. Ignore
91+
# the ResourceWarning.
92+
self.fixtures.enter_context(warnings.catch_warnings())
93+
warnings.filterwarnings(
94+
'ignore', category=ResourceWarning, message='unclosed ZipFile'
95+
)
8996

9097
def zipfile_ondisk(self, alpharep):
9198
tmpdir = pathlib.Path(self.fixtures.enter_context(temp_dir()))

Lib/test/test_zipfile/test_core.py

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,13 @@
2525
from test.support import (
2626
findfile, requires_zlib, requires_bz2, requires_lzma,
2727
requires_zstd, captured_stdout, captured_stderr, requires_subprocess,
28-
cpython_only
28+
cpython_only, gc_collect
2929
)
3030
from test.support.os_helper import (
3131
TESTFN, unlink, rmtree, temp_dir, temp_cwd, fd_count, FakePath
3232
)
3333
from test.support.import_helper import ensure_lazy_imports
34+
from test.support.warnings_helper import check_no_resource_warning
3435

3536

3637
TESTFN2 = TESTFN + "2"
@@ -4051,6 +4052,28 @@ def test_close_on_exception(self):
40514052
except zipfile.BadZipFile:
40524053
self.assertIsNone(zipfp2.fp, 'zipfp is not closed')
40534054

4055+
def test_garbage_collection(self):
4056+
# gh-81954: Warn if a writable zipfile is closed by GC.
4057+
with self.assertWarns(ResourceWarning):
4058+
zipfile.ZipFile(io.BytesIO(), "w")
4059+
gc_collect()
4060+
4061+
# Only warn if there is possible data loss.
4062+
# Properly closed via context manager.
4063+
buf = io.BytesIO()
4064+
with zipfile.ZipFile(buf, "w") as zf:
4065+
zf.writestr("f.txt", b"data")
4066+
4067+
with check_no_resource_warning(self):
4068+
# Read mode: No possible data loss.
4069+
zipfile.ZipFile(buf, "r")
4070+
4071+
# Write with manual explicit close: No pending data.
4072+
zf = zipfile.ZipFile(io.BytesIO(), "w")
4073+
zf.writestr("f.txt", b"data")
4074+
zf.close()
4075+
del zf
4076+
40544077
def test_unsupported_version(self):
40554078
# File has an extract_version of 120
40564079
data = (b'PK\x03\x04x\x00\x00\x00\x00\x00!p\xa1@\x00\x00\x00\x00\x00\x00'
@@ -5503,10 +5526,10 @@ def test_root_folder_in_zipfile(self):
55035526
the zip file, this is a strange behavior, but we should support it.
55045527
"""
55055528
in_memory_file = io.BytesIO()
5506-
zf = zipfile.ZipFile(in_memory_file, "w")
5507-
zf.mkdir('/')
5508-
zf.writestr('./a.txt', 'aaa')
5509-
zf.extractall(TESTFN2)
5529+
with zipfile.ZipFile(in_memory_file, "w") as zf:
5530+
zf.mkdir('/')
5531+
zf.writestr('./a.txt', 'aaa')
5532+
zf.extractall(TESTFN2)
55105533

55115534
def tearDown(self):
55125535
rmtree(TESTFN2)

Lib/zipfile/__init__.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import sys
1313
import threading
1414
import time
15+
lazy import warnings
1516

1617
try:
1718
import zlib # We may need its compression method
@@ -2616,6 +2617,13 @@ def mkdir(self, zinfo_or_directory_name, mode=511):
26162617

26172618
def __del__(self):
26182619
"""Call the "close()" method in case the user forgot."""
2620+
# gh-81954: Warn if ZipFile is implicitly closed with unwritten end
2621+
# records. GC cleanup order is non-deterministic and can result in data
2622+
# loss.
2623+
if (self.fp is not None and self.mode in ('w', 'x', 'a')
2624+
and self._didModify):
2625+
warnings.warn(f"unclosed ZipFile {self!r}",
2626+
ResourceWarning, source=self, stacklevel=2)
26192627
self.close()
26202628

26212629
def close(self):
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Deleting a :class:`zipfile.ZipFile` which contains unwritten data before it
2+
is closed now emits a :exc:`ResourceWarning`. Use as a :term:`context manager`
3+
or call :meth:`~zipfile.ZipFile.close` explicitly.

0 commit comments

Comments
 (0)