ValueError on ephemeral zipfile after exception

[jaraco]

Consider this repro:

 draft @ cat zip-tb.txt
>>> import zipfile
>>> import io
>>> zf = zipfile.ZipFile(io.BytesIO(), 'w')
>>> zf.foo  # doctest: +ELLIPSIS
Traceback (most recent call last):
...
AttributeError: ...

 draft @ py -3.12 -m doctest zip-tb.txt
Exception ignored in: <function ZipFile.__del__ at 0x100f36b60>
Traceback (most recent call last):
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/zipfile/__init__.py", line 1916, in __del__
    self.close()
  File "/Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/zipfile/__init__.py", line 1933, in close
    self.fp.seek(self.start_dir)
ValueError: I/O operation on closed file.

The test runs fine, but triggers a ValueError atexit.

The issue only arises if the last expression in the doctest triggered a traceback and the zipfile was present in the traceback. Changing zf.foo to io.foo or adding >>> pass bypasses the error.

I suspect there's a flaw in doctest that leaves resources unclosed if the last expression is handling a traceback.

[psvi]

This isn't a flaw in doctest.
Let's consider the following script:

# zip-tb.py

import zipfile
import io
import sys

def foo():
    zf = zipfile.ZipFile(io.BytesIO(), 'w')
    try:
        zf.foo
    except:
        exception = sys.exc_info()

if __name__ == '__main__':
    foo()

It raises the same exception:

$ python3 zip-tb.py
Exception ignored in: <function ZipFile.__del__ at 0x7fbc3b4484a0>
Traceback (most recent call last):
  File "/usr/local/lib/python3.11/zipfile.py", line 1874, in __del__
    self.close()
  File "/usr/local/lib/python3.11/zipfile.py", line 1891, in close
    self.fp.seek(self.start_dir)
ValueError: I/O operation on closed file.

There seems to be a bug in the garbage collector. The BytesIO object is removed before the ZipFile object.
The bug affects versions starting from 3.8.17.

[thatch]

Does this work better when using the ZipFile as a context manager?

[duaneg]

Assigning the exception to a local variable creates a reference cycle, which means the objects' finalizers are called in essentially arbitrary order. Code that relies on objects' finalizer ordering must ensure they are not involved in cycles. However, relying on finalizers is (AIUI) generally considered a bad idea if it can possibly be avoided.

As

    with io.BytesIO() as bs:
         with zipfile.ZipFile(bs, 'w') as zf:
             ...

That ensures everything is cleaned up correctly, without relying on GC behaviour.

[jaraco]

Perhaps ZipFile should be more resilient to the inner file object having been closed?

The bug affects versions starting from 3.8.17.

Does that imply that the behavior does not occur on Python 3.8.16 or 3.7, or are you saying you tested to Pythons as old as 3.8 (and happened to test on 3.8.17)?

[duaneg]

Perhaps ZipFile should be more resilient to the inner file object having been closed?

We can't just ignore errors from the underlying file: if it has been closed prior to the zip file that is a serious bug. It is the user's responsibility to ensure this cannot happen, whether through context managers or more esoteric means. We must raise the error or risk silent data loss and corruption!