GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-81954: Warn when ZipFile is closed with unwritten data by cmaloney · Pull Request #152354 · python/cpython

Conversation

@cmaloney

@cmaloney cmaloney commented Jun 27, 2026 •
edited by bedevere-app Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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().

cc: @jaraco the _path/test_path fixtures have quite a few ZipFiles they don't close. For now I just blanket silenced, not sure your preferred upstream fix.

  • Issue: ValueError: I/O operation on closed file. in ZipFile destructor #81954

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@cmaloney
pythongh-81954: Warn when ZipFile is closed with unwritten data
5e16278 
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()`.
@cmaloney
cmaloney requested a review from jaraco as a code owner June 27, 2026 00:05
@bedevere-app bedevere-app Bot added the awaiting review label Jun 27, 2026
@bedevere-app bedevere-app Bot mentioned this pull request Jun 27, 2026
ValueError: I/O operation on closed file. in ZipFile destructor #81954
Closed
@read-the-docs-community

read-the-docs-community Bot commented Jun 27, 2026 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

Documentation build overview

📚 cpython-previews | 🛠️ Build #33863505 | 📁 Comparing 526059b against main (1812162)

  🔍 Preview build  

160 files changed · + 3 added · ± 156 modified · - 1 deleted

+ Added

  • library/asyncio-tools.html
  • library/tkinter.fontchooser.html
  • library/tkinter.systray.html

± Modified

  • glossary.html
  • license.html
  • py-modindex.html
  • c-api/bytes.html
  • c-api/dict.html
  • c-api/exceptions.html
  • c-api/float.html
  • c-api/frame.html
  • c-api/gen.html
  • c-api/init_config.html
  • and 146 more...

- Deleted

  • library/asyncio-policy.html
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
emmatyping
emmatyping reviewed Jul 5, 2026
View reviewed changes
Comment thread Lib/test/test_zipfile/_path/test_path.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
@cmaloney
Move zipfile.Path tests to enter/exit context managers
03007df
vstinner
vstinner reviewed Jul 7, 2026
View reviewed changes
Comment thread Lib/zipfile/__init__.py Outdated
# records. 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')
and self._didModify):

@vstinner vstinner Jul 7, 2026

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why this complex logic? open() logs a ResourceWarning even for files open in read-mode. A ZipFile contains a file, so it should emit a ResourceWarning if it's not closed, even if it's open in read-mode, or even if all data is flushed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@cmaloney cmaloney Jul 7, 2026

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reducing noisy cases that need to be updated for minimal value in an upgrade to 3.16. This parallels the GzipFile logic from gh-130806:

cpython/Lib/gzip.py

Lines 469 to 475 in 48cc257

def __del__(self):
if self.mode == WRITE and not self.closed:
import warnings
warnings.warn("unclosed GzipFile",
ResourceWarning, source=self, stacklevel=2)
super().__del__()

Dropped the self._didModify part (anything opened in writable mode should be reliably closed). The self.fp is not None is how ZipFile checks "has this been closed".

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
cmaloney and others added 6 commits July 7, 2026 09:57
@cmaloney
Simplify condition for warning
e39237d
@cmaloney
Simplify and update comments for simplified check
24cb932
@cmaloney
Merge branch 'main' into zipfile_resourcewarning
327992c
@cmaloney
Merge branch 'main' into zipfile_resourcewarning
a123a3e
@cmaloney
Use existing unittest exit stack
7242f19
@cmaloney
Remove unused import
526059b
vstinner
vstinner approved these changes Aug 7, 2026
View reviewed changes

@vstinner vstinner left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

I would prefer to always emit a ResourceWarning if the zipfile is not closed explicitly. Copying gzip behavior sounds like a reasonable option here. So I'm fine with only emitting a warning if the zipfile is open in write mode.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-app bedevere-app Bot added awaiting merge and removed awaiting review labels Aug 7, 2026
@vstinner
vstinner merged commit 5e0c502 into python:main Aug 7, 2026
57 checks passed

Uh oh!

There was an error while loading. Please reload this page.
@bedevere-app bedevere-app Bot removed the awaiting merge label Aug 7, 2026
@vstinner

vstinner commented Aug 7, 2026

Copy link
Copy Markdown
Member

I tried the original reproducer: #81954 (comment)

$ ./python x.py 
Exception ignored while calling deallocator <function ZipFile.__del__ at 0x2000335a110>:
Traceback (most recent call last):
  File "/home/vstinner/python/main/Lib/zipfile/__init__.py", line 2623, in __del__
ImportError: sys.meta_path is None, Python is likely shutting down

Sadly, the warnings module failed to be imported. I tried to move "lazy import warnings" to __del__() as "import warnings", but it doesn't change the output.

It works as expected if the zipfile object creation is moved into foo() function.

Well, IMO it's fine to leave the code as it is. We cannot do better, we want to import warnings lazily and it doesn't work anymore during Python shutdown.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@vstinner vstinner mentioned this pull request Aug 7, 2026
gh-81954: Use lazy import warnings in zipfile #155326
Merged
@vstinner

vstinner commented Aug 7, 2026

Copy link
Copy Markdown
Member

Oh, I noticed that there are "import warnings" in functions which are no longer needed since "lazy import warnings" was added. I wrote PR gh-155326 to remove these lines.
cmaloney reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@cmaloney
cmaloney deleted the zipfile_resourcewarning branch August 7, 2026 14:45
@cmaloney

cmaloney commented Aug 7, 2026

Copy link
Copy Markdown
Contributor Author

I noticed the now removable lazy imports as well when making this PR but felt like a sufficiently independent change that shouldn't do in the same commit. Thanks for doing the followup!
vstinner reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mbeijen pushed a commit to mbeijen/cpython that referenced this pull request Aug 14, 2026
@cmaloney @mbeijen
pythongh-81954: Warn when ZipFile is closed with unwritten data (pyth…
9d21558 
…on#152354)

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()`.
@jaraco jaraco mentioned this pull request Oct 21, 2026
Port test changes from CPython jaraco/zipp#165
Merged
jaraco added a commit to jaraco/zipp that referenced this pull request Oct 21, 2026
@jaraco
Close the zipfiles in test_at and test_str.
e18b2d0 
CPython now emits a ResourceWarning when a writable ZipFile is closed
implicitly (python/cpython#152354).
@jaraco jaraco mentioned this pull request Oct 21, 2026
gh-160104: Refresh zipfile.Path from zipp 4.1.2 #160105
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@emmatyping emmatyping emmatyping left review comments
@vstinner vstinner vstinner approved these changes
@jaraco jaraco Awaiting requested review from jaraco jaraco is a code owner

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

3 participants

@cmaloney @vstinner @emmatyping
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.