8000 gh-135552: Add tests that check if weakref for tp_subclasses cleared after finalization by sergey-miryanov · Pull Request #136304 · python/cpython · GitHub
[go: up one dir, main page]
Skip to content

gh-135552: Add tests that check if weakref for tp_subclasses cleared after finalization #136304

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
Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

gh-135552: Add tests that check if weakref for tp_subclasses cleared after finalization #136304

wants to merge 11 commits into from

Conversation

sergey-miryanov
Copy link
Contributor
@sergey-miryanov sergey-miryanov commented Jul 4, 2025 •
edited by bedevere-app bot
Loading

@bedevere-app bedevere-app bot added awaiting review tests Tests in the Lib/test dir labels Jul 4, 2025
@bedevere-app bedevere-app bot mentioned this pull request Jul 4, 2025
Open
@sergey-miryanov
Copy link
Contributor Author

I believe this PR should skip news

neonene
neonene reviewed Jul 4, 2025
View reviewed changes
@nascheme
Copy link
Member
nascheme commented Jul 8, 2025

This looks okay to me. Some suggested improvements. The new test might fit better into test_weakref.py but that's minor.

I don't think it's required to run these in a separate Python process. That makes the test run slower and it's not supposed to crash. Generally a separate process is used for the tests that exercise the interpreter shutdown logic and we are not testing that. So you could just do:

    def test_clearing_weakrefs_in_gc(self):
        # This test checks that:
        # 1. weakrefs for types with callbacks are cleared before the
        # finalizer is called
        # 2. weakrefs for types without callbacks are cleared after the
        # finalizer is called
        # 3. other weakrefs cleared before the finalizer is called
        errors = []
        class Class:
            def __init__(self):
                self._self = self
                self._1 = weakref.ref(Class, lambda x: None)
                self._2 = weakref.ref(Class)
                self._3 = weakref.ref(self)

            def __del__(self):
                if self._1() is not None:
                    errors.append("Type weakref with callback is not None as expected")
                if self._2() is not Class:
                    errors.append("Type weakref is not Class as expected")
                if self._3() is not None:
                    errors.append("Instance weakref is not None as expected")
        Class()
        gc.collect()
        self.assertEqual(errors, [])

I find the variable naming a bit esoteric. I would have just called them wr1, wr2 and wr3.

@sergey-miryanov
Copy link
Contributor Author

@nascheme Thanks for review! Fixed.

kumaraditya303
kumaraditya303 reviewed Jul 8, 2025
View reviewed changes
kumaraditya303
kumaraditya303 reviewed Jul 8, 2025
View reviewed changes
@sergey-miryanov @kumaraditya303
Apply suggestions from code review
7788e55 
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@sergey-miryanov
Copy link
Contributor Author

@kumaraditya303 Thanks for review!

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

@neonene neonene neonene left review comments

@kumaraditya303 kumaraditya303 kumaraditya303 left review comments

Assignees
No one assigned
Labels
awaiting review skip news tests Tests in the Lib/test dir
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@sergey-miryanov @nascheme @neonene @kumaraditya303 @StanFromIreland @fxeqxmulfx
0