-
-
Notifications
You must be signed in to change notification settings - Fork 33.7k
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
Changes from all commits
bf3f888
e6c11de
53f63b9
c8ea776
fcd7ca0
0f9b509
6e3b071
86fc36b
329c09c
dd20441
b1f67a4
15c0c5b
a7ef9f2
766d510
53880a6
8a37683
6c5c5e8
File filter
Filter by extension
Conversations
Jump to
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. |
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.