Skip to content

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

Merged
miss-islington merged 1 commit intopython:masterfrom
eduardo-elizondo:fix-capi-leaks
Sep 13, 2019
Merged

bpo-38150 Fix refleak in the finalizer of a _testcapimodule type#16115
miss-islington merged 1 commit intopython:masterfrom
eduardo-elizondo:fix-capi-leaks

Conversation

@eduardo-elizondo
Copy link
Copy Markdown
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
Copy Markdown
Contributor Author

eduardo-elizondo commented Sep 13, 2019

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
Copy Markdown
Member

matrixise commented Sep 13, 2019

@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
Copy Markdown
Member

matrixise commented Sep 13, 2019

works fine on my laptop, need to wait for Travis

@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented Sep 13, 2019

@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
Copy Markdown
Contributor

miss-islington commented Sep 13, 2019

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
Copy Markdown
Contributor

miss-islington commented Sep 13, 2019

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 <[email protected]>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Sep 13, 2019

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 <[email protected]>
@eduardo-elizondo
Copy link
Copy Markdown
Contributor Author

eduardo-elizondo commented Sep 13, 2019

@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