_ast module state should be made per interpreter

[vstinner]

BPO	41796
Nosy	@vstinner, @encukou, @isidentical, @shihai1991
PRs	bpo-41796: Make _ast module state per interpreter #23024 bpo-41796: Call _PyAST_Fini() earlier to fix a leak #23131

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2020-11-03.17:32:34.071>
created_at = <Date 2020-09-16.10:10:24.855>
labels = ['interpreter-core', '3.10']
title = '_ast module state should be made per interpreter'
updated_at = <Date 2020-11-03.17:32:34.070>
user = 'https://github.com/vstinner'

bugs.python.org fields:

activity = <Date 2020-11-03.17:32:34.070>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2020-11-03.17:32:34.071>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2020-09-16.10:10:24.855>
creator = 'vstinner'
dependencies = []
files = []
hgrepos = []
issue_num = 41796
keywords = ['patch']
message_count = 7.0
messages = ['376983', '376989', '376996', '380251', '380272', '380284', '380286']
nosy_count = 4.0
nosy_names = ['vstinner', 'petr.viktorin', 'BTaskaya', 'shihai1991']
pr_nums = ['23024', '23131']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue41796'
versions = ['Python 3.10']

[vstinner]

In September 2019, the _ast extension module was converted to PEP-384 (stable ABI): bpo-38113.

In bpo-41631, I moved the _ast module state back to a global state, rather than a regular module state, to fix multiple bugs.

The state should be made per intepreter (moved into PyInterpreterState).

Also, each interpreter should have its own _ast types (_ast.AST, _ast.Constant, etc.), rather than sharing all types.

Hopefully, in 3.9, _ast types have been converted to heap types.

To fix bpo-41631, I wanted to convert _ast.AST heap type back into a static type, to prevenet a subinterpreter to modify it, but Petr Viktorin asked me to leave it as it is:
#21961 (comment)

I agree since I would like to convert all static types to heap types: bpo-40077.

[encukou]

I started this here: 60960cb
Feel free to take that code.

[vstinner]

I started this here: 60960cb Feel free to take that code.

Oh thanks! I was looking for your work, but I failed to find it.

It is really helpful!

I started to hack asdl_c.py locally, but Pablo asked me in private to wait a few days, since he is going to push large changes on this file soon.

This issue is an enhancement which can wait.

[vstinner]

New changeset 5cf4782 by Victor Stinner in branch 'master':
bpo-41796: Make _ast module state per interpreter (GH-23024)
5cf4782

[vstinner]

There is a leak (I just marked bpo-42250 as duplicate of this issue):

https://buildbot.python.org/all/#/builders/205/builds/83

OK
......
test_ast leaked [23640, 23636, 23640] references, sum=70916
test_ast leaked [7932, 7930, 7932] memory blocks, sum=23794
1 test failed again:
test_ast

test_subinterpreter() test leaks.

There are two problems:

• _PyAST_Fini() is only called in the main interpreter, I forgot to remove the "if _Py_IsMainInterpreter()"
• _PyAST_Fini() is called after the last GC collection, whereas AST_type contains a reference to itself (as any Python type) in its tp_mro member. A GC collection is required to destroy the type. _PyAST_Fini() must be called before the last GC collection.

[vstinner]

New changeset fd957c1 by Victor Stinner in branch 'master':
bpo-41796: Call _PyAST_Fini() earlier to fix a leak (GH-23131)
fd957c1

[vstinner]

The leak is fixed, I close again the issue. Thanks for the report Pablo.