Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork48
Avoid file descriptor refleaks in as_file.#234
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| try: | ||
| os.remove(raw_path) | ||
| except(FileNotFoundError,PermissionError): | ||
| exceptFileNotFoundError: |
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 part though? IIRC it was added because for some reason Windows will set high permissions on temporary files sometimes.
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.
I believe this exception was masking the underlying issue that the file descriptor wasn't being closed. Windows won't allow deletion of files in use, so when we encountered the issue, we suppressed the exception, allowing the file descriptor to remain unclosed and on Windows leaving the file undeleted. Now that the file descriptor is unconditionally closed, there's no longer any need to suppress this exception and as you can see, tests still pass on Windows.
As reported inpython/cpython#27436 (comment), merging the latest importlib_resources into CPython revealed a refleak. It appears the refleak has long been a part of the implementation but was previously uncovered by the tests. This change fixes that refleak.