Skip to content
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

gh-116731: Fix refleaks in importlib tests #116763

Closed
wants to merge 4 commits into from

Conversation

jaraco
Copy link
Member

@jaraco jaraco commented Mar 13, 2024

  • 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

@jaraco jaraco changed the title Fix refleaks in importlib tests gh-116731: Fix refleaks in importlib tests Mar 13, 2024
@jaraco jaraco force-pushed the gh-116731/importlib-refleaks branch from b576685 to b33e4d5 Compare March 13, 2024 21:02
@jaraco
Copy link
Member Author

jaraco commented Mar 13, 2024

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
Member Author

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?

@jaraco jaraco 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

🤖 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

🤖 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-bot bedevere-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 vstinner March 13, 2024 21:06
@jaraco
Copy link
Member Author

jaraco commented Mar 13, 2024

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

@jaraco
Copy link
Member Author

jaraco commented Mar 14, 2024

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 free to 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.

None yet

2 participants