X Tutup
Skip to content

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

Merged
corona10 merged 5 commits intopython:mainfrom
corona10:gh-91632
Apr 20, 2022
Merged

gh-91632: Fix generic_alias_iterator to be finalized at exit.#91727
corona10 merged 5 commits intopython:mainfrom
corona10:gh-91632

Conversation

@corona10
Copy link
Member

@corona10 corona10 commented Apr 20, 2022 •
edited
Loading

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

Fidget-Spinner commented Apr 20, 2022

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
vstinner reviewed Apr 20, 2022
View reviewed changes
@vstinner
Copy link
Member

vstinner commented Apr 20, 2022

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).

vstinner
vstinner reviewed Apr 20, 2022
View reviewed changes
corona10 and others added 2 commits April 20, 2022 21:44
@corona10 @vstinner
Update Misc/NEWS.d/next/Core and Builtins/2022-04-20-14-43-37.gh-issu…
55d04c4 
…e-91632.cvUhsZ.rst

Co-authored-by: Victor Stinner <vstinner@python.org>
@corona10
Address code review
a9a938c
@corona10 corona10 requested a review from vstinner April 20, 2022 12:52
vstinner
vstinner approved these changes Apr 20, 2022
View reviewed changes
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

JelleZijlstra commented Apr 20, 2022

Should this be backported?

@Fidget-Spinner
Copy link
Member

Fidget-Spinner commented Apr 20, 2022 •
edited
Loading

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

@vstinner
Copy link
Member

vstinner commented Apr 20, 2022

Should this be backported?

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

@corona10 corona10 mentioned this pull request May 19, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@gvanrossum gvanrossum Awaiting requested review from gvanrossum

@kumaraditya303 kumaraditya303 Awaiting requested review from kumaraditya303

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Memory leak with static iterator types

5 participants

@corona10 @Fidget-Spinner @vstinner @JelleZijlstra @bedevere-bot
X Tutup