GeezerVison

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

As I understand, this doesn't solve problem, just "shuts up" the noise from known refleaks on test_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 on test_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 -m test -R 3:3 test_import
Running Debug|x64 interpreter...
0:00:00 Run tests sequentially
0:00:00 [1/1] test_import
beginning 6 repetitions
123456
......
test_import leaked [41, 43, 40] references, sum=124
test_import leaked [41, 41, 40] memory blocks, sum=122
test_import failed (reference leak)

== Tests result: FAILURE ==

1 test failed:
    test_import

Total duration: 28.7 sec
Tests result: FAILURE

All ref leaks on Linux come from SinglephaseInitTests, 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=124
test_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 delete ImportTests/test_concurrency refleak tests are passed. Seems that problem in this test

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

Oh, seems your solution also works on Windows. ( I forgot to add -j 1 parameter 🤦‍♂️ , sorry)
But problem with test_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 from test_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.

revert all other changes introduced in this PR?

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

revert all 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 commit 20307d0:

• 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 commit 5934e50 🤖

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 commit 60b4921 🤖

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

Closing in favour of gh-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.