Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 33.9k
gh-140601: Add ResourceWarning to iterparse when not closed#140603
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
bf3f888e6c11de53f63b9c8ea776fcd7ca00f9b5096e3b07186fc36b329c09cdd20441b1f67a415c0c5ba7ef9f2766d51053880a68a376836c5c5e8File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1261,16 +1261,20 @@ def iterator(source): | ||||||||||||||||||||
| gen = iterator(source) | ||||||||||||||||||||
| class IterParseIterator(collections.abc.Iterator): | ||||||||||||||||||||
| __next__ = gen.__next__ | ||||||||||||||||||||
| def close(self): | ||||||||||||||||||||
| nonlocal close_source | ||||||||||||||||||||
| if close_source: | ||||||||||||||||||||
| source.close() | ||||||||||||||||||||
| close_source = False | ||||||||||||||||||||
| gen.close() | ||||||||||||||||||||
| def __del__(self): | ||||||||||||||||||||
| # TODO: Emit a ResourceWarning if it was not explicitly closed. | ||||||||||||||||||||
| # (When the close() method will be supported in all maintained Python versions.) | ||||||||||||||||||||
| def __del__(self, _warn=warnings.warn): | ||||||||||||||||||||
| if close_source: | ||||||||||||||||||||
| source.close() | ||||||||||||||||||||
| try: | ||||||||||||||||||||
| _warn(f"unclosed iterparse iterator{source.name!r}", ResourceWarning, stacklevel=2) | ||||||||||||||||||||
| finally: | ||||||||||||||||||||
| source.close() | ||||||||||||||||||||
Comment on lines +1274 to +1277 Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe this is better? Suggested change
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
BTW, if the Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You're right - I missed that The original code is correct. (Though % formatting might be slightly safer than f-string in | ||||||||||||||||||||
| it = IterParseIterator() | ||||||||||||||||||||
| it.root = None | ||||||||||||||||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,4 @@ | ||
| :func:`xml.etree.ElementTree.iterparse` now emits a :exc:`ResourceWarning` | ||
| when the iterator is not explicitly closed and was opened with a filename. | ||
| This helps developers identify and fix resource leaks. Patch by Osama | ||
| Abdelkader. |
Uh oh!
There was an error while loading. Please reload this page.
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.
Wouldn't this option be better here?
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 wonder. It seems that only exception can be raised here is a KeyboardInterrupt. The difference between calling and not calling
gen.close()is whether you can continue iterating afteriterparse().close()(cached elements can still be yielded). But ifiterparse().close()failed, you do not have any guarantees. Anyway, the file is closed first, so there will not be leaks. I think there will be no practical difference, so we can keep the simplest code.