Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

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

Merged
nascheme merged 13 commits into
python:mainpython/cpython:mainfrom
sergey-miryanov:gh-135552-add-tests-gc-weakref-tp_subclassessergey-miryanov/cpython:gh-135552-add-tests-gc-weakref-tp_subclassesCopy head branch name to clipboard
Aug 8, 2025
Merged

gh-135552: Add tests that check if weakref for tp_subclasses cleared after finalization#136304
nascheme merged 13 commits into
python:mainpython/cpython:mainfrom
sergey-miryanov:gh-135552-add-tests-gc-weakref-tp_subclassessergey-miryanov/cpython:gh-135552-add-tests-gc-weakref-tp_subclassesCopy head branch name to clipboard

Conversation

@sergey-miryanov

@sergey-miryanov sergey-miryanov commented Jul 4, 2025

Copy link
Copy Markdown
Contributor

@sergey-miryanov

Copy link
Copy Markdown
Contributor Author

I believe this PR should skip news

Comment thread Lib/test/test_gc.py Outdated
@nascheme

nascheme commented Jul 8, 2025

Copy link
Copy Markdown
Member

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

@nascheme Thanks for review! Fixed.

Comment thread Lib/test/test_gc.py Outdated
Comment thread Lib/test/test_gc.py Outdated
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
@sergey-miryanov

Copy link
Copy Markdown
Contributor Author

@kumaraditya303 Thanks for review!

Types are not treated specially, it only matters if the weakref has a callback or not.
@nascheme nascheme merged commit 25518f5 into python:main Aug 8, 2025
40 checks passed
@sergey-miryanov

Copy link
Copy Markdown
Contributor Author

Thanks everyone!

@sergey-miryanov sergey-miryanov deleted the gh-135552-add-tests-gc-weakref-tp_subclasses branch August 8, 2025 04:42
Agent-Hellboy pushed a commit to Agent-Hellboy/cpython that referenced this pull request Aug 19, 2025
These are tests to ensure behaviour introduced by pythonGH-136189 is working as expected.

Co-authored-by: Mikhail Borisov <43937008+fxeqxmulfx@users.noreply.github.com>
Co-authored-by: Kumar Aditya <kumaraditya@python.org>
Co-authored-by: Neil Schemenauer <nas-github@arctrix.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

skip news tests Tests in the Lib/test dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.