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-38379: don't claim objects are collected when they aren't #16658

Merged
merged 6 commits into from
Oct 9, 2019
Merged

bpo-38379: don't claim objects are collected when they aren't #16658

merged 6 commits into from
Oct 9, 2019

Conversation

tim-one
Copy link
Member

@tim-one tim-one commented Oct 8, 2019 •
edited by miss-islington
Loading

When a finalizer resurrects an object, nothing is actually collected in this run of gc. Change the stats to reflect that truth.

https://bugs.python.org/issue38379

Automerge-Triggered-By: @tim-one

@tim-one
bpo-38379: when a finalizer resurrects an object,
2b4e806 
nothing is actually collected in this run of gc.
Change the stats to relect that truth.
@nascheme
Copy link
Member

nascheme commented Oct 8, 2019

LGTM. I don't think the performance impact is significant either and the stats are wrong without this fix.

@tim-one
Copy link
Member Author

tim-one commented Oct 8, 2019

I don't think the performance impact is significant either

There is no performance hit here - that would come with the second half of the bpo suggestions, if gc were taught how to keep on collecting despite that a finalizer resurrected some trash. As is (with or without this PR so far), gc gives up after detecting a resurrection. Which leads to the test case on the bpo issue, where trash keeps on building up forever and gc never make progress.

@tim-one tim-one self-assigned this Oct 9, 2019
@tim-one tim-one merged commit ecbf35f into python:master Oct 9, 2019
@tim-one tim-one deleted the gcfiddle branch October 9, 2019 17:37
@bedevere-bot
Copy link

@tim-one: Please replace # with GH- in the commit message next time. Thanks!

@miss-islington
Copy link
Contributor

Thanks @tim-one for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

I'm having trouble backporting to 3.8. Reason: 'Error 110 while writing to socket. Connection timed out.'. Please retry by removing and re-adding the needs backport to 3.8 label.

@miss-islington
Copy link
Contributor

Sorry, @tim-one, I could not cleanly backport this to 3.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker ecbf35f9335b0420cb8adfda6f299d6747a16515 3.7

@tim-one tim-one restored the gcfiddle branch October 9, 2019 17:38
@miss-islington
Copy link
Contributor

Thanks @tim-one for the PR 🌮🎉.. I'm working now to backport this PR to: 3.8.
🐍🍒⛏🤖

@miss-islington
Copy link
Contributor

Sorry @tim-one, I had trouble checking out the 3.8 backport branch.
Please backport using cherry_picker on command line.
cherry_picker ecbf35f9335b0420cb8adfda6f299d6747a16515 3.8

@tim-one
Copy link
Member Author

tim-one commented Oct 9, 2019

I have no idea how to backport this (well - or anything else). I installed cherry_picker and ran the suggested commands, but even in --dry-run mode they blow up with stuff like

subprocess.CalledProcessError: Command '['git', 'log', '-r', 'ecbf35f']' returned non-zero exit status 128.

So I'm giving up before I do real harm ☹️

@tim-one tim-one deleted the gcfiddle branch October 9, 2019 18:02
@pablogsal pablogsal mentioned this pull request Oct 9, 2019
Merged
@bedevere-bot
Copy link

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

pablogsal pushed a commit to pablogsal/cpython that referenced this pull request Oct 9, 2019
@tim-one @pablogsal
[3.8] bpo-38379: don't claim objects are collected when they aren't (p…
bef43ac 
…ythonGH-16658)

* bpo-38379:  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth.
(cherry picked from commit ecbf35f)

Co-authored-by: Tim Peters <tim.peters@gmail.com>
@pablogsal pablogsal mentioned this pull request Oct 9, 2019
Merged
@bedevere-bot
Copy link

GH-16685 is a backport of this pull request to the 3.7 branch.

pablogsal pushed a commit to pablogsal/cpython that referenced this pull request Oct 9, 2019
@tim-one @pablogsal
[3.7] bpo-38379: don't claim objects are collected when they aren't (p…
92fab65 
…ythonGH-16658)

* bpo-38379:  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth..
(cherry picked from commit ecbf35f)

Co-authored-by: Tim Peters <tim.peters@gmail.com>
miss-islington pushed a commit that referenced this pull request Oct 9, 2019
@pablogsal @miss-islington
[3.8] bpo-38379: don't claim objects are collected when they aren't (G…
0bd9fac 
…H-16658) (GH-16683)

* [bpo-38379](https://bugs.python.org/issue38379):  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth.
(cherry picked from commit ecbf35f)

Co-authored-by: Tim Peters <tim.peters@gmail.com>





https://bugs.python.org/issue38379



Automerge-Triggered-By: @pablogsal
miss-islington pushed a commit that referenced this pull request Oct 9, 2019
@pablogsal @miss-islington
[3.7] bpo-38379: don't claim objects are collected when they aren't (G…
a081e93 
…H-16658) (GH-16685)

* [bpo-38379](https://bugs.python.org/issue38379):  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth..
(cherry picked from commit ecbf35f)

Co-authored-by: Tim Peters <tim.peters@gmail.com>





https://bugs.python.org/issue38379



Automerge-Triggered-By: @pablogsal
jacobneiltaylor pushed a commit to jacobneiltaylor/cpython that referenced this pull request Dec 5, 2019
@tim-one
bpo-38379: don't claim objects are collected when they aren't (python…
4b5f86b 
…#16658)

* bpo-38379:  when a finalizer resurrects an object,
nothing is actually collected in this run of gc.
Change the stats to relect that truth.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@nascheme nascheme nascheme left review comments

Assignees

@tim-one tim-one

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@tim-one @nascheme @bedevere-bot @miss-islington @the-knights-who-say-ni