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.

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

gh-91632: Fix generic_alias_iterator to be finalized at exit. #91727

Merged
merged 5 commits into from
Apr 20, 2022

Conversation

corona10
Copy link
Member

@corona10 corona10 commented Apr 20, 2022

AS-IS

➜  cpython git:(main) ✗ cat gh-91632.py
a = type(iter(tuple[int]))
print(a.mro())
➜  cpython git:(main) ✗ ./python.exe -X showrefcount gh-91632.py
[<class 'generic_alias_iterator'>, <class 'object'>]
[119 refs, 46 blocks]

TO-BE

➜  cpython git:(gh-91632) ✗ cat gh-91632.py
a = type(iter(tuple[int]))
print(a.mro())
➜  cpython git:(gh-91632) ✗ ./python.exe -X showrefcount gh-91632.py
[<class 'generic_alias_iterator'>, <class 'object'>]
[0 refs, 0 blocks]

Follow Victor's suggestion :)

closes: #91632

@corona10 corona10 changed the title Fix Py_GenericAliasIterType to be finalized at exit. gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. Apr 20, 2022
@corona10 corona10 force-pushed the gh-91632 branch 2 times, most recently from c7d5777 to 39886d9 Compare April 20, 2022 05:53
@corona10 corona10 changed the title gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. [WIP] gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. Apr 20, 2022
@corona10 corona10 changed the title [WIP] gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. Apr 20, 2022
@corona10 corona10 changed the title gh-91632: Fix Py_GenericAliasIterType to be finalized at exit. gh-91632: Fix generic_alias_iterator to be finalized at exit. Apr 20, 2022
@Fidget-Spinner
Copy link
Member

I can't review this since I have no clue on the process of freeing memory on exit. But when do we add a type to static_types? Is it always?

@vstinner
Copy link
Member

I can't review this since I have no clue on the process of freeing memory on exit. But when do we add a type to static_types? Is it always?

I propose two approaches:

  • Convert static types to heap types to free memory at exit: it's a hard work, and currently hold by the SC. You need to wait until https://peps.python.org/pep-0687/ is approved for that.

  • Clear static types at Python exit (usually in Py_Finalize()):

    • Add the type to the static_types list in object.c (I tried to add all types there),
    • or: add a "Fini" function called by finalize_interp_types() (be careful of sub-interpreters: see _Py_IsMainInterpreter() function).

corona10 and others added 2 commits April 20, 2022 21:44
…e-91632.cvUhsZ.rst

Co-authored-by: Victor Stinner <vstinner@python.org>
@corona10 corona10 requested a review from vstinner April 20, 2022 12:52
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@corona10 corona10 merged commit f571c26 into python:main Apr 20, 2022
@corona10 corona10 deleted the gh-91632 branch April 20, 2022 14:10
@JelleZijlstra
Copy link
Member

Should this be backported?

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Apr 20, 2022

@JelleZijlstra No. Generic alias iterator is part of the variadic generics PEP which is 3.11 only iirc.

@vstinner
Copy link
Member

Should this be backported?

The work to clear static types at exit is new in Python 3.11 (especially the _PyTypes_FiniTypes() function).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Memory leak with static iterator types
5 participants