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.

Sign up for GitHub

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

Jump to bottom

bpo-38150 Fix refleak in the finalizer of a _testcapimodule type #16115

Merged
merged 1 commit into from
Sep 13, 2019
Merged

bpo-38150 Fix refleak in the finalizer of a _testcapimodule type #16115

merged 1 commit into from
Sep 13, 2019

Conversation

eduardo-elizondo
Copy link
Contributor

@eduardo-elizondo eduardo-elizondo commented Sep 13, 2019 •
edited by miss-islington
Loading

The PyLong created in the finalizer was not being cleaned up

https://bugs.python.org/issue38150

Automerge-Triggered-By: @matrixise

@eduardo-elizondo eduardo-elizondo changed the title bpo-38150 Fix refleak in the finalizer of a _testcapimodule type [WIP] bpo-38150 Fix refleak in the finalizer of a _testcapimodule type Sep 13, 2019
@eduardo-elizondo eduardo-elizondo changed the title [WIP] bpo-38150 Fix refleak in the finalizer of a _testcapimodule type bpo-38150 Fix refleak in the finalizer of a _testcapimodule type Sep 13, 2019
@eduardo-elizondo
Copy link
Contributor Author

Since the build bots don't run the test leaks, here's a local repro that is now working:

eelizondo@build -> ./python -m test test_capi -R 3:3 -m test.test_capi.CAPITest.test_c_subclass_of_heap_ctype_with_del_modifying_dunder_class_only_decrefs_once
Run tests sequentially
0:00:00 load avg: 1.63 [1/1] test_capi
beginning 6 repetitions
123456
......

== Tests result: SUCCESS ==

1 test OK.

Total duration: 147 ms
Tests result: SUCCESS

@matrixise
Copy link
Member

@eduardo-elizondo yep, I am waiting for the green lanterns from Travis and I will merge your PR. need to test on my laptop but it is already working on your PR ;-)

@matrixise
Copy link
Member

works fine on my laptop, need to wait for Travis

@miss-islington
Copy link
Contributor

@eduardo-elizondo: Status check is done, and it's a success ✅ .

@miss-islington miss-islington merged commit a67ac2f into python:master Sep 13, 2019
@miss-islington
Copy link
Contributor

Thanks @eduardo-elizondo for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

@miss-islington
Copy link
Contributor

Sorry @eduardo-elizondo, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker a67ac2f2d9550e5a36d28f9b6eeacf6575dda2d5 3.8

@miss-islington miss-islington self-assigned this Sep 13, 2019
matrixise pushed a commit to matrixise/cpython that referenced this pull request Sep 13, 2019
@eduardo-elizondo @matrixise
[3.8] bpo-38150 Fix refleak in the finalizer of a _testcapimodule type (
13a638a 
pythonGH-16115)

The PyLong created in the finalizer was not being cleaned up

https://bugs.python.org/issue38150

Automerge-Triggered-By: @matrixise
(cherry picked from commit a67ac2f)

Co-authored-by: Eddie Elizondo <eelizondo@fb.com>
@bedevere-bot
Copy link

GH-16118 is a backport of this pull request to the 3.8 branch.

matrixise added a commit that referenced this pull request Sep 13, 2019
@matrixise @eduardo-elizondo
[3.8] bpo-38150: Fix refleak in the finalizer of a _testcapimodule ty…
53ff2ca 
…pe (GH-16115) (GH-16118)

The PyLong created in the finalizer was not being cleaned up

https://bugs.python.org/issue38150

Automerge-Triggered-By: @matrixise
(cherry picked from commit a67ac2f)

Co-authored-by: Eddie Elizondo <eelizondo@fb.com>
@eduardo-elizondo
Copy link
Contributor Author

@matrixise Thanks for the quick review!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matrixise matrixise matrixise approved these changes

Assignees

@miss-islington miss-islington

Labels
skip news
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@eduardo-elizondo @matrixise @miss-islington @bedevere-bot @the-knights-who-say-ni