Skip to content

bpo-43901: Fix refleaks in test_module #25754

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

Merged
merged 2 commits into from
Apr 30, 2021
Merged

Conversation

pablogsal
Copy link
Member

@pablogsal pablogsal commented Apr 30, 2021

@pablogsal
Copy link
Member Author

I'm still missing another leak:

𓋹 ./python.exe -m test test_module -R :
0:00:00 load avg: 2.89 Run tests sequentially
0:00:00 load avg: 2.89 [1/1] test_module
beginning 9 repetitions
123456789
.........
test_module leaked [1, 1, 1, 1] references, sum=4
test_module failed

== Tests result: FAILURE ==

1 test failed:
    test_module

Total duration: 2.0 sec
Tests result: FAILURE

@pablogsal
Copy link
Member Author

I'm still missing another leak:

Gotcha! I have it fixed in 3640843

@pablogsal
Copy link
Member Author

Ok, we are clean:

𓋹 ./python.exe -m test test_module -R :
0:00:00 load avg: 3.33 Run tests sequentially
0:00:00 load avg: 3.33 [1/1] test_module
beginning 9 repetitions
123456789
.........

== Tests result: SUCCESS ==

1 test OK.

Total duration: 2.0 sec
Tests result: SUCCESS

@larryhastings
Copy link
Contributor

I was about to make my PR ;-)

@pablogsal
Copy link
Member Author

I was about to make my PR ;-)

Sorry, I was doing this patch and missed your comments on BPO :(

@larryhastings
Copy link
Contributor

It's hard to see how that code could ever have passed a refleak check! It seems like would always have leaked that dict. Yet I swear I ran one!

@pablogsal
Copy link
Member Author

@larryhastings If you already have the PR made I can close this one and we can merge yours if you prefer :)

@larryhastings
Copy link
Contributor

I don't mind the parallel effort, as long as it gets fixed one way or another. I've caused enough headaches and gnashing of teeth this week--let's try and keep this incident to a minimum!

@larryhastings
Copy link
Contributor

Let's go with yours, the PR is already up and checks are running.

@pablogsal
Copy link
Member Author

I've caused enough headaches and gnashing of teeth this week--let's try and keep this incident to a minimum!

No problem Larry! This happens quite a lot for everyone, and the important thing is that we have your patch for 3.10 :)

@larryhastings
Copy link
Contributor

No problem Larry! This happens quite a lot for everyone,

What rot! This never happened during the release process for 3.4 and 3.5! Nobody ever broke the build, ever. I'm clearly a tiresome outlier.

@pablogsal pablogsal merged commit e374a40 into python:master Apr 30, 2021
@pablogsal pablogsal deleted the bpo-43901 branch April 30, 2021 16:53
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.

4 participants