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-101766: Fix refleak for _BlockingOnManager resources at test-suite level #101987

Closed
wants to merge 2 commits into from

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Feb 17, 2023

@corona10 corona10 requested a review from gvanrossum February 17, 2023 12:29
@corona10 corona10 marked this pull request as ready for review February 17, 2023 12:29
@corona10 corona10 added the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 17, 2023
@bedevere-bot
Copy link

🤖 New build scheduled with the buildbot fleet by @corona10 for commit 6d667d4 🤖

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

@bedevere-bot bedevere-bot removed the 🔨 test-with-refleak-buildbots Test PR w/ refleak buildbots; report in status section label Feb 17, 2023
@corona10 corona10 requested a review from sobolevn February 17, 2023 12:29
@corona10
Copy link
Member Author

@exarkun Would you like to take a look?

@corona10
Copy link
Member Author

FYI,
I verified this PR by running 50 times refleak test suite for the test_importlib

@corona10
Copy link
Member Author

corona10 commented Feb 17, 2023

Oops it doesn't solve the issue

@corona10 corona10 closed this Feb 17, 2023
@exarkun
Copy link
Contributor

exarkun commented Feb 17, 2023

Oops it doesn't solve the issue

I would make one PR that just reverts the original change followed by another PR that fixes the object leak. That way the eventual fix is not mixed together with remnants of the earlier incomplete fix. Also, it lets the regression in importlib get fixed immediately while we take what time we need to fix the object leak.

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

3 participants