New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Use of zipfile.Path causes attempt to write after ZipFile is closed #85522
Comments
In both Python 3.8.3 and 3.9.0b3, using zipfile.Path to write a file in a context manager results in an attempt to write to the zip file after it is closed. In Python 3.9.0b3: import io
from zipfile import ZipFile, Path
def make_zip():
"""Make zip file and return bytes."""
bytes_io = io.BytesIO()
zip_file = ZipFile(bytes_io, mode="w")
zip_path = Path(zip_file, "file-a")
# use zipp.Path.open
with zip_path.open(mode="wb") as fp:
fp.write(b"contents of file-a")
zip_file.close()
data = bytes_io.getvalue()
bytes_io.close()
return data
zip_data = make_zip()
# Exception ignored in: <function ZipFile.__del__ at 0x10aceff70>
# Traceback (most recent call last):
# File "/Users/nick/.pyenv/versions/3.9.0b3/lib/python3.9/zipfile.py", line 1807, in __del__
# self.close()
# File "/Users/nick/.pyenv/versions/3.9.0b3/lib/python3.9/zipfile.py", line 1824, in close
# self.fp.seek(self.start_dir)
# ValueError: I/O operation on closed file. In Python 3.8.3: import io
from zipfile import ZipFile, Path
def make_zip():
"""Make zip file and return bytes."""
bytes_io = io.BytesIO()
zip_file = ZipFile(bytes_io, mode="w")
zip_path = Path(zip_file, "file-a")
# use zipp.Path.open
with zip_path.open(mode="w") as fp:
fp.write(b"contents of file-a")
zip_file.close()
data = bytes_io.getvalue()
bytes_io.close()
return data
zip_data = make_zip()
# Exception ignored in: <function ZipFile.__del__ at 0x10922e670>
# Traceback (most recent call last):
# File "/Users/nick/.pyenv/versions/3.8.3/lib/python3.8/zipfile.py", line 1820, in __del__
# self.close()
# File "/Users/nick/.pyenv/versions/3.8.3/lib/python3.8/zipfile.py", line 1837, in close
# self.fp.seek(self.start_dir)
# ValueError: I/O operation on closed file. In the Python 3.8 example, mode="w" is used in the open method on zip_path. |
It looks like a copy of the zip_file object is getting created, probably by the Path constructor: zip_path = Path(zip_file, "file-a") When return is invoked, the copy still has a reference to the now closed bytes_io object which causes the ValueError exception when ZipFile.__del__() calls ZipFile.close(). This is definitely a bug. I'll take a crack at fixing it. |
I created a simpler test case that exercises the buggy code. The problem can be reproduced by passing ZipFile.__init__() a file-like object with the write flag (mode "w") and then closing the file-like object before calling ZipFile.close(). The exception being triggered by the call to ZipFile.__del__() is a side-effect because all ZipFile.__del__() does is call ZipFile.close(). import io
from zipfile import ZipFile
bytes_io = io.BytesIO()
zip_file = ZipFile(bytes_io, mode="w")
bytes_io.close()
zip_file.close() # throws ValueError |
Hi all, I have a working patch for this. Could I submit it for review ? |
I'm a little surprised from Nick's original post that the behavior is triggered. After all, the code already has a protection for a previously-closed zipfile and that value is unconditionally set during close. So how is it that Zipfile.__del__ is being called? Jeffrey suggests that
And indeed, I can confirm the ZipFile state is copied into a new object here. The FastLookup and CompleteDirs functionality is for performance optimizations.
Although the simpler test does trigger the error, I'd argue that the error is expected in this case and that the zip file will be invalid because I expected this example to more accurately capture the failure:
But it does not. I'll need to do more investigation. One option here is for Another approach could be to try to use an adapter pattern to adapt the original ZipFile rather than clone it into a subclass. |
This routine will repro the issue without relying on garbage collection to trigger the error:
|
Not sure if it's related to this issue there is an existing issue with ZipFile.__del__ : bpo-37773 |
Looks like a duplicate of https://bugs.python.org/issue40564 , which was fixed and closed so this can also be closed. |
Thanks Andrei |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: