classification
Title: _ast module state should be made per interpreter
Type: Stage: resolved
Components: Interpreter Core Versions: Python 3.10
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: BTaskaya, petr.viktorin, shihai1991, vstinner
Priority: normal Keywords: patch

Created on 2020-09-16 10:10 by vstinner, last changed 2020-11-03 17:32 by vstinner. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 23024 merged vstinner, 2020-10-29 11:49
PR 23131 merged vstinner, 2020-11-03 16:40
Messages (7)
msg376983 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-16 10:10
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:
https://github.com/python/cpython/pull/21961#issuecomment-685821519

I agree since I would like to convert all static types to heap types: bpo-40077.
msg376989 - (view) Author: Petr Viktorin (petr.viktorin) * (Python committer) Date: 2020-09-16 11:53
I started this here: https://github.com/python/cpython/commit/60960cba606573450e76934c954787419524147d
Feel free to take that code.
msg376996 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-09-16 14:26
> I started this here: https://github.com/python/cpython/commit/60960cba606573450e76934c954787419524147d 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.
msg380251 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-02 21:03
New changeset 5cf4782a2630629d0978bf4cf6b6340365f449b2 by Victor Stinner in branch 'master':
bpo-41796: Make _ast module state per interpreter (GH-23024)
https://github.com/python/cpython/commit/5cf4782a2630629d0978bf4cf6b6340365f449b2
msg380272 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-03 12:33
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.
msg380284 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-03 17:07
New changeset fd957c124c44441d9c5eaf61f7af8cf266bafcb1 by Victor Stinner in branch 'master':
bpo-41796: Call _PyAST_Fini() earlier to fix a leak (GH-23131)
https://github.com/python/cpython/commit/fd957c124c44441d9c5eaf61f7af8cf266bafcb1
msg380286 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2020-11-03 17:32
The leak is fixed, I close again the issue. Thanks for the report Pablo.
History
Date User Action Args
2020-11-03 17:32:34vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg380286

stage: patch review -> resolved
2020-11-03 17:07:19vstinnersetmessages: + msg380284
2020-11-03 16:40:45vstinnersetstage: resolved -> patch review
pull_requests: + pull_request22047
2020-11-03 14:35:42BTaskayasetnosy: + BTaskaya
2020-11-03 12:33:55vstinnersetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg380272
2020-11-03 12:31:10vstinnerlinkissue42250 superseder
2020-11-02 22:15:01vstinnersetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2020-11-02 21:03:35vstinnersetmessages: + msg380251
2020-10-29 15:32:56shihai1991setnosy: + shihai1991
2020-10-29 11:49:54vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request21938
2020-09-16 14:26:30vstinnersetmessages: + msg376996
2020-09-16 11:53:38petr.viktorinsetnosy: + petr.viktorin
messages: + msg376989
2020-09-16 10:10:24vstinnercreate