Skip to content

gh-116731: Fix refleaks in importlib tests - #116763

Closed
jaraco wants to merge 4 commits into
python:mainfrom
jaraco:gh-116731/importlib-refleaks
Closed

gh-116731: Fix refleaks in importlib tests#116763
jaraco wants to merge 4 commits into
python:mainfrom
jaraco:gh-116731/importlib-refleaks

Conversation

@jaraco

@jaracojaraco commented Mar 13, 2024

Copy link
Copy Markdown
Member
  • In PathFinder.invalidate_caches, also invoke MetadataPathFinder.invalidate_caches.
  • Invoke invalidate_caches in tests exhibiting refleaks.
  • Address ref leak in inspect triggered by call to files().
  • Add blurb

@jaracojaraco changed the title Fix refleaks in importlib testsgh-116731: Fix refleaks in importlib testsMar 13, 2024
@jaraco
jaracoforce-pushed the gh-116731/importlib-refleaks branch from b576685 to b33e4d5CompareMarch 13, 2024 21:02
@jaraco

Copy link
Copy Markdown
MemberAuthor

With these changes, refleaks checks now pass:

 cpython gh-116731/importlib-refleaks @ ./python.exe -m test test_importlib -R 3:3
Using random seed: 894808194
0:00:00 load avg: 1.17 Run 1 test sequentially
0:00:00 load avg: 1.17 [1/1] test_importlib
beginning 6 repetitions. Showing number of leaks (. for 0 or less, X for 10 or more)
123:456
XX. 12.
test_importlib leaked [1, 2, 0] references, sum=3 (this is fine)
test_importlib leaked [1, 1, 0] memory blocks, sum=2 (this is fine)
== Tests result: SUCCESS ==
1 test OK.
Total duration: 14.0 sec
Total tests: run=1,274 skipped=6
Total test files: run=1/1
Result: SUCCESS

Comment on lines +1473 to +1474
from importlib.metadata import MetadataPathFinder
MetadataPathFinder().invalidate_caches()

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brettcannon I included you in the review for this section. Any concerns with adding this hook here?

@jaracojaraco added 🔨 test-with-buildbots Test PR w/ buildbots; report in status section 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section labels Mar 13, 2024
@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @jaraco for commit b33e4d5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-buildbots label again.

@bedevere-bot

Copy link
Copy Markdown

🤖 New build scheduled with the buildbot fleet by @jaraco for commit b33e4d5 🤖

If you want to schedule another build, you need to add the 🔨 test-with-refleak-buildbots label again.

@bedevere-botbedevere-bot removed 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section 🔨 test-with-buildbots Test PR w/ buildbots; report in status section labels Mar 13, 2024
@jaraco
jaraco requested a review from vstinnerMarch 13, 2024 21:06
@jaraco

Copy link
Copy Markdown
MemberAuthor

I believe this build was the one triggered for refleaks. It looks like it may have failed to start.

@jaraco

Copy link
Copy Markdown
MemberAuthor

This is superseded by a cleaner approach in #116805. I still think MetadataPathFinder.invalidate_caches should be called from PathFinder.invalidate_caches, as that will match the behavior that would happen if MetadataPathFinder were on sys.meta_path. I'll track that in a separate issue.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@jaraco@bedevere-bot