As I understand, this doesn't solve problem, just "shuts up" the noise from known refleaks ontest_import, right?

As I understand, this doesn't solve problem, just "shuts up" the noise from known refleaks ontest_import, right?

That is not true. I do think I did resolve all ref leaks on Linux.

But I did not test it on Windows.

As I understand, this doesn't solve problem, just "shuts up" the noise from known refleaks ontest_import, right?

That is not true. I do think I did resolve all ref leaks on Linux.

But I did not test it on Windows.

Oh, seems on Windows problem doesn't solved. I rebuilt intepreter with your changes and got it:

 ./python-mtest-R3:3test_importRunningDebug|x64interpreter...0:00:00Runtestssequentially0:00:00 [1/1]test_importbeginning6repetitions123456......test_importleaked [41,43,40]references,sum=124test_importleaked [41,41,40]memoryblocks,sum=122test_importfailed (referenceleak)==Testsresult:FAILURE==1testfailed:test_importTotalduration:28.7secTestsresult:FAILURE

All ref leaks on Linux come fromSinglephaseInitTests, but running

 .\python_d.exe -m test test_import -R 3:3 -v -i *SinglephaseInitTests*

on Windows does give ref leaks

OK (skipped=5).test_import leaked [41, 43, 40] references, sum=124test_import leaked [41, 41, 40] memory blocks, sum=122

@Eclips4 Can you bisect which test leaks ref?

@Eclips4 Can you bisect which test leaks ref?

Sure!
If we deleteImportTests/test_concurrency refleak tests are passed. Seems that problem in this test

Also, I tried to found commit which introduce this reference leak ( intest_concurrency)
and again bisected to#94504..

Oh, seems your solution also works on Windows. ( I forgot to add-j 1 parameter 🤦‍♂️ , sorry)
But problem withtest_concurrency still there, and that's interesting, because it doesn't reproducible on Linux.

Probably, it should be a another PR (at the current moment I still haven't found a solution).
My some notes about it:
If we delete these lines fromtest_concurrency:

sys.modules.pop('package',None)sys.modules.pop('package.submodule',None)

Refleak tests will pass.
Also, this problem confirms that Windows runner in buildbots fleet really needed.

revertall other changes introduced in this PR?

There are 3 leak points when running in Linux. Applying your suggestions only fixes one of them.

revertall other changes introduced in this PR?

There are 3 leak points when running in Linux. Applying your suggestions only fixes one of them.

I suggest you turn this PR¹ into a fix for this one issue, so we can land that fix. The fix is obvious; I can land that for you today. The rest of the changes are controversial; I think it would be wise to separate them out.

Also: please don't tag people with in commit messages. Those commits ends up as noise in the GitHub notifications page.

1. or create a new PR for this↩

I will create a new one.

#105082

Status for buildbot run for commit20307d0:

• buildbot/AMD64 Fedora Stable Refleaks PR:test_peg_generator leaked
• buildbot/AMD64 RHEL7 Refleaks PR:test_peg_generator leaked
• buildbot/AMD64 RHEL8 Refleaks PR:test_peg_generator leaked
• buildbot/PPC64LE RHEL8 Refleaks PR:test_peg_generator leaked
• buildbot/aarch64 RHEL8 Refleaks PR:test_peg_generator leaked
• buildbot/s390x Fedora Refleaks PR:test_peg_generator leaked
• buildbot/s390x RHEL7 Refleaks PR:test_peg_generator leaked
• buildbot/s390x RHEL8 Refleaks PR:test_peg_generator leaked

GitHub took me straight to "Files changed", so I didn't see the other comments before I gave an approving review. The change looks fine to me, but please be sure to address any other comments before this is merged.

The second part is split to#105085

I will make this PR focus on the 3rd
part of the problem.

Now this PR only has changes related to the 3rd part.

test_basic_multiple_interpreters_deleted_no_reset deliberately leaks things. And we have a "no-leak" version test right after this:test_basic_multiple_interpreters_reset_each.

I think the correct thing to do here is to skip this specific test when hunting ref leaks.

However, the previous approach does not work under-j. And the test main has no way to pass the information to the actual test. So I came up with the name trick.

I am open to any other methods.

🤖 New build scheduled with the buildbot fleet by@sunmy2019 for commit5934e50 🤖

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

🤖 New build scheduled with the buildbot fleet by@sunmy2019 for commit60b4921 🤖

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

Closing in favour ofgh-106013. Thanks for the effort you've put into this@sunmy2019; highly appreciated!

Also, thanks for keeping up with me; sorry for pushing you in different directions, but my opinions changed.