Repository navigation
Clear static types in Py_Finalize() for embedded Python #90575
Description
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 2022-01-27.20:22:54.970>
created_at = <Date 2022-01-17.17:19:20.743>
labels = ['interpreter-core', '3.11']
title = 'Clear static types in Py_Finalize() for embedded Python'
updated_at = <Date 2022-01-28.13:08:43.043>
user = 'https://github.com/vstinner'bugs.python.org fields:
activity = <Date 2022-01-28.13:08:43.043>
actor = 'vstinner'
assignee = 'none'
closed = True
closed_date = <Date 2022-01-27.20:22:54.970>
closer = 'vstinner'
components = ['Interpreter Core']
creation = <Date 2022-01-17.17:19:20.743>
creator = 'vstinner'
dependencies = []
files = ['50576', '50577']
hgrepos = []
issue_num = 46417
keywords = ['patch']
message_count = 50.0
messages = ['410808', '410858', '410866', '411025', '411033', '411049', '411063', '411064', '411066', '411070', '411071', '411073', '411076', '411077', '411117', '411118', '411122', '411127', '411139', '411143', '411148', '411182', '411201', '411202', '411203', '411232', '411255', '411256', '411257', '411263', '411271', '411272', '411275', '411276', '411281', '411290', '411291', '411292', '411296', '411300', '411303', '411304', '411305', '411307', '411317', '411329', '411332', '411917', '411918', '411994']
nosy_count = 8.0
nosy_names = ['vstinner', 'phsilva', 'petr.viktorin', 'corona10', 'miss-islington', 'shihai1991', 'erlendaasland', 'kumaraditya']
pr_nums = ['30645', '30732', '30733', '30734', '30735', '30736', '30743', '30744', '30749', '30750', '30760', '30761', '30763', '30764', '30769', '30767', '30775', '30788', '30789', '30790', '30791', '30793', '30795', '30796', '30798', '30804', '30805', '30806', '30807', '30809', '30810', '30815', '30964', '30988']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue46417'
versions = ['Python 3.11']
Activity
vstinner commentedon Jan 17, 2022
Converting static types to heap types is a work-in-progress:
Static types are still causing issues with Py_Initialize() / Py_Finalize() is called multiple times in the same process, which is a supported use case for embedded Python. See bpo-45691 "Partial moving of core objects to interpreter state is incorrect at best, unsafe at worse" for a recent example with sys.float_info where sys.float_info.n_unnamed_fields holds a reference to a Python object.
In june 2020, I wrote #64962 to clear static types at exit. But I abandoned my attempt because of bugs. Copy of my https://bugs.python.org/issue1635741#msg371119 message:
"""
I wrote PR 20763 to "finalize" static types in Py_Finalize(). It mostly works, but "./Programs/_testembed test_forced_io_encoding" crash. This program calls Py_Initialize() and Py_Finalize() multiple times in a loop.
It doesn't look to be safe to clear static types. Many functions rely on the fact that static types are "always there" and are never finalized. Also, only a few static types are cleared by my PR: many static types are left unchanged. For example, static types of the _io module.
It seems like a safer approach is to continue the work on bpo-40077: "Convert static types to PyType_FromSpec()".
"""
I propose a "best effort" approach: only clear a static type if it is no longer used. For example, check its reference count and its __subclasses__() method.
encukou commentedon Jan 18, 2022
If we have static types, that means there is a mechanism to share some objects across interpreters.
And if that's the case, why can't small ints (like sys.float_info.n_unnamed_fields) be static & shared as well?
vstinner commentedon Jan 18, 2022
Sharing objects between interpreters is bad and is causing complex bugs. See a recent example of an object traveling from one interpreter to another and then causing a random crash on Windows related to the garbage collector:
I would prefer to discuss that in other issues like bpo-40255 or bpo-39511, and focus this issue on fixing the static types implementation when Python is embedded in an application.
--
Hum, this issue is not really related to sub-interpreters. My proposed PR only changes Py_Finalize(): the main interpreter.
[-][subinterpreters] Clear static types in Py_Finalize()[/-][+]Clear static types in Py_Finalize() for embedded Python[/+][-][subinterpreters] Clear static types in Py_Finalize()[/-][+]Clear static types in Py_Finalize() for embedded Python[/+]encukou commentedon Jan 20, 2022
That's your opinion, I don't necessarily share it.
But converting static things (types, small ints) to heap is also causing bugs :(
----
Anyway, for this issue: is there a way to test if the types are correctly reinitialized if there are multiple finalize/init cycles?
I worry about introducing more unexpected issues here.
vstinner commentedon Jan 20, 2022
If tomorrow static types are shared between sub-interpreters, it doesn't solve this problem: we still need to release memory allocated by Py_Initialize() in Py_Finalize() when Python is embedded. This issue is a sub-set of the big bpo-1635741 which explains the rationale. Again, this issue is not about sub-interpreters.
I wasn't sure how to test it. But well, Dong-hee and you asked for tests, so I added tests for my PR ;-)
33 remaining items
vstinner commentedon Jan 22, 2022
New changeset 9c8e490 by Victor Stinner in branch 'main':
bpo-46417: Clear _io module static objects at exit (GH-30807)
9c8e490
vstinner commentedon Jan 22, 2022
New changeset 12f4ac3 by Victor Stinner in branch 'main':
bpo-46417: Clear symtable identifiers at exit (GH-30809)
12f4ac3
vstinner commentedon Jan 22, 2022
New changeset a1444f4 by Victor Stinner in branch 'main':
bpo-46417: Fix _PyStaticType_Dealloc() (GH-30810)
a1444f4
vstinner commentedon Jan 22, 2022
The _PyStaticType_Dealloc() function cannot call PyObject_ClearWeakRefs() if the refcount is not zero. I am not sure if it's a real issue or not. Maybe the weakref list must be cleared (release memory), but callbacks must not be called?
vstinner commentedon Jan 22, 2022
cannot_deallocate2.patch: updated patch to debug which types are not cleared at exit.
vstinner commentedon Jan 22, 2022
At this commit, Py_Finalize() no longer leaks references. It exceeded my expectations, it even makes _Py_RefTotal negative :-D
$ ./python -I -X showrefcount -c pass [-4 refs, 61 blocks]I'm not sure why it's negative, but bpo-46449 should be investigate first.
vstinner commentedon Jan 23, 2022
See also bpo-46476: Not all memory allocated by _Py_Quicken() is released at Python exit.
vstinner commentedon Jan 23, 2022
New changeset 976dec9 by Victor Stinner in branch 'main':
bpo-46417: _PyList_Fini() clears indexerr (GH-30815)
976dec9
vstinner commentedon Jan 23, 2022
If you apply my workaround for bpo-46476:
https://bugs.python.org/issue46476#msg411321
Python no longer leaks any memory block at exit for the simplest command!
Moreover, I modified deepfreeze to only freeze importlib._bootstrap and importlib._bootstrap_external. It confirms that bpo-46449 is causing the negative reference count, because with these additional local changes I get a positive _Py_RefTotal:
vstinner commentedon Jan 27, 2022
New changeset 6c6a153 by Victor Stinner in branch 'main':
bpo-46417: signal: move siginfo_type to the module state (GH-30964)
6c6a153
vstinner commentedon Jan 27, 2022
I close the issue. I cleared most static types at exit. Following work can be done in bpo-40077 or other issues.
vstinner commentedon Jan 28, 2022
New changeset 9a24127 by Victor Stinner in branch 'main':
bpo-46417: _PyStructSequence_FiniType() updates _Py_RefTotal (GH-30988)
9a24127