gh-140601: Add ResourceWarning to iterparse when not closed #140603
Conversation
|
All commit authors signed the Contributor License Agreement. |
|
Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the |
osamakader
force-pushed
the
issue-140601-iterparse-warning
branch
5 times, most recently
from
7d1d79a to
54742a1
Compare
When iterparse() opens a file by filename and is not explicitly closed, emit a ResourceWarning to alert developers of the resource leak. This implements the TODO comment at line 1270 of ElementTree.py which has been requesting this feature since the close() method was added. - Add _closed flag to IterParseIterator to track state - Emit ResourceWarning in __del__ if not closed - Add comprehensive test cases - Update existing tests to properly close iterators Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
osamakader
force-pushed
the
issue-140601-iterparse-warning
branch
from
54742a1 to
bf3f888
Compare
|
Please don't force push to CPython PRs; it's preferred to keep commit history intact for easier review. See: https://devguide.python.org/getting-started/pull-request-lifecycle/#quick-guide |
|
cc: @serhiy-storchaka who added the |
This comment was marked as duplicate.
This comment was marked as duplicate.
Sorry, something went wrong.
Lib/test/test_xml_etree.py
Outdated
| it = iterparse(SIMPLE_XMLFILE) | ||
| del it | ||
| import gc | ||
| gc.collect() # Ensure previous iterator is cleaned up |
There was a problem hiding this comment.
Wouldn't it be better to use support.gc_collect() instead of directly calling gc.collect() was specifically added for use in tests to ensure consistent and deterministic garbage collection behavior across platforms and configurations.
There was a problem hiding this comment.
These tests were intentionally added to check that the resource is closed without explicit close(). Don't add close(). Just replace check_no_resource_warning() with assertWarns(ResourceWarning) and add gc_collect(). There are other tests that explicitly call close().
- Use nonlocal close_source instead of _closed flag - Bind warnings.warn as default parameter in __del__ - Use assertWarns(ResourceWarning) instead of adding close() calls - Use support.gc_collect() for consistent test behavior - Remove broad exception handling Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
F-strings can fail during cleanup. Use % formatting like subprocess.Popen does for safer __del__ behavior. Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Check that warning message contains 'unclosed file' and the filename to ensure it's from iterparse, not the file destructor. Addresses review feedback from serhiy-storchaka. Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
The ResourceWarning tests are already covered in test_iterparse() as noted by serhiy-storchaka in review. Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
Removing source=self prevents unraisable exception in C extension. The warning message still contains the filename via %r formatting. Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
|
I've addressed all the review feedback: Is this expected behavior for warnings in Thanks! |
|
No, it is not an expected behavior. I have fixed the warnings. The underlying file is closed when the iterator has been exhausted or raised an exception, so no explicit close() is needed in this case. And many tests depended on this. I have removed a ResourceWarning in such case. Please add a |
Document that iterparse now emits ResourceWarning in Python 3.15 when not explicitly closed. Signed-off-by: Osama Abdelkader <osama.abdelkader@gmail.com>
|
Thanks @serhiy-storchaka for the support. |
4 participants
When iterparse() opens a file by filename and is not explicitly closed, emit a ResourceWarning to alert developers of the resource leak.
This implements the TODO comment at line 1270 of ElementTree.py which has been requesting this feature since the close() method was added.