Repository navigation
importlib.resources.as_file is leaving temporary file pointers open #100585
Description
Bug report
importlib.resources.as_file is leaving temporary file pointers open after writing their contents
see _write_contents function in importlib/resources/_common.py
Easy to repeat, just run the test case below with -We
Lib.test.test_importlib.resources.test_resource.ResourceFromZipsTest01.test_as_file_directory
Your environment
- CPython versions tested on: Python 3.12.0a3+
- Operating system and architecture: Ubuntu 22.04.1 LTS
I think it just needs to use Path.write_bytes and keep the file pointer closed.
Activity
warsaw commentedon Dec 28, 2022
Just from a quick glance, I think you're right in that the code is relying on implicit GC to close the file, which generally isn't good practice. @jaraco to chime in. What I don't remember is whether its
resourcesormetadatathat @jaraco likes to fix upstream and then sync to CPython. (I haven't looked to see if the same code exists inimportlib_resources.)jaraco commentedon Dec 28, 2022
This issue is fixed in python/importlib_resources#274, which just hasn't been merged into CPython yet. All skeleton-based projects (including importlib_metadata) now enable ResourceWarnings so these kinds of issues will get caught early.
In that fix, a context manager was used instead of relying on
write_bytes. I do prefer this one-line approach withwrite_bytes.samety commentedon Dec 28, 2022
Yes I just saw the context manager fix, that's also addressing the issue indeed. I also like one line approach more, I find using
write_bytesa bit cleaner. I don't mind applying the same one line approach to the importlib_resources project if that's the preferred option. Please let me know if you prefer a PR to importlib_resources instead of this one.gh-100585: Fixed a bug where importlib.resources.as_file was leaving …
jaraco commentedon Dec 28, 2022
I cherry-picked the change into importlib_resources as python/importlib_resources@d623078. Thanks for reporting and the fix.