X Tutup
The Wayback Machine - https://web.archive.org/web/20240503025726/https://github.com/python/cpython/issues/100585
Skip to content
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.

Sign up for GitHub

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

Jump to bottom

importlib.resources.as_file is leaving temporary file pointers open #100585

Closed
samety opened this issue Dec 28, 2022 · 4 comments
Closed

importlib.resources.as_file is leaving temporary file pointers open #100585

samety opened this issue Dec 28, 2022 · 4 comments
Labels
topic-importlib type-bug An unexpected behavior, bug, or error

Comments

@samety
Copy link
Contributor

samety commented Dec 28, 2022 •
edited by bedevere-bot

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.

Linked PRs
@samety samety added the type-bug An unexpected behavior, bug, or error label Dec 28, 2022
samety added a commit to samety/cpython that referenced this issue Dec 28, 2022
@samety
pythongh-100585: Fixed open fp bug in the imporlib module
0a2dc2b
samety added a commit to samety/cpython that referenced this issue Dec 28, 2022
@samety
Added news for pythongh-100585
938692f
@samety samety mentioned this issue Dec 28, 2022
Merged
@warsaw
Copy link
Member

warsaw commented 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 resources or metadata that @jaraco likes to fix upstream and then sync to CPython. (I haven't looked to see if the same code exists in importlib_resources.)

@jaraco
Copy link
Member

jaraco commented 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 with write_bytes.

@samety
Copy link
Contributor Author

samety commented Dec 28, 2022 •
edited

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_bytes a 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.

jaraco pushed a commit that referenced this issue Dec 28, 2022
@samety
gh-100585: Fixed a bug where importlib.resources.as_file was leaving …
f10f503 
…file pointers open (GH-100586)

* gh-100585: Fixed open fp bug in the imporlib module

* Added news for gh-100585
@jaraco
Copy link
Member

jaraco commented Dec 28, 2022

I cherry-picked the change into importlib_resources as python/importlib_resources@d623078. Thanks for reporting and the fix.

@jaraco jaraco closed this as completed Dec 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
topic-importlib type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@warsaw @jaraco @samety @AlexWaygood
X Tutup